Improve docs for ResourceExtensionFunction in r_c_shortest_paths

[andrea-cassioli-maersk]

The ResourceExtensionFunction in r_c_shortest_paths takes a new and an old resource container.

To me, in the docs it is unclear whether

• the new Resource_Container is a copy of the old,
• or a fresh one constructed via default constructor.

In the former case I do not fully see the use of the old.

Could anyone clarify this?

[andrea-cassioli-maersk]

Anyone does have a clue? It seems from this constructor

 r_c_shortest_paths_label(const unsigned long n,
        const Resource_Container& rc = Resource_Container(),
        const boost::shared_ptr<
            r_c_shortest_paths_label< Graph, Resource_Container > >
            pl
        = boost::shared_ptr<
            r_c_shortest_paths_label< Graph, Resource_Container > >(),
        const typename graph_traits< Graph >::edge_descriptor& ed
        = graph_traits< Graph >::edge_descriptor(),
        const typename graph_traits< Graph >::vertex_descriptor& vd
        = graph_traits< Graph >::vertex_descriptor())
    : num(n)
    , cumulated_resource_consumption(rc)
    , p_pred_label(pl)
    , pred_edge(ed)
    , resident_vertex(vd)
    , b_is_dominated(false)
    , b_is_processed(false)
    {
    }

that one might pass a default resource container. But the code seems to pass the old container and thus make a copy. This seems conflicting to me.

Since the new and the old containers are passed, I would imagine the new to be pristine and to be set using the old as starting point for the extension. But because the new is a copy of the old, the old is redundant.

Any thoughts?

[andreacassioli]

I have done some digging and

• the implementation does copy the current container and pass the current and the new both to the extension function
• In general the extension function does not need the current container, but just the new one. The only explanation I have is that the new container might be not fully initialized by the copy constructor, and thus passing the current container is necessary.

From my side this issue can be closed. @jeremy-murphy

[Becheler]

Hi @andreacassioli !

Thanks for filing the issue, I looked into it, looks like a documentation gap again 😉

If I understood correctly, these points are not made clear :

• The docs don't say what state new_cont is in when the function is called: it's actually a pre-made copy of old_cont, but the user has to read the source to find out.
• The docs don't explain why both old_cont and new_cont are passed, given that new_cont is already a copy, and that leaves users wondering if old_cont is redundant.
• The return value of the extension function isn't documented: false signals an infeasible extension and prunes the label, but the docs don't say so.

Would something like this have prevented the issue you bumped into:

Before invocation, new_cont is initialized as a copy of old_cont. The function should mutate new_cont to reflect the resources consumed along the edge.

Return true if the extension is feasible, or false to discard the label (e.g. when a resource limit would be exceeded). The old_cont parameter is provided for reference, in case the update is more naturally expressed relative to the prior state.

If so, I will gladly open the PR to make it clear 😄 Since we are at it, any complaint towards the recently reworked documentation page in #491 and its example will be accounted for:

• old documentation page: https://www.boost.org/doc/libs/latest/libs/graph/doc/r_c_shortest_paths.html
• new documentation page: https://becheler.github.io/graph/graph/algorithms/shortest_paths/r_c_shortest_paths.html

[andreacassioli]

Hey @Becheler , you got it right! I think the changes you propose are fine and will clarify the interface and behavior.

As for the new documentation, I think it looks good. As my personal preference,

• the example should be move a bit down the page, and connected with some of the text in the bottom where the link is provided (a la cppreference)
• the layout still has some quirkiness, like the non scrolling right panel, and some headers font size that make sometimes hard to clearly see where sections start stop
• there are reference two papers that are not in the BGL references.

[Becheler]

Thank you Andrea !

I noted your preferences and I generally agree with them. My main issue is that currently it's hard to have an algorithm documentation template that works equally well for all algorithms.

Some algorithms have extremely long descriptions, scrolling to the examples would be (imo) worse than scrolling past an example to reach a description. Indeed examples seem to have a relatively similar length.

And I wonder if a per-algorithm documentation rework would be a better idea once the general infrastructure is merged ?

I dont know yet how to make the right toc pannel scrollable in a satisfying way.

I feel it's also slowly becoming out of scope of the documentation migration, I'm progressively more and more anxious about this 40k loc PR sitting around ahahah

I will look at the paper references problem :)