Ruckig-smoothing : reduce number of duration extensions #1990

ibrahiminfinite · 2023-03-05T05:41:22Z

Description

Enhancement to extent the waypoint durations for only the failing waypoints.

Fixes #1966

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

AndyZe

Looks like you are on the right track!

Apologies that I won't be able to test this for 1 week b/c I'm traveling.

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp

codecov · 2023-03-05T19:44:00Z

Codecov Report

Patch coverage: 15.79% and project coverage change: +0.01 🎉

Comparison is base (3907009) 50.88% compared to head (c006722) 50.89%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1990      +/-   ##
==========================================
+ Coverage   50.88%   50.89%   +0.01%     
==========================================
  Files         374      374              
  Lines       31356    31355       -1     
==========================================
+ Hits        15953    15954       +1     
+ Misses      15403    15401       -2

Impacted Files	Coverage Δ
...rajectory_processing/src/ruckig_traj_smoothing.cpp	`60.41% <15.79%> (+0.82%)`	⬆️

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

AndyZe · 2023-03-06T13:19:48Z

We'll likely need to rebase this PR on #1963. It already got merged into MoveIt1 and is just waiting for MoveIt 2. No big deal, but it breaks this trajectory extension stuff out into a new function and fixes some small bugs.

If you want, you could close this PR and open a new one based on that branch now (andyz/ruckig_duration). Otherwise we can take care of rebasing this one later.

ibrahiminfinite · 2023-03-06T13:24:06Z

If there are no issues in continuing with this PR and rebasing later, will be good to continue in this one.
I will be working on this today.

ibrahiminfinite · 2023-03-06T14:31:38Z

Hi @AndyZe ,

Apologies for pinging you while you are in travel.

Do you happen to have any specific test cases that fail the smoothing and trigger duration extension?
It would be good to run a timing test to see how much of a speed up this gives.

AndyZe · 2023-03-06T15:07:12Z

Great question! The test here has duration extension: https://github.com/ros-planning/moveit2/blob/56f200fd34e64c4304ad8d550318de7d327e3644/moveit_core/trajectory_processing/test/test_ruckig_traj_smoothing.cpp#L106

BTW, I'm sorry that it will be a little pain but I really think you should rebase on #1963. The more I think about it, the more I realize it fixes some other bugs related to this PR. You could even close this PR and start with a brand new one.

ibrahiminfinite · 2023-03-06T17:31:03Z

It should include the changes from andyz/ruckig_duration now.

ibrahiminfinite · 2023-03-08T16:35:50Z

Might be better to close this one and open new.
There seems to be some SHA conflict.

henningkayser · 2023-03-10T11:18:34Z

@ibrahiminfinite #1963 has been merged into main. Please reapply (rebase or cherry-pick) only your commits onto main, then everything should work. No need to close/reopen.

ibrahiminfinite · 2023-03-10T11:26:04Z

@ibrahiminfinite #1963 has been merged into main. Please reapply (rebase or cherry-pick) only your commits onto main, then everything should work. No need to close/reopen.

Thanks ! , will do that.

AndyZe

I built this locally and tested it out. I don't see any weird behavior 👍 Thanks!

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp

ibrahiminfinite · 2023-03-14T01:44:55Z

Thankyou for your guidance !

ibrahiminfinite · 2023-03-15T07:15:07Z

The humble-ci workflow failure seems to be from ompl planning_context_manager.
Is that related to this PR?

AndyZe · 2023-03-15T13:31:48Z

Nope, nothing to do with this PR. main has the same failure. 👍 We need to wait for a second review anyway, so just sit back and relax.

henningkayser

I'll approve the changes after my comment is addressed.

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp

Increase priority for constrained planning state space (moveit#1300) * Change priority for the constrained planning state space * Fix constrained planning tests * Use PRM instead of RRTConnect --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> Remove "new" from smart pointer instantiation (moveit#2019) Temporarily disable TestPathConstraints with the Panda robot (moveit#2016) This test has become flaky since it was modified to use the OMPL constrained state space (moveit#2015). Fix mimic joints with TOTG (moveit#1989) Fix include install destination (moveit#2008) Co-authored-by: Henning Kayser <henningkayser@picknik.ai> Co-authored-by: Tyler Weaver <maybe@tylerjw.dev> Ruckig-smoothing : reduce number of duration extensions (moveit#1990) * extend duration only for failed segment * update comment * Remove trajectory reset before extension * readability improvement * Remove call to RobotState update --------- Co-authored-by: ibrahiminfinite <ibrahimjkd@@gmail.com> Co-authored-by: AndyZe <andyz@utexas.edu> Add stale GHA (moveit#2022) * Issues and PRs are labeled as stale after 45 days. * Stale issues are closed after another 45 days. Enable workflow_dispatch for stale GHA Remove invalid description field in GHA Add callback for velocity scaling override + fix params namespace not being set (moveit#2021) Fix python tests (moveit#1979) * ensure joint models in robot_model submodule * add build tests Upgrade apt dependencies --with-new-pkgs (moveit#2039) 2.7.1 🛠️ Bump actions/stale from 7 to 8 (moveit#2037) Allow ci-testing Dockerfile to update the ROS_DISTRO (moveit#2035) Move displaced launch file into planning_component_tools (moveit#2044) Delete the Ruckig "batches" option, deprecated by moveit#1990 (moveit#2028) Added set_robot_trajectory_msg to python bindings (moveit#2050) Use $DISPLAY rather than assuming :0 (moveit#2049) * Use $DISPLAY rather than assuming : * Double quotes --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> Optionally mitigate Ruckig overshoot (moveit#2051) * Optionally mitigate Ruckig overshoot * Cleanup Update description of moveit_ros_planning_interface (moveit#2045) * Update description of moveit_ros_planning_interface * Update moveit_ros/planning_interface/package.xml Co-authored-by: Henning Kayser <henningkayser@picknik.ai> --------- Co-authored-by: Henning Kayser <henningkayser@picknik.ai> Add URDF Loader Exceptions and Fix Infinite While-Loop when URDF file isn't in a ROS Package (moveit#2032) * Fixed infinite while loop in utilities.cpp and added some exception handling to start screen widget * Fix trailing whitespace, fix getSharePath exception catch on empty request * Fix clang tidy suggestion and error message updates based on pr comments [hybrid planning] improve planning scene monitoring (moveit#1090) * Create new PSM in local constraint solver. Different type of collision checking. * Small boolean logic fixup * Don't configure planning scene monitor twice and pass scene as const * Remove not required call of updateSceneWithCurrentState() * Revert PlanningSceneMonitorConstPtr to PlanningSceneMonitorPtr * Set planning_scene_monitor update rate * Decrease planning scene update rate * Use `updateSceneWithCurrentState()` in psm * Revert the manner of collision checking --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> Document how collision checking includes descendent links (moveit#2058) Move stateless PlanningScene helper functions out of the class (moveit#2025) Readability: kinematic_state -> robot_state (moveit#2078) moveit_py citation (moveit#2029) Extract parallel planning from moveit cpp (moveit#2043) * Add parallel_planning_interface * Add parallel planning interface * Rename package to pipeline_planning_interface * Move plan_responses_container into own header + source file * Add plan_responses_contrainer source file * Add solution selection and stopping criterion function files * Remove parallel planning from moveit_cpp * Move parallel planning into planning package * Update moveit_cpp * Drop planning_interface changes * Add documentation * Update other moveit packages * Remove removed header * Address CI complains * Address clang-tidy complains * Address clang-tidy complains 2 * Address clang-tidy complains 3 * Extract planning pipeline map creation function from moveit_cpp * Cleanup comment * Use const moveit::core::RobotModelConstPtr& * Formatting * Add header descriptions * Remove superfluous TODOs * Cleanup PILZ: Throw if IK solver doesn't exist (moveit#2082) * Throw if IK solver doesn't exist * Format enabled -wformat (moveit#2065) Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> Add test and debug issue where TOTG returns accels > limit (moveit#2084) lint fix lint fix 1 lint fix 2 lint fix 3

AndyZe reviewed Mar 5, 2023

View reviewed changes

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp Outdated Show resolved Hide resolved

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp Outdated Show resolved Hide resolved

ibrahiminfinite force-pushed the ibrahim/trajectory_processing_enhancement branch 3 times, most recently from d880c76 to 572014b Compare March 6, 2023 16:34

mergify bot mentioned this pull request Mar 7, 2023

Fix Ruckig termination condition #1963

Merged

ibrahiminfinite added 4 commits March 10, 2023 17:09

extend duration only for failed segment

896e69b

update comment

3814d2b

Remove trajectory reset before extension

97da2b6

readability improvement

dcacef1

ibrahiminfinite force-pushed the ibrahim/trajectory_processing_enhancement branch from 9542489 to dcacef1 Compare March 10, 2023 12:27

moveit deleted a comment from mergify bot Mar 13, 2023

Merge branch 'main' into ibrahim/trajectory_processing_enhancement

c006722

AndyZe approved these changes Mar 13, 2023

View reviewed changes

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp Show resolved Hide resolved

henningkayser reviewed Mar 15, 2023

View reviewed changes

moveit_core/trajectory_processing/src/ruckig_traj_smoothing.cpp Outdated Show resolved Hide resolved

ibrahiminfinite and others added 2 commits March 16, 2023 18:56

Remove call to RobotState update

35da797

Merge branch 'main' into ibrahim/trajectory_processing_enhancement

8ce3740

AndyZe merged commit 4c8fef5 into moveit:main Mar 16, 2023

AndyZe added a commit to AndyZe/moveit2 that referenced this pull request Mar 20, 2023

Delete the Ruckig "batches" option, deprecated by moveit#1990

5872d23

This was referenced Mar 20, 2023

Delete the Ruckig "batches" option, deprecated by #1990 #2028

Merged

Mitigate Ruckig overshoot moveit/moveit#3376

Merged

sjahr pushed a commit that referenced this pull request Mar 24, 2023

Delete the Ruckig "batches" option, deprecated by #1990 (#2028)

3f7e8e8

ibrahiminfinite deleted the ibrahim/trajectory_processing_enhancement branch June 3, 2023 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ruckig-smoothing : reduce number of duration extensions #1990

Ruckig-smoothing : reduce number of duration extensions #1990

ibrahiminfinite commented Mar 5, 2023 •

edited

Loading

AndyZe left a comment

codecov bot commented Mar 5, 2023 •

edited

Loading

AndyZe commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

AndyZe commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

ibrahiminfinite commented Mar 8, 2023

henningkayser commented Mar 10, 2023

ibrahiminfinite commented Mar 10, 2023

AndyZe left a comment

ibrahiminfinite commented Mar 14, 2023

ibrahiminfinite commented Mar 15, 2023

AndyZe commented Mar 15, 2023

henningkayser left a comment

Ruckig-smoothing : reduce number of duration extensions #1990

Ruckig-smoothing : reduce number of duration extensions #1990

Conversation

ibrahiminfinite commented Mar 5, 2023 • edited Loading

Description

Checklist

AndyZe left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 5, 2023 • edited Loading

Codecov Report

AndyZe commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

AndyZe commented Mar 6, 2023

ibrahiminfinite commented Mar 6, 2023

ibrahiminfinite commented Mar 8, 2023

henningkayser commented Mar 10, 2023

ibrahiminfinite commented Mar 10, 2023

AndyZe left a comment

Choose a reason for hiding this comment

ibrahiminfinite commented Mar 14, 2023

ibrahiminfinite commented Mar 15, 2023

AndyZe commented Mar 15, 2023

henningkayser left a comment

Choose a reason for hiding this comment

ibrahiminfinite commented Mar 5, 2023 •

edited

Loading

codecov bot commented Mar 5, 2023 •

edited

Loading