Fixes for pre-trained anymal-c example/benchmark#687
Merged
eric-heiden merged 3 commits intoAug 29, 2025
Merged
Conversation
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe example updates construct SolverMuJoCo with ls_parallel=True, removes contact computation/storage and downstream usage, calls solver.step without contacts, and eliminates contact logging in render. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Example Script
participant Solver as SolverMuJoCo
participant Viewer as Viewer
rect rgba(230,245,255,0.6)
note over App,Solver: Initialization
App->>Solver: new SolverMuJoCo(..., ls_parallel=true)
end
loop simulate() step
note over App,Solver: Contacts no longer computed
App->>Solver: step(state, controls, contacts=None)
Solver-->>App: next state
end
rect rgba(240,255,240,0.6)
note over App,Viewer: Rendering
App->>Viewer: render(state)
note right of Viewer: No contact logging
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
Closed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
newton/examples/robot/example_robot_anymal_c_walk.py (2)
145-148: Make MuJoCo-contacts intent explicit and add a guard.To prevent future regressions if defaults change, explicitly opt into MuJoCo contacts and assert the expectation at runtime.
- self.solver = newton.solvers.SolverMuJoCo(self.model, ls_parallel=True) + self.solver = newton.solvers.SolverMuJoCo( + self.model, + ls_parallel=True, + use_mujoco_contacts=True, # collisions handled by MuJoCo + ) + # Defensive: this example expects MuJoCo to run collision detection. + if hasattr(self.solver, "mjw_model"): + assert bool(self.solver.mjw_model.opt.run_collision_detection), \ + "Example requires MuJoCo collision detection; don't disable run_collision_detection."
195-195: GuardNonecontacts onuse_mujoco_contacts
By defaultuse_mujoco_contacts=Truedrivesmjw_model.opt.run_collision_detection, sostep(..., contacts=None, ...)is safe; if that flag is ever disabled, passingNonewill break. Around line 195 innewton/examples/robot/example_robot_anymal_c_walk.py, wrap the call in anif use_mujoco_contactscheck (or compute/provide contacts when it’s False) before invokingself.solver.step.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
newton/examples/robot/example_robot_anymal_c_walk.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: preist-nvidia
PR: newton-physics/newton#579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, there's a distinction between solver parameters and Example class attributes. The Example class can have its own use_mujoco attribute for controlling example-level behavior (like CUDA graphs, rendering logic), while the solver uses use_mujoco_cpu for backend selection. These serve different purposes and should not be conflated during API renames.
Learnt from: preist-nvidia
PR: newton-physics/newton#579
File: newton/examples/example_mujoco.py:350-354
Timestamp: 2025-08-19T12:27:30.629Z
Learning: In Newton examples, Example.use_mujoco is a high-level attribute that controls whether to use MuJoCo solver vs other solvers (like XPBD), while SolverMuJoCo.use_mujoco_cpu controls the backend within MuJoCo (CPU vs Warp). These are separate concepts serving different purposes - the PR rename only applies to the solver parameter, not the Example class attributes.
Learnt from: adenzler-nvidia
PR: newton-physics/newton#479
File: newton/solvers/mujoco/solver_mujoco.py:2147-2149
Timestamp: 2025-07-28T15:04:54.360Z
Learning: In the Newton MuJoCo solver implementation, the ls_parallel option may not be available in the pure MuJoCo mjOption structure, despite being documented in newer MuJoCo versions. The user adenzler-nvidia confirmed that mj_model does not have this attribute in their implementation, so ls_parallel should only be set on mjw_model.opt.
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run GPU Benchmarks (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (1)
newton/examples/robot/example_robot_anymal_c_walk.py (1)
145-145: MJWarp parallel line search is wired correctly.Passing ls_parallel=True to SolverMuJoCo aligns with convert_to_mjc setting mjw_model.opt.ls_parallel; safe for CPU fallback (no effect there).
eric-heiden
approved these changes
Aug 29, 2025
maxkra15
pushed a commit
to maxkra15/newton
that referenced
this pull request
Sep 1, 2025
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
This was referenced Sep 4, 2025
4 tasks
4 tasks
eric-heiden
pushed a commit
to eric-heiden/newton
that referenced
this pull request
Jan 28, 2026
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
This was referenced Feb 9, 2026
4 tasks
3 tasks
mmacklin
pushed a commit
to mmacklin/newton
that referenced
this pull request
Apr 7, 2026
Signed-off-by: Alain Denzler <adenzler@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't call collide because we do the collisions in MuJoCo
Enable parallel linesearch.
Before your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
New Features
Refactor