Skip to content

Conversation

@Kenny-Vilella
Copy link
Member

@Kenny-Vilella Kenny-Vilella commented Jan 13, 2026

Description

Following the merge of #1295, the test test_robot.example_robot_anymal_d_cuda_0 seems to fail due to overflow.
This increase of nconmax should solve the issue.
Example of failure:
https://github.com/newton-physics/newton/actions/runs/20937863230/job/60164800649

Newton Migration Guide

Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.

  • The migration guide in docs/migration.rst is up-to date

Before your PR is "Ready for review"

  • Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
  • Documentation is up-to-date
  • Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • Chores
    • Updated robot simulation example configuration parameters for improved contact handling accuracy.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 13, 2026

📝 Walkthrough

Walkthrough

A configuration parameter in the ANYmal D robot example is increased from 35 to 45. The change modifies the maximum number of contacts (nconmax) in the MuJoCo solver configuration for the example simulation. No other logic or functionality is affected.

Changes

Cohort / File(s) Change Summary
Configuration Parameter Adjustment
newton/examples/robot/example_robot_anymal_d.py
Increased nconmax parameter in SolverMuJoCo invocation from 35 to 45

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested reviewers

  • adenzler-nvidia
  • shi-eric
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references fixing a specific failing test, which directly aligns with the PR's objective to fix the test_robot.example_robot_anymal_d_cuda_0 test failure caused by an overflow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kenny-Vilella
Copy link
Member Author

For reference, I could not reproduce the failure locally.
Tried on my workstation and on Horde, but did not see the failure.
The failure seems to occur quite consistently on the main CI.
Hopefully, the increase will be enough but can not guarantee it.

@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
newton/examples/robot/example_robot_anymal_d.py (1)

83-92: LGTM! The increased nconmax addresses the overflow issue.

Increasing the maximum contact constraint capacity from 35 to 45 is a reasonable fix for the test failure caused by the overflow after #1295.

💡 Optional: Consider adding a brief comment for future maintainability

A short comment explaining the rationale could help future maintainers understand why this specific value was chosen:

             ls_iterations=50,
-            nconmax=45,
+            nconmax=45,  # Increased from 35 to handle contact overflow (see PR #1339)
             njmax=100,
📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8ed713 and db8d043.

📒 Files selected for processing (1)
  • newton/examples/robot/example_robot_anymal_d.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-19T12:27:30.629Z
Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 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.

Applied to files:

  • newton/examples/robot/example_robot_anymal_d.py
📚 Learning: 2025-08-19T12:27:30.629Z
Learnt from: preist-nvidia
Repo: newton-physics/newton PR: 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.

Applied to files:

  • newton/examples/robot/example_robot_anymal_d.py
📚 Learning: 2025-12-12T08:45:43.428Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:43.428Z
Learning: In Newtown (Newton) example code, specifically files under newton/examples, computing contacts once per frame and reusing them across all substeps is an intentional design choice, not a bug. Reviewers should verify that contacts are computed before the substep loop and reused for every substep within the same frame. This pattern reduces redundant work and preserves frame-consistency; do not flag as a regression unless the behavior is changed for correctness or performance reasons.

Applied to files:

  • newton/examples/robot/example_robot_anymal_d.py
📚 Learning: 2026-01-13T03:11:40.556Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.

Applied to files:

  • newton/examples/robot/example_robot_anymal_d.py
⏰ 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). (3)
  • GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
  • GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
  • GitHub Check: run-newton-tests / newton-unittests (windows-latest)

@Kenny-Vilella Kenny-Vilella added this pull request to the merge queue Jan 13, 2026
Merged via the queue into newton-physics:main with commit e7494cb Jan 13, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants