Skip to content

Optional forced use of JointModelStateSpaceFactory#541

Merged
davetcoleman merged 2 commits intomoveit:indigo-develfrom
henningkayser:indigo-devel_forcejointstatespace
Jul 23, 2017
Merged

Optional forced use of JointModelStateSpaceFactory#541
davetcoleman merged 2 commits intomoveit:indigo-develfrom
henningkayser:indigo-devel_forcejointstatespace

Conversation

@henningkayser
Copy link
Copy Markdown
Member

Adds an optional ompl group parameter 'force_joint_model_state_space' to enforce the use of JointModelStateSpaceFactory for problem representation. This is convenient to work around issues with planning problems represented in PoseModelStateSpace, as is the case with some cartesian path planning scenarios.

@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented Jul 11, 2017

This is another patch I supervised.

The state space for OMPL is created for each request through a number of rather over-engineered factory abstractions. As a result MoveIt can on-the-fly choose to use either the (traditional) joint space or a cartesian state space that invokes IK for each state (the PoseModelStateSpace) as search space.
The latter is automatically chosen when orientation path constraints are given for the planning problem.
This makes some sense because if you restrict a link's orientation, IK serves as a generative state sampler whereas joint space planning will use rejection sampling.

However, it might happen that paths planned through the latter space flip the chosen IK solutions during the trajectory, rendering the trajectory invalid. This ROS-answers question illustrates the phenomena quite well.

A fix for the particular problem would have to restrict the distance of IK solutions for consecutive points.
This is no trivial task though and involves looking fairly deep into OMPL and its MoveIt interface and would most likely have to change some established interfaces.

On the other hand, I also don't see a reason to require the use of the pose state space for orientation constraint problems. Especially with weak constraints (e.g. restricting yaw to within pi/4), the joint state space works well enough.

So I proposed to add a parameter here, that can enforce the use of the JointModelStateSpace for particular planning groups.

See here for an example configuration we use at the moment.

While this can fix some issues with path-constraint planning, it will also increase planning time for more-strict constraints because of the rejection sampling.
This is the reason for the pose state space to exist in the first place.
Thus, I would not change the defaults here and also refrain from advertising it too much.

Please review @davetcoleman , this is the last upstream change we need to run off-the-shelf MoveIt with one of our demos.

@v4hn v4hn requested a review from davetcoleman July 11, 2017 11:42
Copy link
Copy Markdown
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

This works for me. I think @mamoll should be given a heads up

// the set of planning parameters that can be specific for the group (inherited by configurations of that group)
static const std::string KNOWN_GROUP_PARAMS[] = { "projection_evaluator", "longest_valid_segment_fraction" };
static const std::string KNOWN_GROUP_PARAMS[] = { "projection_evaluator", "longest_valid_segment_fraction",
"force_joint_model_state_space" };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i don't like the term "force" because i keep thinking you are referring to power/torque. how about "mandate_joint_model_state_space"?

Copy link
Copy Markdown
Contributor

@v4hn v4hn Jul 17, 2017

Choose a reason for hiding this comment

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

Could you agree to "enforce" instead? I see your point, but I don't really like mandate...


ModelBasedPlanningContextPtr context =
getPlanningContext(pc->second, boost::bind(&PlanningContextManager::getStateSpaceFactory2, this, _1, req), req);
// Check for forced use of JointModelStateSpace
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please expand this comment (just copy some of the PR description) because this is somewhat of a hack

@davetcoleman
Copy link
Copy Markdown
Member

a number of rather over-engineered factory abstractions

Finally someone understands! I made a state space for humanoids once... OMPL interface was so complicated to wade through especially since there is a lot of dead code :)

@davetcoleman
Copy link
Copy Markdown
Member

Also @v4hn do you mind putting some of your great documentation on the State Spaces in the tutorial page?

@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented Jul 17, 2017

Also @v4hn do you mind putting some of your great documentation on the State Spaces in the tutorial page?

Once this is merged, I'll put it on my todo list :)

@mamoll
Copy link
Copy Markdown
Contributor

mamoll commented Jul 17, 2017

@zkingston, can you have a look at this?

@davetcoleman
Copy link
Copy Markdown
Member

@mamoll I'd like to merge unless you or @zkingston have something to add?

@mamoll
Copy link
Copy Markdown
Contributor

mamoll commented Jul 22, 2017

Seems fine to me, although I am probably not the best person to review this.

@davetcoleman davetcoleman merged commit bb92558 into moveit:indigo-devel Jul 23, 2017
davetcoleman pushed a commit that referenced this pull request Jul 23, 2017
* Implements optional ompl_planning config parameter 'force_joint_model_state_space'.

* Renames parameter to 'enforce_joint_model_state_space'.
Expands workaround comment.
davetcoleman pushed a commit that referenced this pull request Jul 23, 2017
* Implements optional ompl_planning config parameter 'force_joint_model_state_space'.

* Renames parameter to 'enforce_joint_model_state_space'.
Expands workaround comment.
@davetcoleman
Copy link
Copy Markdown
Member

Cherry picked to jade and kinetic

@pdmike
Copy link
Copy Markdown

pdmike commented Oct 24, 2017

Hello, i cannot find the file,
moveit_planners/ompl/ompl_interface/src/ompl_interface.cpp in my ROS. Have i missed to install some package?

@moveit moveit deleted a comment from pdmike Nov 17, 2017
@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented Nov 17, 2017

It's part of the moveit git repository. But if you want to use the feature in this branch, you have to set the ros-parameter. It has nothing to do with editing the file.

@henningkayser henningkayser deleted the indigo-devel_forcejointstatespace branch December 8, 2017 11:17
@davetcoleman
Copy link
Copy Markdown
Member

@v4hn I noticed you never documented this extra parameter, could you please do that by simply copying your explanation into the tutorials?

@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented Feb 21, 2018

It's on my TODO list (now)

@as-wanfang
Copy link
Copy Markdown

as-wanfang commented May 2, 2018

@v4hn I used the parameter as you suggested with Approximated Constraint Manifolds and it works well. But I have a doubt: I used RRTConnect and I'd like to known how to guarantee that the interpolated point in joint space also satisfies the orientation constraints. For example, the extend step in RRTConnect involves interpolating between the nearest point and the sampled new point from the Approximated Constraint Manifolds.

@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented May 2, 2018 via email

@as-wanfang
Copy link
Copy Markdown

@v4hn That's exactly what I understands about interpolation. But I found in moveit that the interpolated points in the growTree function in RRTConnect always satisfy the constraint (end effector kept upright). The parameter controls the interpolation step is the range parameter in RRTConnect. I'm using UR5 and below is an example of interpolating between two joint configurations:
from = (-0.532821, 0.674111, -1.799397, -0.512343, -1.575813, 0.027108)
to = (-0.927468, 0.004256, -2.347224, 0.777637, -1.603555, -0.268515)
midlle = (-0.7301445, 0.3391835, -2.0733105, 0.132647 , -1.589684 ,-0.1207035)

mintar added a commit to DFKI-NI/mobipick that referenced this pull request Apr 23, 2021
This should avoid trajectory jumps (sudden, discontinuous reconfigurations) when using cartesian constraints, at the cost of making planning slower.

See moveit/moveit#541 (comment).
yifeifang added a commit to yifeifang/fetch_ros that referenced this pull request May 13, 2021
This will enforce JointModelStateSpaceFactory for problem representing see
moveit/moveit#541 for detail
basically this would allow path constraints in moveit_commander to work properly
@rbtchun3
Copy link
Copy Markdown

rbtchun3 commented Feb 21, 2022

Hey,
Does enforce_joint_model_state_space work with melodic devel branch?
I changed ompl_planning.yaml like this. I tried to make the end-effector of the panda arm level when moving, but the end effector still flips in the path.

RRTConnectkConfigDefault:
type: geometric::RRTConnect
range: 0.0
enforce_joint_model_state_space: True

I wonder if I missed something or what could be wrong.
Thanks!

@v4hn
Copy link
Copy Markdown
Contributor

v4hn commented Feb 22, 2022 via email

rhaschke added a commit to ubi-agni/moveit that referenced this pull request Jun 27, 2024
Constrained-based planning enables the PoseModelStateSpace by default, meaning that state samples are created by sampling
in SE(3) and then performing IK. This state space frequently caused issues, leading to the introduction of the parameter
 in PR moveit#541 as a work-around.
A more detailed analysis shows that also interpolation between states was performed in Cartesian space.
If the two interpolation ends are close in Cartesian but distant in joint space, the interpolated state (obtained from IK)
can be far away from both start and end as well (particularly for redundant arms), while its Cartesian pose is naturally close
to both interpolation states. This will lead to consecutive waypoints being close in Cartesian space but distant in joint space.
If such a trajectory is executed, the controller will interpolate in joint space, leading to a large and unchecked motion.

To avoid this issue, interpolation should be performed in joint space during planning as well.
The only drawback of this change is that trajectories are less linear in Cartesian space (see videos).
However, as the trajectory is random-sampled anyway, this shouldn't be a big issue.
rhaschke added a commit to ubi-agni/moveit that referenced this pull request Jun 27, 2024
Constrained-based planning enables the PoseModelStateSpace by default, meaning that state samples are created by sampling
in SE(3) and then performing IK. This state space frequently caused issues, eventually leading to the introduction of the
ROS parameter enforce_joint_model_state_space in PR moveit#541 as a work-around.

A more detailed analysis shows that the state space's interpolation approach is the culprit - performing interpolation in
Cartesian space. If the two interpolation ends are close in Cartesian but distant in joint space, the interpolated state
(obtained from IK) can be far away from both start and end (particularly for redundant arms), while its Cartesian pose is
naturally close to both interpolation states. This will lead to consecutive waypoints being close in Cartesian space but
distant in joint space. If such a trajectory is executed, the controller will interpolate in joint space, leading to a large
and unchecked motion.

To avoid this issue, interpolation should be performed in joint space during planning as well.
The only drawback of this change is that trajectories are less linear in Cartesian space.
However, as the trajectory is random-sampled anyway, this shouldn't be a big issue.
rhaschke added a commit that referenced this pull request Jul 2, 2024
* Fix constrained-based planning

Constrained-based planning enables the PoseModelStateSpace by default, meaning that state samples are created by sampling in SE(3) and then performing IK. This state space frequently caused issues, eventually leading to the introduction of the ROS parameter enforce_joint_model_state_space in PR #541 as a work-around.

A more detailed analysis shows that the state space's interpolation approach is the culprit - performing interpolation in Cartesian space. If the two interpolation ends are close in Cartesian but distant in joint space, the interpolated state (obtained from IK) can be far away from both start and end (particularly for redundant arms), while its Cartesian pose is naturally close to both interpolation states. This will lead to consecutive waypoints being close in Cartesian space but distant in joint space. If such a trajectory is executed, the controller will interpolate in joint space, leading to a large and unchecked motion.

To avoid this issue, interpolation should be performed in joint space during planning as well.
The only drawback of this change is that trajectories are less linear in Cartesian space.
However, as the trajectory is random-sampled anyway, this shouldn't be a big issue.

* PoseModelStateSpace: Use joint-space distance

Configurations may be similar in Cartesian EE pose, but very far apart in joint space.
What actually matters when connecting states is the joint-space distance.
LeroyR pushed a commit to CentralLabFacilities/moveit that referenced this pull request Jul 11, 2024
* Fix constrained-based planning

Constrained-based planning enables the PoseModelStateSpace by default, meaning that state samples are created by sampling in SE(3) and then performing IK. This state space frequently caused issues, eventually leading to the introduction of the ROS parameter enforce_joint_model_state_space in PR moveit#541 as a work-around.

A more detailed analysis shows that the state space's interpolation approach is the culprit - performing interpolation in Cartesian space. If the two interpolation ends are close in Cartesian but distant in joint space, the interpolated state (obtained from IK) can be far away from both start and end (particularly for redundant arms), while its Cartesian pose is naturally close to both interpolation states. This will lead to consecutive waypoints being close in Cartesian space but distant in joint space. If such a trajectory is executed, the controller will interpolate in joint space, leading to a large and unchecked motion.

To avoid this issue, interpolation should be performed in joint space during planning as well.
The only drawback of this change is that trajectories are less linear in Cartesian space.
However, as the trajectory is random-sampled anyway, this shouldn't be a big issue.

* PoseModelStateSpace: Use joint-space distance

Configurations may be similar in Cartesian EE pose, but very far apart in joint space.
What actually matters when connecting states is the joint-space distance.
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.

7 participants