Newton gear insertion - #6833
Conversation
Add Newton point-SDF and hydroelastic presets, local concave collision assets, and backend-specific reset, reward, and gripper configuration for Rizon 4s gear assembly. Keep PhysX behavior unchanged, document Newton training, and add configuration regression coverage plus a flange IK variant.
Raise the per-rank triangle-pair capacity above the measured reset peak so 256-environment shards do not discard candidate contacts. Document the sizing assumption and cover it in the task configuration test.
Greptile SummaryAdds Newton support for the Rizon 4s gear-assembly task.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking defects identified. The backend presets preserve existing PhysX configuration while coherently selecting Newton-specific assets, timing, reset behavior, contact handling, and action variants, with regression tests covering the principal configuration contracts. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
CLI[Training task and preset] --> Registry[Gym task registration]
Registry --> Config[Rizon 4s environment config]
Config --> Backend{Resolved physics preset}
Backend -->|default or physx| PhysX[PhysX assets and behavior]
Backend -->|newton_mjwarp| MJWarp[Newton with MuJoCo contacts]
Backend -->|newton_sdf| SDF[Newton point-SDF contacts]
Backend -->|newton_hydroelastic| Hydro[Newton hydroelastic-SDF contacts]
SDF --> LocalAssets[Package-local concave gear assets]
Hydro --> LocalAssets
Config --> Reset[Backend-specific reset and shaft frames]
Config --> Grasp[Fingertip-centered grasp, reward, and drop frames]
Config --> Actions{Action variant}
Actions --> Joint[7-D relative joint actions]
Actions --> IK[6-D Newton flange IK actions]
Reviews (1): Last reviewed commit: "Add Curie Park to contributors" | Re-trigger Greptile |
Use one centered collision mesh set for PhysX and Newton while preserving the legacy PhysX asset frames for existing checkpoints. Select Newton hydroelastic behavior at spawn time and expose an explicit PhysX SDF validation preset.
There was a problem hiding this comment.
DOes this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
There was a problem hiding this comment.
Does this USDA need to live within IsaacLab? Could it be on Nucleus?
AntoineRichard
left a comment
There was a problem hiding this comment.
AI-generated review.
Requesting changes before this showroom task is merged. I confirmed two incompatible defaults that the current tests miss: the Newton-IK task resolves to PhysX and still passes validation, and Newton collision capacity is sized/documented for 256 environments while the task defaults to 4,096. The every-step gear-pinning path also performs avoidable CUDA allocations and host synchronizations.
Please also consolidate the duplicated reset/placement logic, keep comments and docs functional rather than recording tuning history, and replace brittle configuration-literal assertions with behavioral regressions for frame composition, selected-vs-unselected gear handling, and fingertip-midpoint behavior.
Verification performed on commit 1ea7a1e:
- all pre-commit hooks passed;
test_deploy_gear_assembly_cfg.py: 6 passed;- focused config check confirmed
PhysxCfg + NewtonInverseKinematicsActionCfgpasses validation; - focused config check confirmed Newton resolves to 4,096 environments with
max_triangle_pairs=4,194,304.
I did not run GPU simulation or training.
|
|
||
| # A 256-world GPU shard reached 1.54M broad-phase pairs during randomized resets. | ||
| # Keep power-of-two headroom so reset spikes do not discard candidate contacts. | ||
| _GEAR_MAX_TRIANGLE_PAIRS = 4_194_304 |
There was a problem hiding this comment.
AI-generated review — Important: this capacity is justified from a 256-environment shard, but the resolved Newton task still defaults to scene.num_envs == 4096 (confirmed locally). Running the task with only presets=newton_sdf or presets=newton_hydroelastic therefore violates this assumption and can silently discard candidate contacts. Set a Newton-specific environment default, size/derive the capacity for the actual default, or validate the supported shard size. The code comment should state the functional limit and overflow behavior; keep the 1.54M measurement and power-of-two selection history in benchmark/PR notes.
| solver_cfg=MJWarpSolverCfg( | ||
| solver="newton", | ||
| integrator="implicitfast", | ||
| # The hydroelastic SDF pipeline produces distributed contact areas (thousands of points |
There was a problem hiding this comment.
AI-generated review — Moderate: this comment narrates the tuning exercise and compares presets rather than documenting a durable invariant. Please reduce it to the functional fact (for example, hydroelastic contacts require larger per-world contact/constraint buffers); the measured contact count and why 4096 was selected belong in benchmark evidence, not production comments.
|
|
||
| # Grav gripper actuator configuration for gear manipulation | ||
| self.scene.robot.actuators["gripper_drive"] = ImplicitActuatorCfg( | ||
| # Use the validated bare-arm gains for Newton while preserving the existing PhysX gains. |
There was a problem hiding this comment.
AI-generated review — Moderate: “validated bare-arm gains” is unverifiable here and does not explain code behavior. Remove the claim or make the comment functional (for example, “Use backend-specific arm actuator gains”); validation evidence and tuning history should live in tests/benchmarks or the PR.
| __all__ = [ | ||
| "randomize_gear_type", | ||
| "randomize_gears_and_base_pose", | ||
| "pin_unselected_gears_to_shafts", |
There was a problem hiding this comment.
AI-generated review — Moderate: this helper is only consumed by the Rizon task configuration, but exporting it from the public mdp stub creates a compatibility commitment—removing it later would require deprecation. Keep this task-specific workaround internal and import it from .events unless external users genuinely need a supported public API.
|
|
||
|
|
||
| @configclass | ||
| class Rizon4sGearAssemblyIKNewtonEnvCfg(joint_pos_env_cfg.Rizon4sGearAssemblyEnvCfg): |
There was a problem hiding this comment.
AI-generated review — Important: this class inherits the PhysX default. I confirmed that resolving the registered Newton-IK task with default produces PhysxCfg + NewtonInverseKinematicsActionCfg, and cfg.validate() still passes, despite the docstring saying the combination is unsupported. Make the task select Newton by default or add a validate_config() backend/action compatibility check, and add a negative regression test for the unsupported combination.
|
|
||
| for gear_idx, asset_name in enumerate(gear_asset_names): | ||
| mask = selected_gear_indices != gear_idx | ||
| if not torch.any(mask): |
There was a problem hiding this comment.
AI-generated review — Important: converting torch.any(mask) to a Python boolean forces a CUDA-to-host synchronization for each gear on every control step. Avoid data-dependent Python branching here; masked indexed writes already handle empty masks, or precompute/cache the environment groups.
| assert hydro_cfg.scene.factory_gear_small.spawn.usd_path.endswith("factory_gear_small_hydroelastic.usda") | ||
|
|
||
|
|
||
| def test_rizon_gear_uses_shaft_targets_relative_actions_and_physical_gripper(): |
There was a problem hiding this comment.
AI-generated review — Important: these new tests assert configuration wiring and selected literals, but none executes the behaviors this PR says it fixes. Add regressions that fail without the fix: rotate/translate the base and verify reset shaft poses through frame composition; verify the pin term writes only non-selected gears; and verify reward/drop distance uses the fingertip midpoint. Remove literal snapshots that do not protect one of those contracts.
| sdf_prims = [prim for prim in stage.Traverse() if prim.HasAttribute("newton:sdfMaxResolution")] | ||
|
|
||
| assert len(sdf_prims) == 1 | ||
| assert sdf_prims[0].GetAttribute("newton:sdfMaxResolution").Get() == 128 |
There was a problem hiding this comment.
AI-generated review — Moderate: pinning sdfMaxResolution == 128 freezes a tuning choice, and the test still cannot prove the docstring’s concavity claim—any mesh with these attributes passes. Either make this a metadata test and assert only the required schema/hydroelastic switch, or add a geometric assertion that actually protects the bore/concavity contract.
|
|
||
| .. code-block:: bash | ||
|
|
||
| uv run isaaclab train --rl_library rsl_rl \ |
There was a problem hiding this comment.
AI-generated review — Important: this is under “Full-Scale Training with Video Recording,” but the Newton command does not pass --video, --video_length, or --video_interval; the later command breakdown nevertheless describes Newton video length. Add a supported Newton recording command or separate this into a headless-training section and remove the video claims.
| --visualizer none \ | ||
| presets=newton_hydroelastic | ||
|
|
||
| The Newton hydroelastic preset uses package-local concave SDF collision assets. Its defaults |
There was a problem hiding this comment.
AI-generated review — Moderate: this paragraph duplicates implementation details already encoded in the config/tests (dt, substeps, decimation, gap, gravity compensation, actuator behavior) and will drift as tuning changes. Keep the user-actionable distinctions—how to choose the preset and what to do on overflow—and remove the internal configuration narrative and exact-value rationale.
Description
This PR adds Newton physics support to the Flexiv Rizon 4s gear-assembly
task while preserving the existing PhysX behavior.
gear-base shafts instead of approximating them with convex hulls.
composition for randomized gear-base poses.
between the fingertips as the physical grasp frame.
solver-native arm gravity compensation without rewriting the selected
gear pose.
fragment.
Motivation and Context
The PhysX-authored task relied on collision and asset conventions that did
not transfer directly to Newton. In particular, convex approximations closed
concave gear geometry, reset and reward targets used inconsistent frames, and
the gripper linkage required explicit passive-joint behavior.
These changes provide a physical Newton training path for contact-rich gear
insertion without a pose-rewrite stabilization action. PhysX defaults remain
unchanged.
Fixes: N/A (no tracking issue was provided).
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there