Retune Franka deformable lift for stable grasping - #6831
Conversation
github-actions
Bot
added
documentation
4a20f46 to
7e1d2a9
Compare
53ae0f5 to
e9225ac
Compare
Greptile SummaryRetunes the Franka soft-body and cloth lift environments and adds deformable-aware MDP terms plus Newton full-surface contact configuration.
Confidence Score: 4/5The PR should not merge until the cloth gravity curriculum is changed to a compatible modifier, because its current configuration raises during environment reset. The cloth task routes the current gravity-distribution tuple into a function that treats it as an event name, causing event lookup to fail when the curriculum runs. Files Needing Attention: source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka_soft/franka_cloth_env_cfg.py; source/isaaclab_tasks/isaaclab_tasks/core/lift/mdp/curriculums.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Policy[Policy or scripted controller] --> Actions[Joint or IK action preset]
Actions --> Env[Franka deformable lift environment]
Env --> Sim[Newton or PhysX simulation]
Sim --> State[Robot and deformable state]
State --> MDP[COM observations, rewards, and terminations]
MDP --> Policy
Curriculum[Gravity and reward curricula] --> Env
Contact[Proxy full-surface rigid-soft contact] --> Sim
Reviews (1): Last reviewed commit: "Fix: Clean up lift mdp, unify the soft a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| gravity = CurrTerm( | ||
| func=mdp.modify_term_cfg, | ||
| params={ | ||
| "address": "events.variable_gravity.params.gravity_distribution_params", | ||
| "modify_fn": mdp.gravity_range_linear, | ||
| "modify_params": { | ||
| "start_gravity_z": -1.0, | ||
| "end_gravity_z": -9.81, | ||
| "start_step": 0, | ||
| "end_step": 20000, | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
When the cloth curriculum runs, modify_term_cfg passes the current gravity tuple as the third argument to gravity_range_linear, which treats it as an event name and raises ValueError, preventing the environment from resetting.
| gravity = CurrTerm( | |
| func=mdp.modify_term_cfg, | |
| params={ | |
| "address": "events.variable_gravity.params.gravity_distribution_params", | |
| "modify_fn": mdp.gravity_range_linear, | |
| "modify_params": { | |
| "start_gravity_z": -1.0, | |
| "end_gravity_z": -9.81, | |
| "start_step": 0, | |
| "end_step": 20000, | |
| }, | |
| }, | |
| ) | |
| gravity = CurrTerm( | |
| func=mdp.gravity_range_linear, | |
| params={ | |
| "event_name": "variable_gravity", | |
| "start_gravity_z": -1.0, | |
| "end_gravity_z": -9.81, | |
| "start_step": 0, | |
| "end_step": 20000, | |
| }, | |
| ) |
Knowledge Base Used: isaaclab_tasks: Task Registration and Organization
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The Franka deformable lift retune introduces coherent task-local MDP terms, action presets, and Newton/VBD configuration, but four issues need maintainer action: the cloth gravity curriculum is wired through an incompatible callback path, deformable mesh spawning now performs an unconditional repository-wide remesh, the resulting core Isaac Lab behavior change lacks a changelog fragment, and two exported termination APIs are removed without prior deprecation.
- Design and architecture: The
joint/ikaction presets and deformable-specific MDP terms are appropriately structured. However, placing unconditional deformable remeshing in the shared mesh spawner silently changes authored geometry, nodal-state sizes, and simulation cost for every backend and task usingdeformable_props. This should be opt-in or confined to the task geometry. - API: The new command/config exports and default-inert Newton/VBD fields are aligned and documented. The removal of exported
deformable_outside_table_boundsanddeformable_com_below_minimumviolates the repository's deprecation-first rule; compatibility wrappers and migration documentation are required. The user-visiblesource/isaaclabspawner change also requires its own package changelog fragment. - Implementation: The cloth curriculum passes
gravity_range_linearthroughmodify_term_cfg, although that helper expects anevent_nameand updates the event configuration itself. The configured callback therefore cannot execute with the supplied parameters and should use the direct curriculum form already used by the soft-beam environment. The unconditional remesh in the shared spawner should also be gated or moved into task-specific geometry authoring.
Significant concerns. Posted 4 actionable findings inline.
Automated review; human maintainers own approval decisions.
|
|
||
| # Since we use 24 steps per env, 20000 steps correspond to 20000/24 = 833.33 learning iterations | ||
| gravity = CurrTerm( | ||
| func=mdp.modify_term_cfg, |
There was a problem hiding this comment.
🔴 Critical · Implementation — Cloth gravity curriculum cannot call helper
gravity_range_linear requires event_name and already rewrites the event term itself, returning a log dict. Routed through modify_term_cfg, modify_params supplies no event_name, so it can only bind positionally to the addressed value, and any return would be written into gravity_distribution_params. Use the direct form the soft env uses: CurrTerm(func=mdp.gravity_range_linear, params={"event_name": "variable_gravity", ...}).
| @@ -420,6 +420,12 @@ def _spawn_mesh_geom_from_mesh( | |||
| if not is_rigid_material: | |||
| raise ValueError("Rigid properties require a rigid physics material.") | |||
|
|
|||
| # refine the surface for deformable primitives | |||
There was a problem hiding this comment.
🟡 Warning · Implementation — Missing isaaclab changelog fragment
This PR modifies source/isaaclab, but fragments were added only for isaaclab_contrib, isaaclab_newton, isaaclab_ovphysx (skip), and isaaclab_tasks. Repository rules require one fragment per touched package, and this deformable-spawn behavior change is user-visible. Add source/isaaclab/changelog.d/<slug>.rst with a Changed entry.
| @@ -420,6 +420,12 @@ def _spawn_mesh_geom_from_mesh( | |||
| if not is_rigid_material: | |||
| raise ValueError("Rigid properties require a rigid physics material.") | |||
|
|
|||
| # refine the surface for deformable primitives | |||
| if cfg.deformable_props is not None: | |||
| max_edge = 0.3 * float(np.linalg.norm(mesh.bounding_box.extents)) | |||
There was a problem hiding this comment.
🟡 Warning · Design Architecture — Global remesh of all deformable mesh spawns
Every mesh spawned with deformable_props is now subdivided to 0.3 * bbox_diagonal, for all backends and all existing assets, with no config flag and no docstring mention. This silently changes vertex/tet counts, nodal-state shapes for node-based observations, and simulation cost outside this task, and can override explicitly authored tessellation. Gate it behind an opt-in mesh-cfg field or author the finer geometry in the task spawn cfg.
|
|
||
|
|
||
| def deformable_outside_table_bounds( | ||
| def deformable_outside_bounds( |
There was a problem hiding this comment.
🟡 Warning · Api — Termination terms renamed without deprecation
deformable_outside_table_bounds becomes deformable_outside_bounds with a newly required z_bounds, and deformable_com_below_minimum is deleted; both were exported from isaaclab_tasks.core.lift.mdp. Repository rules require deprecating public symbols before removal, and the changelog Removed section mentions only the ovphysx preset. Keep thin deprecated wrappers for one release and document the migration.
240884f to
7180920
Compare
AntoineRichard
left a comment
AntoineRichard
left a comment
There was a problem hiding this comment.
Couple small things!
There was a problem hiding this comment.
Removing the video option? Could be nice to keep it for debug.
There was a problem hiding this comment.
(AI oriented debugging)
There was a problem hiding this comment.
Given that the rest of the state machines don't have the camera option, I'm not sure it belonged there. What do you think, keep it?
There was a problem hiding this comment.
I thought keeping the state machine demo as short as possible is higher priority than the camera recording
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@10402ecbaf2d8afda00507123155042cbcb5c3fb | ||
| newton-usd-schemas>=0.4.0 | ||
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@fd8d9d4e184b1298f989be13b0fbe3b59c5bbd58 | ||
| newton-usd-schemas>=0.4.1 |
There was a problem hiding this comment.
This is all in newton 1.5 ? We can't port changes that are not in 1.5.
There was a problem hiding this comment.
ah yes this should be based on the newton pin #6911, will update this to match once that PR is in!
| def test_spawn_cuboid_with_edge_refinement(sim): | ||
| """Test cuboid surface edge refinement.""" | ||
| size = (1.0, 2.0, 3.0) | ||
| edge_refinement = 3.0 | ||
| cfg = sim_utils.MeshCuboidCfg(size=size, edge_refinement=edge_refinement) | ||
| cfg.func("/World/RefinedCube", cfg) | ||
|
|
||
| prim = sim.stage.GetPrimAtPath("/World/RefinedCube/geometry/mesh") | ||
| points = np.asarray(prim.GetAttribute("points").Get()) | ||
| faces = np.asarray(prim.GetAttribute("faceVertexIndices").Get()).reshape(-1, 3) | ||
| edges = points[faces[:, [0, 1, 1, 2, 2, 0]]].reshape(-1, 2, 3) | ||
|
|
||
| assert len(points) > 8 | ||
| assert len(faces) > 12 | ||
| assert np.linalg.norm(edges[:, 0] - edges[:, 1], axis=1).max() <= np.linalg.norm(size) / edge_refinement |
There was a problem hiding this comment.
Claude would tell you your test is not testing the guards. It's likely fine though :)
There was a problem hiding this comment.
Shouldn't that be on devleop already?
There was a problem hiding this comment.
this was a newton pin #6911 right, so if we have that in develop now, then yes should be fine! but I just checked develop and I don't think this is in develop yet?
| spawn=sim_utils.MeshRectangleCfg( | ||
| size=(0.2, 0.2), | ||
| resolution=(30, 30), | ||
| resolution=(8, 8), |
There was a problem hiding this comment.
it's alright, if our target is speed, this will be fine for simulating a soft cloth, if we need more complex geometry than a square it might make sense to change this resolution a bit.
There was a problem hiding this comment.
also consider that we have watertight contact now, so no chance of penetration between node points, which was an issue until now
3f167ae to
c0514a3
Compare
The Franka deformable lift tasks could not grasp reliably. The gripper tunneled through the beam between soft vertices, and the shared lift MDP terms offered no deformable-aware rewards or divergence guards, so diverged environments kept poisoning the rollout. Add a task-local mdp package with deformable-aware rewards, observations, terminations, events, a gravity curriculum, and a pose command that tracks the deformable center of mass. Retune the beam material, the collider contact and rest offsets, and the arm and hand actuator gains so the gripper no longer crushes the deformable. Enable full-surface rigid-soft contact backed by signed-distance fields on the gripper so contacts are caught between vertices. Give the cloth task a kinematic rigid support that is registered with the coupler so the cloth no longer passes through it. Select the action space through a preset, with joint-space targets as the default and task-space inverse kinematics available through presets=ik. Support the above in Newton with NewtonShapeSDFCfg to provision volume SDFs on collider shapes selected by label regex, a collision pipeline flag for full-surface rigid-soft contact, and a configurable per-body particle contact buffer so contacts are not silently dropped. Full-surface contact on the coupled proxy solver additionally requires Newton with proxy-body contact harvesting, tracked in newton-physics/newton#3756.
Use Newton's canonical joint target attributes for articulation bindings and actuator forwarding. Keep the legacy PhysX wrapper fields available for compatibility.
Delay USD and trimesh imports until geometry helpers run so Lift configurations remain backend-free during discovery. Refresh the Newton cloth rendering baselines for the redesigned scene.
The retuned task changes the initial Franka pose rendered by the kitless correctness tests. Refresh the Newton Warp and OVRTX baselines from the failed CI run so they match the intended configuration.
099c17c to
a6a8d1c
Compare
There was a problem hiding this comment.
@mmichelis Sorry Mike, but I see two issues with this MR regarding the camera golden data:
-
If you update the env config, you would be expected to update the golden images for all backend combinations. In this PR, you only update the goldens for the newton sim backend.
-
The table is now missing in the newton warp and ovrtx renderers. I know @pbarejko / @StafaH / @daniela-hase had put some serious efforts to make sure the table can be correctly shown up just recently. The missing of the table, this time, is likely caused by the use of TABLE_SPAWN_CFG in which the table is set hidden.
Could you fix them please?
# Description Adds `isaacsim_physx` support to the Franka cloth lift tasks as a follow-up to #6831, while retaining `newton_mjwarp_vbd_proxy` as the default physics preset. This PR: - Adds PhysX physics, deformable material, collision, and scene presets for the standard and camera cloth tasks. - Disables scene replication for PhysX deformables and increases the GPU found/lost pair capacity for large environments. - Matches backend-specific cloth density and material conventions, lowers support friction, adjusts reset clearance, and fully closes the gripper on the thin cloth. - Updates the state-machine demo to use the task-specific IK preset, allow enough time for the low-PD Franka, and grasp the raised cloth fold. - Updates the environment catalog and generated browser data to advertise PhysX support. - Restores the already released removal of `NewtonCfg.simplify_meshes`, which was reintroduced during branch integration, and records the Newton-only cleanup with a skip fragment. No new dependencies are required. ### Validation - [x] `uv run isaaclab -f` - [x] `uv run python tools/changelog/cli.py check develop` in a clean clone - [x] `uv run --isolated --extra test -- make -C docs current-docs` in a clean clone - [x] `PYTHONPATH="$PWD/source/isaaclab_tasks:$PYTHONPATH" uv run python scripts/environments/state_machine/lift_franka_soft.py --task Isaac-Lift-Cloth-Franka --num_envs 1 --num_steps 500 --viz none presets=isaacsim_physx` ## Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Documentation update ## Screenshots Not applicable. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the pre-commit checks with `uv run isaaclab -f` - [x] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
4 participants
Description
Retunes the Franka soft-beam and cloth lift environments for gravity-based training.
This PR:
jointandikaction presets, with relative joint control as the default.--num_steps.rsl_rlexperiment.Prerequisite: This PR depends on #6911, which updates the pinned Newton revision and migrates articulation target bindings to the Newton 1.5 control API required by the Newton proxy presets. Merge #6911 first, then rebase this PR onto
develop.Breaking changes
presets=ikto retain inverse-kinematics control.isaacsim_physxfor the soft-beam task or the Newton proxy preset after its dependency lands.rsl_rlexperiment name changed fromfranka_deformabletofranka_soft. Update log and checkpoint paths.deformable_lifting(addsstd),DeformableComGoalDistance(addssuccess_threshold), anddeformable_outside_bounds(addsz_bounds); replace COM-height termination with workspace bounds.--num_stepsto control the finite demo and an external capture workflow to record it.Type of change
Screenshots
Not applicable.
Checklist
uv run isaaclab -fsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there