Add proxy-coupled MJWarp+VBD solver and Franka cable lifting task#5657
Add proxy-coupled MJWarp+VBD solver and Franka cable lifting task#5657mmichelis wants to merge 45 commits into
Conversation
There was a problem hiding this comment.
Code Review: PR #5657 - Add proxy-coupled MJWarp+VBD solver and Franka cable lifting task
Thanks for this comprehensive contribution adding proxy-coupled physics simulation! This is a significant addition to Isaac Lab's deformable simulation capabilities. I've reviewed the implementation and have the following observations:
✅ Strengths
- Clean architecture: The
NewtonProxyCoupledMJWarpVBDManagerproperly extendsNewtonVBDManagerand follows existing Isaac Lab patterns - Good error handling: Clear ValueError messages when body selection fails to match expected patterns
- Thorough documentation: Docstrings explain the physics coupling semantics well
- Comprehensive test coverage:
test_rigid_deformable_coupling.pycovers one-way and two-way coupling modes
🔍 Suggestions for Improvement
1. Silent no-op in sync_particles_to_usd early returns (Low severity)
In newton_manager.py, sync_particles_to_usd() returns silently when various conditions aren't met:
if cls._usdrt_stage is None or cls._state_0 is None or cls._state_0.particle_q is None:
return
if not cls._particles_dirty:
returnConsider adding debug logging to help diagnose viewport rendering issues when particles aren't syncing as expected.
2. Potential race condition with _cable_registry (Medium severity)
In cable_object.py, install_cable_builder_hooks() resets the registry:
SimulationManager._cable_registry = []If called concurrently (e.g., multiple environments initializing in parallel), this could clear entries from other CableObjects. Consider using thread-safe initialization or documenting the single-call requirement more prominently.
3. Hardcoded collision pipeline thresholds (Low severity)
In proxy_coupled_mjwarp_vbd_manager.py:
collision_pipeline=lambda destination_model: CollisionPipeline(
destination_model,
broad_phase="explicit",
contact_matching="sticky",
contact_matching_pos_threshold=0.005,
contact_matching_normal_dot_threshold=0.95,
)These magic numbers (0.005m, 0.95 dot product) could be exposed in ProxyCoupledMJWarpVBDSolverCfg for advanced tuning, similar to how proxy_mass_scale is already configurable.
4. Missing validation for empty partition lists (Low severity)
In _partition_model_by_entities(), if both mjwarp_bodies and vbd_bodies are empty lists (not None), all bodies will be "unclaimed" and raise an error. This edge case might benefit from a clearer error message distinguishing "empty lists provided" from "patterns matched nothing".
5. TODO comment indicates incomplete implementation (Note)
# TODO: Temporary solution: Overwrite visual mesh with tet mesh surface points...This temporary solution might affect visual fidelity. Worth tracking for future cleanup.
6. Consider adding integration test for the full lift task (Enhancement)
The test file covers coupling mechanics, but Isaac-Lift-Cable-Franka-v0 would benefit from a lightweight smoke test that runs a few steps to verify end-to-end task setup.
📋 Minor Items
- The 150-file changeset includes merge content from base branch - the core new functionality is well-isolated in
isaaclab_contrib/deformable/andisaaclab_contrib/cable/ - CI passes ✅
Overall, this is a well-structured PR that adds valuable physics coupling capabilities. The suggestions above are refinements rather than blockers.
Update (590fae0): Reviewed the incremental changes. This update includes:
✅ Positive Changes
- Unified naming convention: Renamed
deformable/cable→objectacross all env configs - cleaner and more consistent - Improved documentation: Better docstring for
rigid_avbd_betaexplaining the AVBD penalty-stiffness ramp semantics - Added visualizer configs: Consistent
KitVisualizerCfg/NewtonVisualizerCfgsetup with shared eye/lookat - Tuning refinements: Adjusted cable material properties, gripper stiffness, and solver parameters (appears to be ongoing tuning work)
🆕 New: tools/cable_tuning/ Module
Nice addition of an iterative-bisection hyperparameter tuning framework:
eval_run.py- Headless evaluation script with metrics capturetune.py- Driver with parameter schedule and resumable sweepsscoring.py- Tiered cost function (NaN >> state >> exploded >> settle >> tracking >> oscillation)- Good test coverage for
overrides.pyandscoring.py
Minor observations on tuning module:
- The
PARAM_SCHEDULEreferencesscene.cable.*paths but env config now usesscene.object.*- may need alignment - Consider adding a brief README in
tools/cable_tuning/explaining usage
📝 Other Notes
- Terminations temporarily commented out in
franka_cable_env_cfg.py- assume this is for tuning iteration; would need re-enabling before merge .gitignoreaddition for/tuning_results/is appropriate
The unified naming and tuning infrastructure are good improvements. No new blockers introduced.
Update (29d172d): Reviewed the latest push with 51 commits (many from develop merge). Key cable/deformable-specific changes:
✅ New Additions
-
Complete cable infrastructure:
CableObjectandCableObjectCfgclasses addedNewtonCableMaterialCfgfor cable rod material parameters (stretch/bend stiffness, damping, density)install_cable_builder_hooks()for Newton model builder integration- Cable bodies now properly excluded from FK evaluation with
_build_non_cable_articulation_mask()
-
Proxy-coupled MJWarp+VBD solver (
proxy_coupled_mjwarp_vbd_manager.py):- Full 303-line implementation of
NewtonProxyCoupledMJWarpVBDManager ProxyCoupledMJWarpVBDSolverCfgwith mjwarp_bodies, vbd_bodies, proxy_bodies, and proxy_mass_scale- Partition validation and proper error messages
- Full 303-line implementation of
-
Franka cable lifting task (
franka_cable_env_cfg.py):- Full 281-line env config for
Isaac-Lift-Cable-Franka-v0 - Uses
CoupledNewtonCfgwithProxyCoupledMJWarpVBDSolverCfg - IK-based actions with differential IK controller
- Lift-to-target reward structure
- Full 281-line env config for
-
Cable tuning framework (
tools/cable_tuning/):eval_run.py- 385 lines, headless eval with PickAndLiftSm state machinetune.py- 185 lines, iterative bisection parameter sweeperscoring.py- Tiered cost function- Tests for overrides and scoring
-
Comprehensive cable tests (
test_cable_object.py):- 446 lines of test coverage
- Tests for
add_cable_entry_to_builder, body offsets, FK preservation - Integration test
test_cable_replicate_body_countverifying multi-env cloning
🔍 Observations
1. eval_run.py inlines the state machine (Info)
The PickAndLiftSm is built inline inside _build_pick_and_lift_sm() rather than importing from scripts.environments.state_machine.lift_franka_soft due to argparse conflicts. This is fine but means two implementations exist that could drift.
2. PARAM_SCHEDULE path consistency (Low severity)
ParamSpec("scene.cable.spawn.physics_material.stretch_stiffness", [...])The env config uses scene.object but the tuning schedule references scene.cable. One of these needs updating for the override paths to resolve correctly.
3. gravity compensation pattern (Info)
self.robot.spawn.rigid_props.disable_gravity = True
self.robot.spawn.rigid_props = sim_utils.MujocoRigidBodyPropertiesCfg(gravcomp=1.0)Setting disable_gravity then immediately replacing rigid_props - the first line has no effect. Could simplify to just the second line.
4. Test imports isaaclab_newton directly (Info)
In test_cable_object.py, tests import from isaaclab_newton.physics import ... which creates a hard dependency. This is likely fine since cables are Newton-only, but worth noting for test isolation.
✅ Quality
- Well-structured code following existing patterns
- Good docstrings explaining physics semantics
- Proper error handling with informative messages
- Tests cover key functionality paths
📋 Summary
This is a substantial and well-implemented feature addition. The proxy-coupled solver, cable infrastructure, and tuning framework are all solid. The main actionable items are:
- Fix: Update
PARAM_SCHEDULEpaths intune.pyto matchscene.object.*naming - Cleanup: Remove redundant
disable_gravityline - Consider: Re-enable terminations in
franka_cable_env_cfg.pybefore final merge
The cable lifting task and tuning framework provide a complete workflow for developing and tuning cable manipulation policies. Nice work! 🚀
Update (d87e448): Reviewed the major v3 push (52 commits since last review). This is a significant milestone with full documentation, demos, and Newton ray-caster support.
🆕 Major Additions
1. Comprehensive Documentation (docs/source/experimental-features/newton-physics-integration/using-cables.rst)
Excellent 241-line guide covering:
- Quick-start demo commands
- Cable authoring with
CableCfgandNewtonCableMaterialCfg - Solver selection guidance (VBD, coupled solvers)
- Material parameter reference table (stretch/bend stiffness, damping, density)
- Spawner parameter reference
- Kit/Fabric visualization notes
- Limitations section (Newton-only, no actuators, no FK case for cable joints)
This is production-ready documentation that will help users adopt cables.
2. Cable Demo (scripts/demos/cables.py)
194-line standalone demo spawning 25+ cables that collide and settle:
- Uses
CableObject,CableObjectCfg,NewtonCableMaterialCfg - Demonstrates VBD solver with configurable iterations
- Soft body-body contact tuning for pile stability
spawn_cfg = sim_utils.UsdFileCfg(
usd_path="/home/mmichelis/Documents/IsaacLab-Origin/scripts/demos/cable001.usda",
...
)This appears twice (for cable001.usda and cable002.usda). These either need to:
- Use relative paths or ISAAC_LAB_ASSETS paths
- Be removed if they're dev-only test cables
- Have the referenced USD files included in the repo
3. Newton Ray-Caster Sensors (isaaclab_newton/sensors/ray_caster/)
312-line ray_caster.py plus supporting modules:
_NewtonRayCasterMixinfor site registration and pose trackingNewtonRayCasterclass extendingBaseRayCaster- Proper site injection into prototype builders before cloning
multi_mesh_ray_caster.pyandmulti_mesh_ray_caster_camera.pystubs
This enables proper ray-casting in Newton scenes - important for obstacle detection sensors.
4. Cable Spawn Test (test_spawn_cable.py)
111-line test verifying cable spawner writes valid UsdGeomBasisCurves.
5. Changelog Updates
Proper changelog entries in:
isaaclab/changelog.d/mym-cable.minor.rstisaaclab_contrib/changelog.d/mym-cable.minor.rstisaaclab_newton/changelog.d/mym-cable.minor.rst
🔍 Issues to Address
1. Hardcoded Local Paths in Demo (🔴 High severity - blocks merge)
usd_path="/home/mmichelis/Documents/IsaacLab-Origin/scripts/demos/cable001.usda"
usd_path="/home/mmichelis/Documents/IsaacLab-Origin/scripts/demos/cable002.usda"Must be fixed or removed before merge.
2. PARAM_SCHEDULE still references scene.cable.* (Low severity)
The tuning schedule paths haven't been updated to match scene.object.* from the env config.
✅ Quality Improvements
- Documentation follows Isaac Lab style guide
- API reference RST files added (
isaaclab_contrib.cable.rst) - Newton spawners docs updated
- Index updated to link cable documentation
📋 Summary
This push brings the cable feature to release-ready state with excellent documentation and a working demo. The only blocking issue is the hardcoded local file paths in scripts/demos/cables.py - those will cause immediate failures for anyone trying the demo.
Recommended action: Remove or fix the two UsdFileCfg blocks referencing /home/mmichelis/... paths before merging.
Everything else looks great - the documentation quality is particularly impressive. 🎉
Update (49215b5): Reviewed v4 push (21 commits). This update focuses on env reset robustness, solver refactoring, and unified object naming.
🔴 Critical: Indentation Error
vbd_manager.py line 331 has a syntax error:
def forward(cls) -> None:
...
if cls._non_cable_articulation_mask is None: # ← 7 spaces, should be 8
if cls._cable_registry:The if statement has 7 spaces instead of 8, which will cause a Python IndentationError when the module is imported. This will break the entire VBD solver.
🆕 Key Changes in v4
1. Improved FK Reset Handling (newton_manager.py)
Removed the _needs_collision_pipeline gate from eval_fk in step():
# Now always runs eval_fk after resets, not just for solvers with collision pipelines
eval_fk(cls._model, cls._state_0.joint_q, cls._state_0.joint_qd, cls._state_0, cls._fk_reset_mask)Good fix - ensures body_q is fresh for all downstream consumers (proxy sync, observations, IK, visualizers), not just collision-pipeline solvers.
2. Refactored _filter_solver_kwargs (newton_manager.py)
Centralized the solver-kwargs filtering logic that was previously duplicated:
@staticmethod
def _filter_solver_kwargs(solver_cls: type, solver_cfg) -> dict:
"""Return cfg fields that match solver_cls.__init__ parameters."""Clean DRY improvement across featherstone_manager.py, mjwarp_manager.py, xpbd_manager.py, vbd_manager.py, and coupled managers.
3. Unified object Naming
Renamed deformable → object consistently across:
- Scene config:
scene.deformable→scene.object - Commands:
deformable_pose→object_pose - Rewards:
deformable_*→object_*(e.g.,object_lifted,object_com_goal_distance) - Observations:
DeformableSampledPointsInRobotRootFrame→ObjectSampledPointsInRobotRootFrame
This is a good consistency improvement making the API work uniformly for volumetric deformables and cables.
4. Rewards/Observations Now Support Cables
Added _points_w() and _com_w() dispatch functions in rewards.py:
def _points_w(asset: DeformableObject | Articulation) -> torch.Tensor:
if hasattr(asset.data, "nodal_pos_w"):
return wp.to_torch(asset.data.nodal_pos_w)
return asset.data.body_pos_w.torchEnables reward/observation code to work with both deformable objects (nodal particles) and cable articulations (body segments).
5. Lazy Cable Mask Build in forward()
Changed vbd_manager.py to build the non-cable articulation mask lazily on first forward() call instead of in start_simulation():
if cls._non_cable_articulation_mask is None:
if cls._cable_registry:
cls._build_non_cable_articulation_mask()This is cleaner, but the indentation error breaks it.
6. Cable Label Expansion
cable_object.py now expands env_.* in labels immediately:
expanded_prim_path = entry.prim_path.replace("env_.*", f"env_{env_idx}")
builder.add_rod_graph(..., label=f"{expanded_prim_path}/cable", ...)Fixes label matching for USD-imported body naming conventions.
7. State Machine Simplifications (lift_franka_soft.py)
- Removed
OPEN_GRIPPERstate (lift holds gripper closed indefinitely) - Faster approach/grasp timings (0.5s, 0.75s vs 1.0s, 1.0s)
- Added support for
Isaac-Lift-Cable-Franka-v0task - Adjusted grasp offset to
[0.0, 0.0, -0.002]
8. Cable Tuning Framework (tools/cable_tuning/)
New additions from earlier commits now fully landed:
eval_run.py,tune.py,scoring.pywith test coverage- Iterative bisection hyperparameter search
- Tiered cost function for evaluation
📋 Summary
| Status | Item |
|---|---|
| 🔴 BLOCKING | vbd_manager.py:331 indentation error - fix required before merge |
| 🔴 Still Open | Hardcoded local paths in scripts/demos/cables.py from v3 |
PARAM_SCHEDULE in tune.py references scene.cable.* but env uses scene.object.* |
|
| ✅ Good | FK reset handling improvement |
| ✅ Good | Centralized _filter_solver_kwargs |
| ✅ Good | Unified object naming across configs |
| ✅ Good | Reward/observation code now supports both deformables and cables |
Required before merge:
- Fix indentation at
vbd_manager.py:331(7 spaces → 8 spaces) - Fix or remove hardcoded paths in
scripts/demos/cables.py
Once those are fixed, this is ready to go. The proxy-coupled solver, cable infrastructure, and unified naming are solid additions. 🚀
Update (470f2ac): Reviewed v5 push (26 files changed). This update addresses the blocking issues from v4.
✅ Fixed Issues
1. Indentation Error Fixed ✅
The vbd_manager.py:331 indentation error has been fixed:
- if cls._non_cable_articulation_mask is None:
+ if cls._non_cable_articulation_mask is None:The 7-space indent is now correct 8-space indent. The VBD solver will import correctly.
2. Code Cleanup: Removed unused import
Removed import inspect from vbd_manager.py since the centralized _filter_solver_kwargs is now used instead.
🔴 Remaining Issues
1. Hardcoded Local Paths Still Present (🔴 Blocks merge)
scripts/demos/cables.py still contains:
usd_path="/home/mmichelis/Documents/IsaacLab-Origin/scripts/demos/cable001.usda" # Line 102
usd_path="/home/mmichelis/Documents/IsaacLab-Origin/scripts/demos/cable002.usda" # Line 119These paths will cause FileNotFoundError for all users.
2. PARAM_SCHEDULE Path Mismatch (
tools/cable_tuning/tune.py still references scene.cable.*:
ParamSpec("scene.cable.spawn.physics_material.stretch_stiffness", [...])
ParamSpec("scene.cable.spawn.physics_material.stretch_damping", [...])
ParamSpec("scene.cable.spawn.physics_material.bend_stiffness", [...])
ParamSpec("scene.cable.spawn.physics_material.bend_damping", [...])
ParamSpec("scene.cable.spawn.physics_material.density", [...])But franka_cable_env_cfg.py uses scene.object. The override paths won't resolve unless updated to scene.object.*.
🆕 New in v5
1. New proxy_coupled_mjwarp_vbd_manager.py (305 lines)
Full proxy-coupled solver implementation now landed separately from the previous coupled managers.
2. Cable Env Config Complete
franka_cable_env_cfg.py added with:
CableObjectCfgfor the cable assetProxyCoupledMJWarpVBDSolverCfgconfiguration- IK-based action space
- Object-centric rewards
📋 Summary
| Status | Item |
|---|---|
| ✅ FIXED | vbd_manager.py:331 indentation error |
| 🔴 Still blocking | Hardcoded paths in scripts/demos/cables.py |
PARAM_SCHEDULE paths need scene.cable → scene.object update |
One blocking issue remains: Remove or fix the hardcoded /home/mmichelis/... paths in cables.py before merging.
The indentation fix was the critical one - the solver is now functional. Nice work addressing that quickly! 🎉
f1d527c to
470f2ac
Compare
…BasisCurve in USD. Currently only supported in Newton backend as add_rod_graph
eval_run.py runs the Isaac-Lift-Cable-Franka-v0 state machine for one cycle under user-supplied dotted-path overrides, captures per-step state to metrics.parquet, and writes an aggregated summary.json suitable for scoring. Failures (NaN tensors, env.step exceptions, or unexpected exceptions) cleanly exit with nan_flag=1 in the summary so the driver can score them.
eval_run: cfg now exposes 'object_pose' (not 'cable_pose'); the demo script in scripts/environments/state_machine/lift_franka_soft.py still references cable_pose, so the inline state-machine driver in eval_run uses the actual cfg name. tune.py: walks an 11-parameter schedule (solver + cable material), sweeps 3-4 trial values per parameter in fresh eval_run subprocesses, keeps the lowest-cost value, appends to tuning_log.jsonl, and writes final_best.json when done.
470f2ac to
1b36738
Compare
Description
Add a proxy-coupled MJWarp + VBD solver (wrapping Newton's
SolverProxyCoupled) and aIsaac-Lift-Cable-Franka-v0task where the Franka picks up a Newton cable. The arm lives in the MJWarp entry, the cable particles in the VBD entry, and the gripper fingers are exposed as virtual proxies so VBD detects them as contacts on the cable and returns feedback wrenches to MuJoCo via lagged impulses.Builds on top of PR #5641. Requires newton branch: gdaviet/coupled-solver-framework
Changes
New: proxy-coupled MJWarp + VBD solver
NewtonProxyCoupledMJWarpVBDManager(isaaclab_contrib/deformable/proxy_coupled_mjwarp_vbd_manager.py): wrapsnewton.solvers.SolverProxyCoupledwith MuJoCo Warp + VBD entries. Bodies/joints/shapes are partitioned between the entries fromSceneEntityCfgselectors; joints inherit their child body's owner; static shapes (body == -1) always go to VBD. Proxy bodies are filtered to those owning at least oneCOLLIDE_SHAPES-flagged shape.ProxyCoupledMJWarpVBDSolverCfg: declarative cfg withmjwarp_bodies/vbd_bodies/proxy_bodies(alllist[SceneEntityCfg]), plusproxy_mode("lagged"/"staggered"),proxy_iterations,proxy_collide_interval,proxy_mass_scale. Body selection usesre.fullmatchagainst body short names, scoped by the asset'sprim_path.Newton cfg consolidation
CoupledNewtonCfgreplacesDeformableNewtonCfg. Adds ascene_cfgslot so coupled solvers can resolveSceneEntityCfgselectors at solver-build time; envs now doself.sim.physics = CoupledNewtonCfg(solver_cfg=..., scene_cfg=self.scene).solver_typestrings from VBD / coupled cfgs.Bug fixes
NewtonManager.step()now always runseval_fkafter env resets, arm + binary gripper actions; 10-segment cable; goal sampled in robot root frame; reach / lift / goal-tracking rewards.Test plan
./isaaclab.sh -p scripts/environments/state_machine/lift_franka_soft.py --task Isaac-Lift-Cable-Franka-v0— gripper picks up the cable and reaches the goal sphere.Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there