Skip to content

[BUG FIX] Restore the carried object's pose when plan_path fails#2938

Open
HaozheZhang6 wants to merge 2 commits into
Genesis-Embodied-AI:mainfrom
HaozheZhang6:fix/plan-path-restore-object-on-failure
Open

[BUG FIX] Restore the carried object's pose when plan_path fails#2938
HaozheZhang6 wants to merge 2 commits into
Genesis-Embodied-AI:mainfrom
HaozheZhang6:fix/plan-path-restore-object-on-failure

Conversation

@HaozheZhang6

Copy link
Copy Markdown

Description

plan_path(..., with_entity=obj) mutates the scene whenever a planning attempt fails. Both RRT.plan and RRTConnect.plan restore the robot's qpos in the failure early-return (if is_invalid.all():), but only the success path also calls update_object(...) to put the carried object back. So a failed attempt leaves the object at whatever configuration the planner last explored. Because planning is retried (max_retry), a failed attempt also corrupts the relative grasp offset that the next retry captures, so even an eventually-successful plan can return with the object displaced.

The fix mirrors the success path: restore the object via update_object(...) in the failure branch of both planners (+2 lines each).

Related Issue

Resolves #2715

Motivation and Context

plan_path is a query and should not change the scene. Today, calling it with with_entity (e.g. a grasped cube) and hitting a failed attempt teleports the object, which corrupts downstream control and makes retried planning unsound.

How Has This Been / Can This Be Tested?

Issue repro (Franka + cube), headless, recording the cube pose immediately before/after plan_path(with_entity=cube) with no trajectory executed:

cube pose after plan_path |Δ|max
before fix [0.399, -0.021, 1.078] 0.844 m
after fix [0.300, 0.132, 0.233] 1.5e-8

Isolated single failed attempt (max_retry=0, in-collision goal so the attempt fails deterministically): before → the cube is dragged 1.12 m; after → preserved (~3e-8).

Added a regression test that reproduces this without depending on the planner's RNG (an in-collision goal makes the single attempt fail deterministically), then asserts the attached object's pose is unchanged:

pytest tests/test_rigid_physics.py -k plan_path_with_entity_preserves_state_on_failure

It passes with the fix and fails without it (the object moves ~1.16 m). ruff check / ruff format are clean on both files. I ran the new test on the CPU backend; I was not able to run the full suite on my box (its CUDA driver is too old for the installed torch), so I'm relying on CI for the rest.

Checklist:

  • I read the CONTRIBUTING document.
  • I followed the Submitting Code Changes section of CONTRIBUTING document.
  • I tagged the title correctly (including BUG FIX/FEATURE/MISC/BREAKING)
  • I updated the documentation accordingly or no change is needed.
  • I tested my changes and added instructions on how to test it for reviewers.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab7d96156d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread genesis/utils/path_planning.py Outdated
Comment on lines +593 to +594
if is_plan_with_obj:
self.update_object(ee_link_idx, obj_link_idx, _pos, _quat, envs_idx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restore the original object pose for non-current starts

When callers pass a qpos_start that differs from the robot's current pose, get_exclude_geom_pairs(...) above has already moved the robot to qpos_start before _pos/_quat are captured, so this new restore applies a start-relative grasp transform after setting the robot back to qpos_cur. In that scenario a failed plan_path(with_entity=...) still teleports the attached object instead of preserving its original world pose; the same pattern was added in the RRTConnect failure branch, so the regression test only covers the qpos_start=None case.

Useful? React with 👍 / 👎.

…urrent qpos_start

The grasp transform is captured relative to qpos_start (the robot is moved there by
get_exclude_geom_pairs before capture), so re-deriving the object pose at qpos_cur on
failure teleported it whenever qpos_start differed from the current pose. Snapshot the
object's world pose up front and restore it directly; extend the regression test with a
custom-qpos_start case that fails on the old restore.
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.

[Bug]: Calling plan_path with with_entity causes unintended object motion

1 participant