Skip to content

Add Newton physics preset for Anymal-D rough terrain#5096

Closed
hujc7 wants to merge 1 commit intoisaac-sim:developfrom
hujc7:jichuanh/newton-rough-terrain-preset
Closed

Add Newton physics preset for Anymal-D rough terrain#5096
hujc7 wants to merge 1 commit intoisaac-sim:developfrom
hujc7:jichuanh/newton-rough-terrain-preset

Conversation

@hujc7
Copy link
Copy Markdown

@hujc7 hujc7 commented Mar 24, 2026

Summary

Add RoughPhysicsCfg Newton preset enabling Anymal-D rough terrain locomotion training with the Newton physics backend. This is a small, self-contained change that makes the existing stable Isaac-Velocity-Rough-Anymal-D-v0 task runnable with presets=newton.

Changes:

  • Add RoughPhysicsCfg(PresetCfg) with Newton solver config tuned for mesh terrain
  • Disable RayCaster height scanner for Newton (requires Kit via omni.physics)
  • Fix zero contact margin from USD import that causes CCD divergence on mesh terrain
  • Increase CollisionPipeline max_triangle_pairs to 2M for mesh terrain
  • Bump mujoco/mujoco-warp to 3.6.0, pin Newton to 141baffff9

Upstream issues and workarounds

Pyramidal cone for solver stability (WAR)

MuJoCo Warp uses float32 (vs float64 in MuJoCo C). The constraint solver diverges to NaN with elliptic cone + high impratio on mesh terrain contacts.

Zero contact margin from USD import (WAR)

Newton USD importer produces 100% of shapes with zero margin (225,281/225,281 at 4096 envs). Zero margin causes the CCD narrow phase to use a degenerate path where GJK fails to converge. Flat terrain is unaffected (plane geometry doesn't trigger this path); mesh terrain NaNs.

  • TODO: file upstream Newton issue
  • Workaround: enforce minimum margin of 0.01 before ModelBuilder.finalize()

CollisionPipeline buffer overflow (WAR)

Default max_triangle_pairs (1M) overflows with mesh terrain at 4096 envs (~2M triangle pairs). Overflow silently drops contacts, causing robots to fall through terrain.

  • No upstream issue tracking auto-sizing
  • Workaround: increase max_triangle_pairs to 2M

Related PRs

Test plan

Tested on single GPU with env_isaaclab_develop conda env:

conda run -n env_isaaclab_develop python \
  scripts/reinforcement_learning/rsl_rl/train.py \
  --task Isaac-Velocity-Rough-Anymal-D-v0 \
  --num_envs 4096 --max_iterations 300 --headless presets=newton

Newton backend confirmed via log: Registered backend 'newton' for factory Articulation.

Metric Result Benchmark (Anymal-C rough)
Reward 13.33 ≥ 7
Episode length 957 ≥ 875
time_out % 89.7%
NaN 0
Training time 14 min
  • 300 iterations at 4096 envs with presets=newton, zero NaN
  • Multiple runs show consistent results (reward 11-13, episode length 940-970)
  • Pre-commit checks pass

Add RoughPhysicsCfg preset enabling rough terrain locomotion on Newton.
Key solver settings: pyramidal friction cone (elliptic diverges in
float32 with many mesh contacts), Newton collision pipeline, increased
buffer sizes for mesh terrain contacts.

- Add RoughPhysicsCfg(PresetCfg) with Newton solver config
- Disable RayCaster height scanner for Newton (requires Kit)
- Fix zero contact margin from USD import causing CCD divergence
- Increase CollisionPipeline max_triangle_pairs for mesh terrain
- Bump mujoco/mujoco-warp to 3.6.0, pin Newton to 141baffff9

Tested with: conda run -n env_isaaclab_develop python
  scripts/reinforcement_learning/rsl_rl/train.py
  --task Isaac-Velocity-Rough-Anymal-D-v0
  --num_envs 4096 --max_iterations 300 --headless presets=newton
Result: zero NaN, reward 13.33, episode length 957, 14 min.
@hujc7 hujc7 requested a review from Mayankm96 as a code owner March 24, 2026 02:09
@github-actions github-actions bot added the isaac-lab Related to Isaac Lab team label Mar 24, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 24, 2026

Greptile Summary

This PR adds a Newton physics preset (RoughPhysicsCfg) enabling Anymal-D rough-terrain locomotion training with the Newton/MuJoCo-Warp backend, alongside three global workarounds in newton_manager.py for known upstream issues (zero contact margin, CollisionPipeline buffer overflow, and float32 cone divergence).

Key changes:

  • RoughPhysicsCfg(PresetCfg) in rough_env_cfg.py with Newton solver tuned for mesh terrain: pyramidal cone, use_mujoco_contacts=False, ccd_iterations=100, impratio=1.0
  • Height scanner and corresponding height_scan observation term disabled for Newton (RayCaster requires Kit / omni.physics)
  • newton_manager.py: global zero-margin patch (hardcoded 0.01) applied to all shapes before ModelBuilder.finalize() — two previous review threads have flagged the global scope of this fix and the hardcoded max_triangle_pairs=2_000_000 as needing opt-in configurability via NewtonCfg; those remain open
  • setup.py: bumps mujoco / mujoco-warp to 3.6.0 and pins newton to an abbreviated git SHA — the version-format change for mujoco-warp (from 3.5.0.2 to 3.6.0) and the abbreviated SHA are worth confirming before merge

Confidence Score: 5/5

  • Safe to merge; the only new finding is a P2 verification note on the mujoco-warp version format, and the two global-scope workarounds from prior threads are acknowledged open items.
  • All remaining findings are P2 or lower. The two substantive global-scope concerns (hardcoded max_triangle_pairs and zero-margin patch) were raised in previous review threads and are known open items, not new regressions. The new comment is purely a verification suggestion (confirm mujoco-warp==3.6.0 exists on PyPI). The task-specific changes in rough_env_cfg.py are clean and consistent with existing patterns. PR is supported by successful 300-iteration training runs at 4096 envs with zero NaN.
  • source/isaaclab_newton/setup.py — verify mujoco-warp==3.6.0 (3-part) exists on PyPI and confirm the abbreviated newton SHA; source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py — prior open items on global scope of workarounds remain

Important Files Changed

Filename Overview
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py Adds global zero-margin contact fix (hardcoded 0.01) and doubles max_triangle_pairs to 2M for CollisionPipeline; both applied universally to all Newton simulations regardless of task type — previous review threads flagged these as needing configurability via NewtonCfg.
source/isaaclab_newton/setup.py Bumps mujoco and mujoco-warp to 3.6.0 and switches newton from a PyPI release (1.0.0) to a git commit pin (abbreviated 9-char SHA); version format of mujoco-warp changed from 4-part (3.5.0.2) to 3-part (3.6.0), worth verifying the package exists under that exact name.
source/isaaclab_tasks/isaaclab_tasks/manager_based/locomotion/velocity/config/anymal_d/rough_env_cfg.py Adds RoughPhysicsCfg preset with Newton solver tuned for mesh terrain (pyramidal cone, no MuJoCo contacts, CCD), wires it into AnymalDRoughEnvCfg via SimulationCfg, and disables the RayCaster height scanner (and corresponding observation term) for Newton via the preset() factory — pattern is consistent with the rest of the codebase.

Sequence Diagram

sequenceDiagram
    participant CLI as train.py CLI
    participant HydraPreset as Hydra/PresetCfg
    participant EnvCfg as AnymalDRoughEnvCfg
    participant NM as NewtonManager
    participant CP as CollisionPipeline

    CLI->>HydraPreset: presets=newton
    HydraPreset->>EnvCfg: resolve RoughPhysicsCfg → NewtonCfg<br/>(pyramidal, use_mujoco_contacts=False)
    HydraPreset->>EnvCfg: scene.height_scanner = None
    HydraPreset->>EnvCfg: observations.policy.height_scan = None

    EnvCfg->>NM: start_simulation()
    NM->>NM: WAR: fix zero margin<br/>(shape_margin[i]==0 → 0.01)
    NM->>NM: ModelBuilder.finalize()

    EnvCfg->>NM: initialize_solver() [use_mujoco_contacts=False]
    NM->>CP: CollisionPipeline(model, broad_phase="explicit",<br/>max_triangle_pairs=2_000_000)
    CP-->>NM: contacts()

    loop Each simulation step
        NM->>CP: generate contacts (mesh terrain)
        CP-->>NM: contact data
        NM->>NM: MJWarpSolver.step()<br/>(pyramidal cone, impratio=1.0)
    end
Loading

Reviews (2): Last reviewed commit: "Add Newton physics preset for Anymal-D r..." | Re-trigger Greptile

"mujoco-warp==3.6.0",
"PyOpenGL-accelerate==3.1.10",
"newton==1.0.0",
"newton @ git+https://github.com/newton-physics/newton.git@141baffff9",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Short git commit hash for newton dependency

The newton package is pinned to a 9-character abbreviated SHA (141baffff9). Abbreviated hashes can theoretically collide as the repository grows, and some pip / dependency-resolution paths handle short hashes less reliably than full 40-character SHAs. The full hash should be used here for reproducibility and robustness.

Suggested change
"newton @ git+https://github.com/newton-physics/newton.git@141baffff9",
"newton @ git+https://github.com/newton-physics/newton.git@141baffff9ccab09bc07e3c4bff3c7e785acba4a",

(Replace the long hash above with the actual full SHA; verify with git -C newton rev-parse 141baffff9.)

Comment on lines +418 to +420
cls._collision_pipeline = CollisionPipeline(
cls._model, broad_phase="explicit", max_triangle_pairs=2_000_000
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Hardcoded max_triangle_pairs affects all collision-pipeline simulations

max_triangle_pairs=2_000_000 is now applied globally to every simulation that uses the Newton CollisionPipeline (i.e. any solver with use_mujoco_contacts=False, including flat-terrain and manipulation tasks). The default was 1M; this doubles the memory allocation for all such scenarios, not just mesh-terrain ones at 4096 envs.

Consider driving this from NewtonCfg (e.g. max_triangle_pairs: int = 1_000_000) so callers can opt in to the larger budget only when needed (as RoughPhysicsCfg would), rather than silently doubling memory overhead for every pipeline user.

Comment on lines +296 to +304
contact_margin = 0.01
n_margin_fixed = 0
for i in range(len(cls._builder.shape_margin)):
if cls._builder.shape_margin[i] == 0.0:
cls._builder.shape_margin[i] = contact_margin
n_margin_fixed += 1
if n_margin_fixed > 0:
n_total = len(cls._builder.shape_margin)
logger.info(f"Set contact margin={contact_margin} on {n_margin_fixed}/{n_total} shapes with zero margin")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Global zero-margin fix applied to all Newton builds

The loop unconditionally patches every shape whose margin is exactly 0.0 across the entire model, including non-locomotion tasks (e.g. manipulation). For tasks where zero contact margin is intentional or where a different default is desirable, this silently changes behavior.

The comment in the removed line mentioned "manipulation examples" as the motivation for the old margin logic; applying 0.01 across the board may be too broad. At minimum, consider making the fallback value (and whether to apply it at all) configurable via NewtonCfg, similar to how max_triangle_pairs could be:

contact_margin = getattr(cfg, "default_contact_margin", 0.0)
if contact_margin > 0.0:
    for i in range(len(cls._builder.shape_margin)):
        if cls._builder.shape_margin[i] == 0.0:
            cls._builder.shape_margin[i] = contact_margin
            ...

This keeps the workaround available while letting tasks that don't need it (or prefer a different value) avoid the side-effect.

Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot bot left a comment

Choose a reason for hiding this comment

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

🤖 Isaac Lab Review Bot — PR #5096

Summary

This PR adds a RoughPhysicsCfg Newton preset enabling the existing Isaac-Velocity-Rough-Anymal-D-v0 task to run with presets=newton. It includes three well-documented workarounds for upstream issues (zero contact margin, pyramidal cone for float32 stability, collision pipeline buffer overflow), version bumps for mujoco/mujoco-warp, and a Newton pin to a specific commit.

Files changed: 3 (newton_manager.py, setup.py, rough_env_cfg.py)
CI: ✅ labeler pass


Design Assessment

Well-structured. The PR correctly follows the established preset pattern used by the flat terrain config (flat_env_cfg.py). Specifically:

  • RoughPhysicsCfg(PresetCfg) mirrors the existing PhysicsCfg(PresetCfg) in flat_env_cfg with default/newton/physx attributes — consistent convention.
  • The preset(default=..., newton=None) usage for disabling the height scanner on Newton matches how other backend-conditional features are handled.
  • The newton_manager.py changes are generic workarounds (zero margin fix, buffer sizing) that benefit all Newton users, not just this task — good placement in the shared manager.
  • The solver parameters (cone="pyramidal", impratio=1.0, use_mujoco_contacts=False, integrator="implicitfast") are each justified by the upstream issues documented in the PR description.
  • Comprehensive test results: zero NaN, strong reward convergence, multiple runs.

Findings

1. Hard-coded max_triangle_pairs doesn't scale with environment count (Medium)

File: newton_manager.py, _initialize_contacts

The collision pipeline is created with max_triangle_pairs=2_000_000, which is sized for ~4096 envs. However, _initialize_contacts is a shared path for all Newton users. If someone runs with 8192+ envs (or denser terrain meshes), they'll hit the same silent overflow bug.

Consider computing this from cls._num_envs or the model's triangle count:

# Example: scale based on envs, with headroom
tri_pairs = max(2_000_000, cls._num_envs * 500) if cls._num_envs else 2_000_000
cls._collision_pipeline = CollisionPipeline(
    cls._model, broad_phase="explicit", max_triangle_pairs=tri_pairs
)

Alternatively, if Newton exposes the actual triangle pair count after broad phase, that could auto-size it. Either way, the current value is a better default than 1M but will eventually be too small again.

2. Contact margin fix applies unconditionally to all Newton scenes (Low — Design Question)

File: newton_manager.py, start_simulation

The zero-margin → 0.01 fix runs for every Newton scene, not just rough terrain. For manipulation tasks where shapes intentionally have small/zero margins (e.g., tight finger grasps), forcing 0.01 could change contact behavior. The old comment even says "Set smaller contact margin for manipulation examples (default 10cm is too large)" — which this PR removes.

This is likely fine in practice (0.01 is small, and the original code wasn't setting margins either), but worth confirming that manipulation benchmarks (e.g. Allegro, Shadow hand) are not regressed. A targeted approach would be to only fix zero margins for mesh collision shapes, though that adds complexity.

3. Newton pinned to git commit hash — reproducibility concern (Low)

File: setup.py

"newton @ git+https://github.com/newton-physics/newton.git@141baffff9"

This is appropriate for development/testing, but pinning to a commit hash means:

  • No version metadata in the installed package (pip show newton won't show a meaningful version)
  • pip won't cache it efficiently (rebuilds every install)
  • If the newton repo is reorganized, the hash becomes un-fetchable

Is there a plan to tag a Newton release (e.g. 1.1.0) that includes the required fixes? If so, noting it as a TODO would help track it.


Verdict

Good PR. The preset is well-designed, follows established patterns, and the workarounds are individually justified with upstream references. The training results are solid. The findings above are suggestions for robustness — none are blocking.

@hujc7
Copy link
Copy Markdown
Author

hujc7 commented Mar 30, 2026

Will be fixed by @ooctipus

@hujc7 hujc7 closed this Mar 30, 2026
@hujc7
Copy link
Copy Markdown
Author

hujc7 commented Mar 31, 2026

@greptileai Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant