Add missing parameters for Mujoco Solver#424
Conversation
📝 WalkthroughWalkthroughThe changes add two new parameters, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MuJoCoSolver
participant MuJoCoModel
User->>MuJoCoSolver: Initialize(cone, impratio, ...)
MuJoCoSolver->>MuJoCoSolver: convert_to_mjc(cone, impratio, ...)
MuJoCoSolver->>MuJoCoModel: Set option.cone and option.impratio
MuJoCoSolver->>MuJoCoModel: Compile model
MuJoCoSolver->>MuJoCoModel: Set opt.cone and opt.impratio
📜 Recent review detailsConfiguration used: .coderabbit.yml 📥 CommitsReviewing files that changed from the base of the PR and between 48865d7373f1db9dd182cc52298c33f71049eb40 and ada1ac6. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 399d549 and 48865d7373f1db9dd182cc52298c33f71049eb40.
📒 Files selected for processing (1)
newton/solvers/mujoco/solver_mujoco.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Kenny-Vilella
PR: newton-physics/newton#398
File: newton/examples/example_mujoco.py:352-352
Timestamp: 2025-07-14T03:57:29.659Z
Learning: The use_mujoco option in newton/examples/example_mujoco.py is currently unsupported and causes crashes. The code automatically disables this option with a warning message when users attempt to enable it. This is intentionally kept as a placeholder for future implementation.
🔇 Additional comments (6)
newton/solvers/mujoco/solver_mujoco.py (6)
756-757: Parameter additions look good.The new parameters are properly typed and have sensible default values. The
coneparameter accepts both integer and string values, andimpratiois a float with a default of 1.0.
779-780: Documentation is clear and informative.The parameter descriptions accurately explain the purpose of each parameter - contact friction cone type and frictional-to-normal constraint impedance ratio.
816-817: Parameters correctly passed to convert_to_mjc method.The constructor properly forwards the new parameters to the
convert_to_mjcmethod for processing.
1179-1183: String to constant conversion logic is correct.The mapping from string values to MuJoCo constants is properly implemented with appropriate fallback to
mjCONE_PYRAMIDALfor unknown strings.
1201-1202: MuJoCo spec options correctly configured.The
coneandimpratioparameters are properly set in the MuJoCo specification object, ensuring they will be used during model compilation.
1694-1700: Model options correctly applied after compilation.The parameters are properly set on the compiled model's options, ensuring they take effect during simulation. The assignment to both
coneandimpratiois correct.
Signed-off-by: Julia von Muralt <jvonmuralt@nvidia.com>
a844eec to
ada1ac6
Compare
Signed-off-by: Julia von Muralt <jvonmuralt@nvidia.com>
A few fixes and hacks to fix recent failures in unit tests: - TODO: enable ground plane again in tiled_camera and multi_tiled_camera tests - TODO: enable factory mesh insertion task when fixed in physics - TODO: enable teddy bear environment in test_environments.py - TODO: check if scipy test threshold makes sense <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] 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 - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: Rafael Wiltz <rwiltz@nvidia.com>
Signed-off-by: Julia von Muralt <jvonmuralt@nvidia.com>
Description
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this MR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit