Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiPlanner #429

Merged
merged 5 commits into from
Feb 23, 2023
Merged

MultiPlanner #429

merged 5 commits into from
Feb 23, 2023

Conversation

rhaschke
Copy link
Contributor

This introduces MultiPlanner, a new meta planner implementing the PlannerInterface, that runs multiple alternative planners in sequence and returns the first found solution.
This is useful to sequence different planning strategies of increasing complexity, e.g. Cartesian or joint-space interpolation first, then OMPL, ...
This is (slightly) different from the Fallbacks container, as the MultiPlanner directly applies its planners to each individual planning job. In contrast, the Fallbacks container first runs the active child to exhaustion before switching to the next child, which possibly applies a different planning strategy.

As the children planners should be able to use different timeouts (than the stage employing MultiPlanner), I also introduced a timeout property for all planners.

Doing so, I noticed and fixed a bug in the timeout handling of JointInterpolation.

The timeout parameter was essentially ignored and the check was always true.
a planner that tries multiple planners in sequence
The MultiPlanner requires to set individual timeouts for its planners.
@rhaschke rhaschke requested a review from v4hn February 17, 2023 17:32
@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Base: 58.85% // Head: 58.63% // Decreases project coverage by -0.21% ⚠️

Coverage data is based on head (db6d90a) compared to base (eae0bdc).
Patch coverage: 17.50% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #429      +/-   ##
==========================================
- Coverage   58.85%   58.63%   -0.21%     
==========================================
  Files          89       90       +1     
  Lines        8386     8419      +33     
==========================================
+ Hits         4935     4936       +1     
- Misses       3451     3483      +32     
Impacted Files Coverage Δ
...oveit/task_constructor/solvers/planner_interface.h 100.00% <ø> (ø)
core/src/solvers/cartesian_path.cpp 82.93% <0.00%> (ø)
core/src/solvers/multi_planner.cpp 0.00% <0.00%> (ø)
core/src/solvers/joint_interpolation.cpp 87.76% <100.00%> (ø)
core/src/solvers/pipeline_planner.cpp 67.75% <100.00%> (ø)
core/src/solvers/planner_interface.cpp 100.00% <100.00%> (ø)
demo/src/pick_place_task.cpp 96.78% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rhaschke rhaschke merged commit 4a320f3 into moveit:master Feb 23, 2023
@rhaschke rhaschke deleted the multi-planner branch February 23, 2023 19:44
@v4hn
Copy link
Contributor

v4hn commented Feb 28, 2023

Sorry for leaving this specifically hanging. There's clearly no harm in supporting the new mechanism in MTC now that it's upstream, but I still think this whole project should actually be handled as a variation of the Fallback container within the MTC framework instead of having its own opaque implementation as a single planner.
However, to get the same behavior rather big changes are required in MTC (including parallel stage computations).

The other fixes/cleanups make sense too.

@rhaschke
Copy link
Contributor Author

As discussed in the initial comment, I don't see how this feature could be implemented as a Fallback container.

@v4hn
Copy link
Contributor

v4hn commented Feb 28, 2023 via email

@rhaschke
Copy link
Contributor Author

Even if you can run multiple children in parallel, the semantics is different: Fallback containers run their children to exhaustion before switching to the next child/method, while the MultiPlanner switches to the next method on failure of the previous one(s) for each and every job immediately.

@v4hn
Copy link
Contributor

v4hn commented Feb 28, 2023 via email

sjahr pushed a commit to sjahr/moveit_task_constructor that referenced this pull request Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants