Skip to content

Fix shape transforms, improve ModelBuilder.add_builder() logic#368

Merged
eric-heiden merged 5 commits into
newton-physics:mainfrom
eric-heiden:fix-shape-kinematics
Jul 10, 2025
Merged

Fix shape transforms, improve ModelBuilder.add_builder() logic#368
eric-heiden merged 5 commits into
newton-physics:mainfrom
eric-heiden:fix-shape-kinematics

Conversation

@eric-heiden

@eric-heiden eric-heiden commented Jul 10, 2025

Copy link
Copy Markdown
Member

Description

Example code to run Anymal D:

# SPDX-FileCopyrightText: Copyright (c) 2025 The Newton Developers
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import warp as wp

import newton
import newton.examples
import newton.utils

wp.config.enable_backward = False


class Example:
    def __init__(self, stage_path="example_usd.usd", num_envs=8, use_cuda_graph=True):
        self.num_envs = num_envs
        articulation_builder = newton.ModelBuilder()

        newton.utils.parse_usd(
            # newton.examples.get_asset("anymal_c.usda"),
            newton.examples.get_asset("anymal_d.usda"),
            articulation_builder,
            joint_drive_gains_scaling=1.0,
            collapse_fixed_joints=False,
            enable_self_collisions=False,
        )

        spacing = 3.0
        sqn = int(wp.ceil(wp.sqrt(float(self.num_envs))))

        builder = newton.ModelBuilder()
        for i in range(self.num_envs):
            pos = wp.vec3((i % sqn) * spacing, (i // sqn) * spacing, 1.3)
            builder.add_builder(articulation_builder, xform=wp.transform(pos, wp.quat_identity()))
        builder.add_ground_plane()

        self.sim_time = 0.0
        fps = 600
        self.frame_dt = 1.0 / fps

        self.sim_substeps = 10
        self.sim_dt = self.frame_dt / self.sim_substeps

        # finalize model
        self.model = builder.finalize()

        # self.solver = newton.solvers.FeatherstoneSolver(self.model)
        # self.solver = newton.solvers.SemiImplicitSolver(self.model, joint_attach_kd=100, joint_attach_ke= 1000)
        # self.solver = newton.solvers.XPBDSolver(self.model, iterations=20)
        self.solver = newton.solvers.MuJoCoSolver(
            self.model,
            use_mujoco=False,
            solver="newton",
            integrator="euler",
            iterations=5,
            ls_iterations=25,
            nefc_per_env=300,
            ncon_per_env=150,
            contact_stiffness_time_const=0.02,
            save_to_mjcf="example_usd.xml",
        )
        self.renderer = None

        if stage_path:
            self.renderer = newton.utils.SimRendererOpenGL(
                path=stage_path,
                model=self.model,
                scaling=1.0,
                up_axis=str(newton.Axis.Z),
                screen_width=1280,
                screen_height=720,
                camera_pos=(0, 1, 4),
            )

        self.state_0, self.state_1 = self.model.state(), self.model.state()
        self.control = self.model.control()

        newton.sim.eval_fk(self.model, self.model.joint_q, self.model.joint_qd, self.state_0)
        self.contacts = self.model.collide(self.state_0)
        self.use_cuda_graph = (
            not getattr(self.solver, "use_mujoco", False) and wp.get_device().is_cuda and use_cuda_graph
        )

        if self.use_cuda_graph:
            with wp.ScopedCapture() as capture:
                self.simulate()
            self.graph = capture.graph

    def simulate(self):
        if not isinstance(self.solver, newton.solvers.MuJoCoSolver):
            self.contacts = self.model.collide(self.state_0)
        for _ in range(self.sim_substeps):
            self.state_0.clear_forces()
            self.solver.step(self.state_0, self.state_1, self.control, self.contacts, self.sim_dt)
            self.state_0, self.state_1 = self.state_1, self.state_0

    def step(self):
        with wp.ScopedTimer("step", active=True):
            if self.use_cuda_graph:
                wp.capture_launch(self.graph)
            else:
                self.simulate()
        self.sim_time += self.frame_dt

    def render(self):
        if self.renderer is None:
            return

        with wp.ScopedTimer("render", active=False):
            self.renderer.begin_frame(self.sim_time)
            self.renderer.render(self.state_0)
            self.renderer.end_frame()


if __name__ == "__main__":
    import argparse

    parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter)
    parser.add_argument("--device", type=str, default=None, help="Override the default Warp device.")
    parser.add_argument(
        "--stage_path",
        type=lambda x: None if x == "None" else str(x),
        default="example_usd.usd",
        help="Path to the output USD file.",
    )
    parser.add_argument("--num_frames", type=int, default=12000, help="Total number of frames.")
    parser.add_argument("--num_envs", type=int, default=1, help="Total number of simulated environments.")
    parser.add_argument(
        "--show_mujoco_viewer",
        type=bool,
        default=True,
        help="Show MuJoCo viewer next to Newton renderer if MuJoCoSolver is used.",
    )
    parser.add_argument("--use_cuda_graph", default=True, action=argparse.BooleanOptionalAction)

    args = parser.parse_known_args()[0]

    with wp.ScopedDevice(args.device):
        example = Example(stage_path=args.stage_path, num_envs=args.num_envs, use_cuda_graph=args.use_cuda_graph)

        show_mujoco_viewer = args.show_mujoco_viewer and isinstance(example.solver, newton.solvers.MuJoCoSolver)
        if show_mujoco_viewer:
            import mujoco
            import mujoco.viewer
            import mujoco_warp

            mjm, mjd = example.solver.mj_model, example.solver.mj_data
            m, d = example.solver.mjw_model, example.solver.mjw_data
            viewer = mujoco.viewer.launch_passive(mjm, mjd)

        for _ in range(args.num_frames):
            example.step()
            example.render()

            if show_mujoco_viewer:
                if not example.solver.use_mujoco:
                    mujoco_warp.get_data_into(mjd, mjm, d)
                viewer.sync()

        if example.renderer:
            example.renderer.save()

Newton Migration Guide

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

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

Before your PR is "Ready for review"

  • All commits are signed-off to indicate that your contribution adheres to the Developer Certificate of Origin requirements
  • I understand that GitHub does not perform any GPU testing of this pull request
  • Necessary tests have been added
  • Documentation is up-to-date
  • Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • Bug Fixes

    • Prevented mixing of models with different up axes when adding builders, ensuring consistent orientation handling.
    • Corrected the orientation adjustment for capsules and cylinders to use a negative 90-degree rotation, improving geometric accuracy.
  • New Features

    • Incorporated an additional transform for each shape, allowing more flexible and accurate computation of shape positions and orientations.
  • Tests

    • Updated tests to validate the new transform logic and corrected orientation, ensuring reliability of geometry property calculations.

Signed-off-by: Eric Heiden <eric-heiden@outlook.com>
Signed-off-by: Eric Heiden <eric-heiden@outlook.com>
Signed-off-by: Eric Heiden <eric-heiden@outlook.com>
@eric-heiden eric-heiden marked this pull request as ready for review July 10, 2025 05:18
@coderabbitai

coderabbitai Bot commented Jul 10, 2025

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The changes introduce stricter validation and more predictable behavior in the ModelBuilder's add_builder method by preventing mixing builders with different up_axis values and ensuring gravity and up_axis are not overwritten. Additionally, the MuJoCo solver and its tests are updated to propagate and utilize an additional transform (shape_incoming_xform) for shapes, affecting how geometry properties are computed and verified.

Changes

File(s) Change Summary
newton/sim/builder.py Modified ModelBuilder.add_builder to raise ValueError if up_axis differs; removed overwriting of up_axis and gravity.
newton/solvers/mujoco/solver_mujoco.py Added shape_incoming_xform propagation throughout geometry property computation; adjusted capsule/cylinder rotation sign; updated kernel and model attributes.
newton/tests/test_mujoco_solver.py Updated tests to account for shape_incoming_xform and corrected rotation sign in expected results.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ModelBuilder
    User->>ModelBuilder: add_builder(other_builder)
    ModelBuilder->>ModelBuilder: Validate up_axis
    alt up_axis mismatch
        ModelBuilder-->>User: Raise ValueError
    else up_axis match
        ModelBuilder->>ModelBuilder: Add builder (no up_axis/gravity overwrite)
    end
Loading
sequenceDiagram
    participant MuJoCoSolver
    participant Model
    participant Kernel
    MuJoCoSolver->>Model: convert_to_mjc()
    Model->>Model: Store shape_incoming_xform
    MuJoCoSolver->>Kernel: update_geom_properties_kernel(..., shape_incoming_xform, ...)
    Kernel->>Kernel: Compute geom properties using incoming_xform
Loading

Assessment against linked issues

Objective Addressed Explanation
Prevent ModelBuilder.add_builder() from overwriting gravity and up_axis with those of the added builder (#298)
Raise an error if ModelBuilder.add_builder() is called with a builder of a different up_axis (#298)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes found.


📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 126ddc3 and 38106ce.

📒 Files selected for processing (3)
  • newton/sim/builder.py (1 hunks)
  • newton/solvers/mujoco/solver_mujoco.py (6 hunks)
  • newton/tests/test_mujoco_solver.py (4 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@eric-heiden eric-heiden merged commit 49215d2 into newton-physics:main Jul 10, 2025
6 of 7 checks passed
This was referenced Jul 11, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Dec 8, 2025
1 task
@coderabbitai coderabbitai Bot mentioned this pull request Dec 16, 2025
3 tasks
eric-heiden added a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026
…ton-physics#368)

Signed-off-by: Eric Heiden <eric-heiden@outlook.com>
mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026
…ton-physics#368)

Signed-off-by: Eric Heiden <eric-heiden@outlook.com>
camevor added a commit to camevor/newton that referenced this pull request May 12, 2026
* Extract joint wrenches for equality constraint-backed joints

* Add tests
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.

ModelBuilder takes the gravity and up_axis to be the values of the last ModelBuilder added with add_builder(). This is an unexpected behaviour.

1 participant