Allow user to specify planner termination condition. #1695

mamoll · 2019-10-16T23:36:03Z

Description

It adds the ability to specify planner termination conditions in the ompl_planning.yaml file. It is mostly useful for optimizing/anytime planners, since it allows you to let them terminate before the timeout occurs (using either the CostConvergence or ExactSolution termination condition) and testing purposes (running for a fixed number of iterations). The default behavior doesn't change.

Checklist

Required by CI: Code is auto formatted using clang-format
Extend the tutorials / documentation reference
Document API changes relevant to the user in the MIGRATION.md notes
Create tests, which fail without this PR reference
Include a screenshot if changing a GUI
While waiting for someone to review your request, please help review another open pull request to support the maintainers

…er instances and their parameters via config file

…anning.yaml file

v4hn · 2019-10-17T11:33:44Z

Repeating Robert's comment in #1686, is it (easily) possible to #ifdef this change to succeed at least with OMPL in ROS melodic (or even kinetic)?

…older versions of MoveIt

mamoll · 2019-10-18T17:56:40Z

I added an OMPL version check for backwards compatibility. This feature should probably be documented somewhere. Where?

rhaschke · 2019-10-18T20:34:33Z

I cannot verify compatibility with released OMPL. Generally looks good to me. Please fix the compiler warning disturbing Travis.

henningkayser · 2019-11-01T11:26:27Z

@mamoll #1686 is merged. Could you rebase this PR and fix remaining warnings?

mamoll · 2019-11-04T20:29:19Z

@mamoll #1686 is merged. Could you rebase this PR and fix remaining warnings?

done.

mamoll · 2019-11-13T21:02:05Z

@henningkayser can you approve & merge? I need this to add your multi query planner support that touches some of the same code.

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

BryceStevenWilley

LGTM. Some documentation, either in the header or in a tutorial would help (not sure if there's a open PR for that already).

BryceStevenWilley · 2019-11-20T14:57:22Z

...it_planners/ompl/ompl_interface/include/moveit/ompl_interface/model_based_planning_context.h

@@ -317,6 +317,8 @@ class ModelBasedPlanningContext : public planning_interface::PlanningContext
  virtual ob::StateSamplerPtr allocPathConstrainedSampler(const ompl::base::StateSpace* ss) const;
  virtual void useConfig();
  virtual ob::GoalPtr constructGoal();
+  virtual ob::PlannerTerminationCondition constructPlannerTerminationCondition(double timeout,


I'd add documentation here, with what params the functions looks at (the spec's "termination condition"), and what it expects/ generates.

Beautiful documentation! Can you copy it to https://ros-planning.github.io/moveit_tutorials/doc/ompl_interface/ompl_interface_tutorial.html also?

See moveit/moveit_tutorials#427

davetcoleman

Looking good!

davetcoleman · 2019-11-20T18:49:51Z

...it_planners/ompl/ompl_interface/include/moveit/ompl_interface/model_based_planning_context.h

@@ -317,6 +317,8 @@ class ModelBasedPlanningContext : public planning_interface::PlanningContext
  virtual ob::StateSamplerPtr allocPathConstrainedSampler(const ompl::base::StateSpace* ss) const;
  virtual void useConfig();
  virtual ob::GoalPtr constructGoal();
+  virtual ob::PlannerTerminationCondition constructPlannerTerminationCondition(double timeout,


Beautiful documentation! Can you copy it to https://ros-planning.github.io/moveit_tutorials/doc/ompl_interface/ompl_interface_tutorial.html also?

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

davetcoleman · 2019-11-20T18:53:00Z

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

+ompl::base::PlannerTerminationCondition ompl_interface::ModelBasedPlanningContext::constructPlannerTerminationCondition(
+    double timeout, const ompl::time::point& start)
+{
+  auto it = spec_.config_.find("termination_condition");


Add code to the setup assistant to add this feature to the OMPL outputOMPLPlanningYAML() config file:
https://github.com/ros-planning/moveit/blob/master/moveit_setup_assistant/src/tools/moveit_config_data.cpp#L206

It's an advanced, optional setting. Like optimization_objective it typically doesn't get set explicitly.

Fair, as long as its in the tutorials

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

…c and older has been dropped

davetcoleman · 2019-11-20T20:30:45Z

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

 #include <ompl/base/samplers/UniformValidStateSampler.h>
 #include <ompl/base/goals/GoalLazySamples.h>
 #include <ompl/tools/config/SelfConfig.h>
 #include <ompl/base/spaces/SE3StateSpace.h>
 #include <ompl/datastructures/PDF.h>
+// \todo remove when ROS Melodic and older are no longer supported


Suggested change

// \todo remove when ROS Melodic and older are no longer supported

// TODO(mamoll) remove when ROS Melodic and older are no longer supported

Small style standard that wasnt documented. Now it is:
https://github.com/ros-planning/moveit.ros.org/pull/379/files

davetcoleman · 2019-11-20T20:31:16Z

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

+ompl::base::PlannerTerminationCondition ompl_interface::ModelBasedPlanningContext::constructPlannerTerminationCondition(
+    double timeout, const ompl::time::point& start)
+{
+  auto it = spec_.config_.find("termination_condition");


Fair, as long as its in the tutorials

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp

henningkayser and others added 5 commits October 7, 2019 14:05

Add OMPL planner 'AnytimePathShortening'

d6cd469

clean up registration of planner allocators

a3c0a30

add parameter to geometric::AnytimePathShortening method to set plann…

c07ee4f

…er instances and their parameters via config file

clang-format

f459ebe

add support for specifying planner termination conditions via ompl_pl…

c90e38f

…anning.yaml file

mamoll requested review from BryceStevenWilley, davetcoleman and rhaschke as code owners October 16, 2019 23:36

rhaschke force-pushed the master branch from dce9fd6 to b1b1b98 Compare October 17, 2019 07:20

add OMPL version check to make new feature backwards compatible with …

5b206bf

…older versions of MoveIt

mamoll changed the title ~~Pr terminationcondition~~ Allow user to specify planner termination condition. Oct 18, 2019

mamoll mentioned this pull request Oct 23, 2019

WIP: Implement CostConvergenceTerminationCondition for OMPL* planners #1557

Closed

fix catkin_lint warning

d7044c7

mamoll added 2 commits November 4, 2019 12:23

fix compiler warning

25d2a29

Merge branch 'master' into pr-terminationcondition

392c028

henningkayser reviewed Nov 20, 2019

View reviewed changes

moveit_planners/ompl/ompl_interface/src/model_based_planning_context.cpp Show resolved Hide resolved

mamoll mentioned this pull request Nov 20, 2019

Can I set OMPL optimization_objective programmingly? #1445

Open

fix logical error (ROS_ERROR is not fatal)

265a135

BryceStevenWilley approved these changes Nov 20, 2019

View reviewed changes

add docs for setting termination condition

3adc39a

davetcoleman reviewed Nov 20, 2019

View reviewed changes

add note to remove #if guard for OMPL version once support for Melodi…

e3dd92c

…c and older has been dropped

mamoll mentioned this pull request Nov 20, 2019

document new feature for setting planner termination conditions in OMPL moveit/moveit_tutorials#427

Merged

2 tasks

davetcoleman approved these changes Nov 20, 2019

View reviewed changes

correct/add TODOs for OMPL version checks

e63564c

davetcoleman merged commit f96e127 into moveit:master Nov 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow user to specify planner termination condition. #1695

Allow user to specify planner termination condition. #1695

mamoll commented Oct 16, 2019 •

edited

Loading

v4hn commented Oct 17, 2019

mamoll commented Oct 18, 2019

rhaschke commented Oct 18, 2019

henningkayser commented Nov 1, 2019

mamoll commented Nov 4, 2019

mamoll commented Nov 13, 2019

BryceStevenWilley left a comment

BryceStevenWilley Nov 20, 2019

mamoll Nov 20, 2019

davetcoleman Nov 20, 2019

mamoll Nov 20, 2019

davetcoleman left a comment

davetcoleman Nov 20, 2019

davetcoleman Nov 20, 2019

mamoll Nov 20, 2019

davetcoleman Nov 20, 2019

davetcoleman Nov 20, 2019 •

edited

Loading

davetcoleman Nov 20, 2019

	// \todo remove when ROS Melodic and older are no longer supported
	// TODO(mamoll) remove when ROS Melodic and older are no longer supported

Allow user to specify planner termination condition. #1695

Allow user to specify planner termination condition. #1695

Conversation

mamoll commented Oct 16, 2019 • edited Loading

Description

Checklist

v4hn commented Oct 17, 2019

mamoll commented Oct 18, 2019

rhaschke commented Oct 18, 2019

henningkayser commented Nov 1, 2019

mamoll commented Nov 4, 2019

mamoll commented Nov 13, 2019

BryceStevenWilley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davetcoleman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davetcoleman Nov 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mamoll commented Oct 16, 2019 •

edited

Loading

davetcoleman Nov 20, 2019 •

edited

Loading