Start and End are already used for an entirely different concept,
so if anyone ever wants to read this code, we should use new terms instead.
Because the source state is the disabled state that *failed* to extend,
triggering the whole subtree to be disabled, I went for the new terms
DISABLED and DISABLED_FAILED.
The key to pruning in the Connecting stage was the following:
- Don't remove states during pruning, but only disable them.
They might become re-enabled due to further input.
- Distinguish START and END sides of a disabled solution tree to break their symmetry.
The START side from where we started disabling, can be re-enabled by a new partner state in
Connecting, the END side must not. This was important as, otherwise, the states would simply
get re-enabled immediately. The END side only gets re-enabled if the START side actually
connects the whole solution branch.
If a stage fails to find a solution, this often implies that further planning
on the open end(s) of connected InterfaceStates is not needed anymore.
Thus the InterfaceStates along all connected solution paths will be marked as disabled.
They are not removed from the pending state lists though, because they might get
reactivated by solutions found in future.
To this end, we introduced the method ContainerBase::onNewFailure().
To allow pruning, we need to enable and disable InterfaceStates to be considered for further
planning. In the past, we just indicated the disabled status with infinite costs.
However, because we might need to re-enable states (with previous state),
we need to separate these two concepts.
This is somewhat cumbersome because of the additional internal/external
layer introduced through the container.
But it's still better than leaving this unverified.
* unify usage of pimpl()
* fix StatePair constructors
* improve/add comments
* test_container: reset MOCK_ID for each test to facilitate identification of stages
requested in review.
Without support for custom aggregators, which we dropped again
after finding more flaws with it, I agree that this is the nicer solution.
On the downside, it converts the interfaces from copyable objects
to another round of shared_ptrs.
I added shortcuts for lambda costs to keep support for
`stage->setCostTerm([](auto&& s){ return 42; })`
without the additional
`stage->setCostTerm(LambdaCostTerm{ [](auto&& s){ return 42; } } )`
This reverts commit dc7ce9bdfac97eb468f5a850adcb27cc118b5fd7.
It turns out multiple places in SerialContainer's cost inference
expect 0.0 as the neutral element (which is why std::min and multiply did not work).
While these additional issues can be fixed, it would make the interface much less elegant.
We should consider adding it back if an actual use-case is there to discuss.
This pattern allows cost::Constant to override the hierarchical cost computation
for the SerialContainer and avoid traversing the graph.
I implemented the CostTerm::supports() pattern over a full double visitor pattern
with overloads for each SolutionBase specialization, because the SerialContainer
needs to know whether cost aggregation of the subsolutions should take place or whether
the SolutionSequence should be forwarded to the CostTerm.
This would not be possible with a `virtual double operator()(const SolutionSequence&)`
callback in CostTerm.
Alternatively, implementing the hierarchical aggregation in the default implementation
of this operator would be possible as well, but breaks intuition:
- the corresponding methods to handle `SubTrajectory` and `WrappedSolution` *have to*
default to not touching the solution's cost at all so it is inappropriate to have
the default implementation for the Sequence do something else
- The SerialContainer also aggregates costs outside the `computeCost()` interface
(in multiple places in `onNewSolution()` to aggregate costs along partial paths)
and thus moving the hierarchical aggregation to the CostTerm methods requires
the aggregator to be shared between the Container and the CostTerm,
The only shortcoming of the implemented approach, by contrast, is that user implementations
that want to handle WrappedSolution or SolutionSequence differently have to ensure
the supports_ flags are set correctly. Notice that most custom CostTerms will only
ever access SubTrajectories and this case is simplified with the provided CostTerm constructors.
It can be useful to change the default addition to other operators.
The simplest example is applying a Constant cost term to a container.
As the tests show, the visitor-based cost computation ends up adding
cost::Constant *for each subtrajectory*.
Probably the most invasive format patch, also changing some internal API.
I deliberately disabled ClassCase and MethodCase checks for the moment
to avoid public API changes in this patch set.