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

Tutorial on OMPL's Constrained State Space for position or orientation constraints #651

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

gautz
Copy link

@gautz gautz commented Jun 28, 2021

Description

Tutorial that goes together with the pull request on the main MoveIt repo: moveit/moveit#2742

Co-authored-by: @JeroenDM [email protected]

This PR is consists of:

Required PR in MoveIt:
-moveit/moveit#2742

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

@welcome
Copy link

welcome bot commented Jun 28, 2021

Thanks for helping in improving MoveIt and open source robotics!

Copy link
Contributor

@JStech JStech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I ran the tutorial (using the appropriate MoveIt branch) and it ran fine. I just suggested a few grammar things.

There are a few places in the text where there are links that would also be appropriate to be formatted in fixed-width font (such as the enforce_joint_model_state_space option). I'm not familar with reStructuredText well enough to know how to do that (or if it's even possible). It's no big deal either way.

I'd also suggest adding the directory to the root CMakeLists.txt. I'm guessing if you built this in an installed workspace it wouldn't work without that.

Co-authored-by: John Stechschulte <[email protected]>
@gautz
Copy link
Author

gautz commented Jul 6, 2021

I changed a bit the position constrained tutorial and added an example for collision-avoidance. It consists of:

  • position constraints represented with a box defining error bounds (goal translated+rotated)
    box_bounds_constrained_position
  • position constraints represented with a plane. The infinitesimal dimension is defined thanks to use_equality_constraints (goal translated+rotated)
    1_plane_equality_constrained_position
  • position constraints represented with a plane with Obstacle collision avoidance (goal translated)
    2_plane_equality_constrained_collision
  • position constraints represented with a line. The infinitesimal dimensions is defined thanks to use_equality_constraints (goal translated)
    3_line_equality_constrained_position

Computation times are mostly 1s except for the collision avoidance 15-30s

Copy link
Contributor

@JeroenDM JeroenDM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it that we finally can add an example with orientation constraints. But it seems it was not added to the tutorial text yet. This PR needs a bit of work, I'm sorry I did not go through it in detail earlier.

I will first focus my attention on the main PR for now. I can always come back here later and fix some of my own review comments myself.


When can I use this planner?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The interface currently only supports position constraints on any link of the robot, where the constrained region is represented using a box_. The planning approach provides an alternative for the `enforce_joint_model_state_space`_ option. It is expected to be most valuable for constraint regions that have a small (or zero) volume in Cartesian space, where the rejection sampling does not always works. For example, keeping the end-effector on a plane or along a line.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true anymore as the tutorial contains an example with orientation constraints.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we split the tutorial in two PRs: 1 for position + 1 for orientation (+ eventually pose) ?
Or we keep one PR that will work partly after the moveit position PR is merged and entirely once the moveit orientation PR is merged?


.. tutorial-formatter:: ./scripts/ompl_constrained_planning_tutorial.py

.. _this video: https://youtu.be/RkPydgtIq-M
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should create a new video once this is finished.

Comment on lines +660 to +700
# print("============ Press enter to continue with the pose constrained planning problem.")
# input()
# tutorial.remove_all_markers()
# tutorial.remove_obstacle()

# pcm = tutorial.create_vertical_plane_constraints()
# ocm = tutorial.create_orientation_constraints()

# path_constraints = moveit_msgs.msg.Constraints()
# path_constraints.orientation_constraints.append(ocm)
# path_constraints.position_constraints.append(pcm)
# pose_goal = tutorial.create_pose_goal_under_obstacle()
# # pcm = tutorial.create_vertical_plane_constraints()

# move_group.set_start_state(start_state)
# move_group.set_pose_target(pose_goal)
# move_group.set_path_constraints(path_constraints)
# move_group.set_planning_time(1)
# move_group.plan()
# move_group.clear_path_constraints()

# print("============ Press enter to continue with the pose constrained planning problem with obstacle.")
# input()
# tutorial.remove_all_markers()
# tutorial.add_obstacle()

# pcm = tutorial.create_vertical_plane_constraints()
# ocm = tutorial.create_orientation_constraints()

# path_constraints = moveit_msgs.msg.Constraints()
# path_constraints.orientation_constraints.append(ocm)
# path_constraints.position_constraints.append(pcm)
# pose_goal = tutorial.create_pose_goal_under_obstacle()
# # pcm = tutorial.create_vertical_plane_constraints()

# move_group.set_start_state(start_state)
# move_group.set_pose_target(pose_goal)
# move_group.set_path_constraints(path_constraints)
# move_group.set_planning_time(60)
# move_group.plan()
# move_group.clear_path_constraints()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is replaced by a new version in the tutorial we can remove the code in comments to avoid confusion. I forgot why this is in comments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added and could test it. I had in mind to maybe add pose constraints to my moveit PR. I think if we keep orientation constraints in this PR here we can keep pose constraints too. I will wait to see how we arrange the moveit PRs before resolving this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest keeping this PR open until orientation constraints are also merged. So indeed it could be interesting to add this example for real.

Also, I noticed some of the tutorial cases still fail sometimes. (Planner timeout I think?) @gautz if you have ideas to improve this situation they're more than welcome :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan to work on an orientation constraints PR ? Otherwise I could try to cherry-pick relevant commits and open a PR this week.

Solving also fails for collision-avoidance examples around 1/4th of the time. My plan was to test Atlas based solving to see whether it improves performance and reliability. I don't have more ideas, except maybe tuning some numerical parameters.

@JeroenDM
Copy link
Contributor

@gautz

Solving also fails for collision-avoidance examples around 1/4th of the time. My plan was to test Atlas based solving to see whether it improves performance and reliability. I don't have more ideas, except maybe tuning some numerical parameters.

Maybe we can add a note to the tutorial that performance is still spotty and needs improvement. But it still provides a valid alternative to try when solving constraint problems? (And using Atlas could be one of these future improvements.)

Do you plan to work on an orientation constraints PR ? Otherwise I could try to cherry-pick relevant commits and open a PR this week.

I'm sorry I hoped to have some more time now but, alas, I don't. If you cherry-pick (again) I'll try to be very active in the review :) Thanks again for keeping this alive!

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