VBD cable: add BALL/FIXED joint support #1333
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the VBD solver's constraint representation from per-DOF to per-constraint-slot semantics, updates the cable rod builder to pre-scale stiffness values by segment length, adds comprehensive examples and tests for BALL and FIXED joint types, and extends the solver initialization to support per-constraint Dahl friction parameters loaded from model metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as Model Builder
participant Init as SolverVBD.__init__
participant Layout as _init_joint_constraint_layout()
participant Penalty as _init_joint_penalty_k()
participant Kernels as VBD Kernels
participant Solve as solve_rigid_body()
Builder->>Init: Create solver with model
Note over Init: Parse joint types (CABLE, BALL, FIXED)
Init->>Layout: Call constraint layout init
Layout->>Layout: Count constraints per joint<br/>(Cable: 2, Ball: 1, Fixed: 2)
Layout->>Layout: Compute joint_constraint_start offsets
Init->>Penalty: Call penalty array init<br/>(k_start_linear, k_start_angular)
Penalty->>Penalty: Allocate per-constraint arrays<br/>(k, k_min, k_max, kd)
Penalty->>Penalty: Initialize per-constraint limits
Init->>Kernels: Pass joint_constraint_start<br/>& per-constraint penalties
Solve->>Kernels: Launch warmstart_joints<br/>with joint_penalty_k_max
Kernels->>Kernels: Clamp penalties per constraint
Solve->>Kernels: Launch constraint<br/>force/Hessian evaluation<br/>using constraint indices
Kernels->>Kernels: Map constraint_slot→DOF space
Solve->>Kernels: Launch dual updates<br/>with per-constraint indices
Kernels->>Kernels: Update per-constraint<br/>penalty scalars
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
newton/_src/sim/builder.py (2)
2067-2084: Fix sentinel preservation for numpy integer types in custom-attribute merges.
transform_value()only preserves negative sentinels forint, butnp.int32/np.int64will fall through tov + offset, turning-1intooffset-1. That can corrupt remapped indices.Proposed fix
@@ - def transform_value(v, offset=value_offset, replace_with_world=use_current_world): + def transform_value(v, offset=value_offset, replace_with_world=use_current_world): if replace_with_world: return self.current_world if offset == 0: return v # Handle integers, preserving negative sentinels (e.g., -1 means "invalid") - if isinstance(v, int): - return v + offset if v >= 0 else v + if isinstance(v, (int, np.integer)): + return int(v) + offset if int(v) >= 0 else int(v) # Handle list/tuple explicitly, preserving negative sentinels in elements if isinstance(v, (list, tuple)): - transformed = [x + offset if isinstance(x, int) and x >= 0 else x for x in v] + transformed = [ + (int(x) + offset if isinstance(x, (int, np.integer)) and int(x) >= 0 else x) + for x in v + ] return type(v)(transformed) # For other types (numpy, warp, etc.), try arithmetic offset try: return v + offset except TypeError: return v
2085-2101: Pad list-valued custom-frequency attributes when the frequency already exists.When
merged is Noneandfreq_keyis a custom string frequency, copyingattr.values“as-is” can misalign indices ifselfalready has entries for that frequency (i.e.,index_offset > 0) but this specific attribute key didn’t exist yet.Proposed fix
@@ if merged is None: if attr.values: if isinstance(freq_key, str): - # String frequency: copy list as-is (no offset for sequential data) - mapped_values = [transform_value(value) for value in attr.values] + # String frequency: align with existing frequency count by left-padding. + # (Earlier indices will use defaults via build_array()). + pad = [None] * index_offset + mapped_values = pad + [transform_value(value) for value in attr.values] else: # Enum frequency: remap dict indices with offset mapped_values = { index_offset + idx: transform_value(value) for idx, value in attr.values.items() }newton/_src/solvers/vbd/solver_vbd.py (1)
1086-1113: Bug: particle self-contact is skipped whencontacts is None, contradicting the docstring.
- Doc says (Line 1111–1112) particle self-contact “does not depend on this argument”.
- But in
solve_particle_iteration()(Line 1401–1448), self-contact accumulation is guarded byif contacts is not None, and the only kernel used in the self-contact mode isaccumulate_contact_force_and_hessian(which currently requirescontacts.soft_contact_*inputs).This means
solver.step(..., contacts=None, ...)will silently drop self-contact forces/hessians whenparticle_enable_self_contact=True(likely destabilizing / incorrect).Fix options:
- Split kernels: always run a self-contact-only accumulation kernel, and additionally run body-particle accumulation when
contacts != None, or- Provide an “empty contacts” object/arrays to satisfy the kernel signature when
contacts is None, or- Change the API contract/docs to require
contactswhenever self-contact is enabled (least desirable given current doc).Also applies to: 1363-1485
🤖 Fix all issues with AI agents
In @newton/_src/solvers/vbd/solver_vbd.py:
- Around line 620-750: In _init_joint_penalty_k the CABLE branch indexes
model.joint_target_ke and model.joint_target_kd at dof0 and dof0+1 without
bounds checks; add a cheap CPU-side validation before the loop (or inside the
CABLE case) that ensures dof0 >= 0 and dof0 + 1 < len(jtarget_ke) and
len(jtarget_kd) (use jdofs/jtarget arrays already loaded: jdofs, jtarget_ke,
jtarget_kd), and if the check fails raise a clear RuntimeError mentioning
JointType.CABLE and the offending joint index/jdof; only proceed to write
joint_k_max_np/joint_k_min_np/joint_k0_np/joint_kd_np for that cable after
validation so you avoid IndexError and get a helpful init-time error message.
🧹 Nitpick comments (8)
newton/_src/sim/builder.py (5)
1025-1033: Make scalar handling explicit in_process_joint_custom_attributes()for multi-DOF/coord cases.For
dof_count/coord_count != 1, passing a scalar currently triggersTypeErroratlen(value_sanitized); consider raising a clearer error before callinglen().Also applies to: 1072-1080
1633-1684:begin_world(..., attributes=...)acceptsattributesbut doesn’t persist them.Either store world metadata (e.g.,
self.world_key,self.world_attributes) or drop the parameter to avoid a “looks-supported-but-isn’t” API.
2705-2719: Enforce “JOINT-only” custom attributes for FIXED joints (optional).
add_joint_fixed()’s doc says only JOINT-frequency attrs are valid, but it forwards to_process_joint_custom_attributes()which will error oddly for JOINT_DOF/JOINT_COORD. Consider pre-validating and raising a targetedValueError.
2890-2963: CABLE joint stiffness semantics: doc + storage mapping are clear; consider axis clarity.Doc now states stiffness/damping are stored in
target_ke/target_kdper DOF and axis is unused by SolverVBD for cable joints—good. Minor:JointDofConfig(...)defaultsaxis=Axis.X; consider setting an explicit dummy axis (or documenting the default) to avoid confusion when inspecting model data.
4609-4822: Rod stiffness pre-scaling by segment length: good, but consider extreme-length robustness.The
EA/LandEI/Lconversion and passing effective values intoadd_joint_cable()is consistent with the new “effective per-joint” semantics. Only concern: very smallseg_lencan explode*_ke_eff; consider optionally warning/clamping ifseg_lenis below a practical epsilon.newton/_src/solvers/vbd/solver_vbd.py (2)
145-306: Good separation of “cable per-DOF tuning” vs “rigid-joint global tuning”; clamping is sensible.
Ctor docs and_init_rigid_system()clamping (max(0.0, …)) look consistent and should prevent negative penalties/damping. One small edge case: if the model has joints but no supported constraints (e.g., FREE-only), consider guarding launches byself.joint_constraint_count > 0(not justmodel.joint_count > 0) to avoid relying on Warp’s behavior fordim=0.Also applies to: 417-451
528-540: Custom Dahl attributes look good; consider enforcing device/shape expectations when present.
Line 535–539 assignsmodel.vbd.dahl_eps_max/taudirectly to solver state. If those attributes can ever be authored on a different device or with an unexpected length, failures will happen later inside kernels. A quick init-time assertion (device +shape[0] == model.joint_count) would make misconfiguration much easier to debug.Also applies to: 799-829
newton/tests/test_cable.py (1)
902-1156: Great end-to-end coverage for BALL/FIXED cable endpoint anchoring (including driven kinematic anchor).
These tests exercise exactly the new solver behavior (BALL/FIXED constraints + penalty caps), and the added geometric assertions (_surface_attachment + capsule chain attachment) are strong invariants beyond “no crash”.
One minor robustness tweak: if you see occasional CPU-vs-CUDA drift, consider asserting on a trackedmax(err)over the step (instead of per-substep), or slightly relaxing the 1e-3 position threshold for BALL while keeping FIXED’s angular threshold as-is.Also applies to: 1198-1209
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/images/examples/example_cable_bend.jpgis excluded by!**/*.jpgdocs/images/examples/example_cable_bundle_hysteresis.jpgis excluded by!**/*.jpgdocs/images/examples/example_cable_twist.jpgis excluded by!**/*.jpg
📒 Files selected for processing (12)
README.mdnewton/_src/sim/builder.pynewton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_bend.pynewton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_fixed_joints.pynewton/examples/cable/example_cable_twist.pynewton/tests/test_cable.pynewton/tests/test_up_axis.py
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/tests/test_up_axis.pynewton/examples/cable/example_cable_fixed_joints.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_bundle_hysteresis.pynewton/_src/solvers/vbd/solver_vbd.pynewton/tests/test_cable.py
📚 Learning: 2025-08-18T15:56:26.587Z
Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.
Applied to files:
newton/tests/test_up_axis.pynewton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/tests/test_up_axis.pynewton/_src/solvers/vbd/solver_vbd.pynewton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-11-28T11:12:40.805Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:206-213
Timestamp: 2025-11-28T11:12:40.805Z
Learning: In Newton's standard solvers (XPBD, SemiImplicit, SolverMuJoCo), spatial vectors in State.body_qd use the ordering (linear, angular): wp.spatial_top(qd) returns linear velocity and wp.spatial_bottom(qd) returns angular velocity. This is the opposite of typical Modern Robotics spatial twist notation where (angular, linear) is used.
Applied to files:
newton/tests/test_up_axis.pynewton/examples/cable/example_cable_twist.pynewton/tests/test_cable.pynewton/_src/solvers/vbd/rigid_vbd_kernels.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/cable/example_cable_fixed_joints.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_bend.pynewton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_twist.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/examples/cable/example_cable_bend_damping.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/examples/cable/example_cable_bend_damping.py
📚 Learning: 2025-10-29T14:55:48.171Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 940
File: newton/examples/mpm/example_mpm_granular.py:92-101
Timestamp: 2025-10-29T14:55:48.171Z
Learning: In newton/examples/mpm/example_mpm_granular.py, the capture() method intentionally advances the simulation state once during CUDA graph capture. This is expected behavior because CUDA graph capture records exact memory addresses, so the state arrays (state_0, state_1) cannot be reset or reallocated after capture without invalidating the graph.
Applied to files:
newton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_twist.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-12-03T09:33:27.083Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/examples/sim/example_sim_basics_cartpole.py:388-392
Timestamp: 2025-12-03T09:33:27.083Z
Learning: In the Kamino ModelBuilder class located at newton/_src/solvers/kamino/core/builder.py, the attributes `gravity`, `bodies`, `joints`, `collision_geoms`, and `physical_geoms` are implemented as property decorated methods, so they should be accessed as attributes (e.g., `builder.gravity`) rather than called as methods (e.g., `builder.gravity()`).
Applied to files:
newton/examples/cable/example_cable_bundle_hysteresis.py
📚 Learning: 2025-09-22T21:03:18.367Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.pynewton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.pynewton/tests/test_cable.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is properly re-exported from the spatial module via "from .spatial import (..., transform_twist, ...)" and included in __all__, making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/examples/cable/example_cable_twist.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is re-exported from the spatial module via "from .spatial import transform_twist", making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/examples/cable/example_cable_twist.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-08-12T17:58:16.929Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/tests/test_ik.py:25-26
Timestamp: 2025-08-12T17:58:16.929Z
Learning: In the Newton physics engine codebase, it's acceptable for tests to import and use private `_src` internal modules and functions when needed for testing purposes, even though this breaks the public API boundary. This is an intentional project decision for test code.
Applied to files:
newton/tests/test_cable.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).
Applied to files:
newton/tests/test_cable.py
📚 Learning: 2025-11-24T08:05:21.390Z
Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.
Applied to files:
newton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-12-01T16:21:36.581Z
Learnt from: jvonmuralt
Repo: newton-physics/newton PR: 1160
File: newton/_src/sim/articulation.py:0-0
Timestamp: 2025-12-01T16:21:36.581Z
Learning: In Newton's Featherstone solver, during forward kinematics for FREE/DISTANCE joints, the computed spatial velocity v_wc follows Featherstone's spatial twist convention where the linear component (spatial_top) represents the velocity at the world origin (0,0,0), not at the joint anchor or COM. To convert to State.body_qd (which stores COM velocity), use v_com = v_origin + ω × x_com, where x_com is the absolute world position of the COM.
Applied to files:
newton/_src/solvers/vbd/rigid_vbd_kernels.py
🧬 Code graph analysis (4)
newton/tests/test_up_axis.py (2)
newton/_src/sim/builder.py (1)
color(5886-5950)newton/_src/solvers/vbd/solver_vbd.py (1)
SolverVBD(97-1994)
newton/examples/cable/example_cable_bundle_hysteresis.py (1)
newton/_src/solvers/vbd/solver_vbd.py (2)
SolverVBD(97-1994)register_custom_attributes(801-828)
newton/tests/test_cable.py (3)
newton/_src/sim/model.py (2)
Model(100-909)set_gravity(636-656)newton/_src/sim/builder.py (5)
ModelBuilder(70-6833)add_rod(4609-4822)finalize(6055-6729)copy(282-283)dot(5239-5240)newton/tests/unittest_utils.py (1)
add_function_test(320-339)
newton/_src/solvers/vbd/rigid_vbd_kernels.py (2)
newton/_src/sim/joints.py (1)
JointType(20-47)newton/_src/utils/import_utils.py (1)
transform(139-140)
🪛 GitHub Actions: Pull Request
newton/tests/test_up_axis.py
[error] 50-50: Test gravity_y_up_mujoco_warp_cpu failed: expected -0.981, got NaN.
[error] 50-50: Test gravity_z_up_mujoco_warp_cpu failed: expected -0.981, got NaN.
🪛 Ruff (0.14.10)
newton/examples/cable/example_cable_fixed_joints.py
518-522: Avoid specifying long messages outside the exception class
(TRY003)
newton/examples/cable/example_cable_ball_joints.py
488-488: Avoid specifying long messages outside the exception class
(TRY003)
newton/_src/solvers/vbd/solver_vbd.py
647-650: Avoid specifying long messages outside the exception class
(TRY003)
653-655: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (2)
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
🔇 Additional comments (40)
newton/examples/cable/example_cable_bend_damping.py (5)
52-53: LGTM!The docstring improvements clarify that
pointsare segment endpoints (not capsule centers) and correctly note thatedge_indicesare not consumed byadd_rod(). This prevents confusion for future maintainers.
167-167: LGTM!Comment accurately describes the cable positioning logic.
194-200: LGTM!Explicitly zeroing both
body_inertiaandbody_inv_inertiaalongside mass ensures the first body is fully kinematic, preventing any unintended rotational dynamics. This is a good robustness improvement.
228-234: LGTM!Using
self.solver.device.is_cudainstead ofwp.get_device().is_cudais more robust—it ensures CUDA graph capture only occurs when the solver itself is on a CUDA device, avoiding potential mismatches in multi-device scenarios.
325-332: LGTM!The updated test logic is more robust:
- Uses
cable_bodies_listindices instead of hardcoded valuesfirst_cable[0](anchored/kinematic) vsfirst_cable[-1](free end) correctly identifies the endpoints- The sag assertion properly validates that the free end hangs below the anchored end under gravity
newton/_src/sim/builder.py (2)
5952-6054: Verify the “global (-1) only at start/end” ordering constraint won’t break existing workflows.
_validate_world_ordering()rejects-1entries in the middle of entity arrays. This can break common patterns like: add world 0, add global ground plane, add world 1. If this is intended, it likely needs an explicit docs/migration callout; otherwise consider relaxing this check.
6078-6114: Finalize-time joint/articulation validation looks appropriately lightweight.The orphan-joint check is O(n) and avoids heavier graph validation; this aligns with keeping
finalize()validation lightweight. Based on learnings, this direction is good.newton/examples/cable/example_cable_bundle_hysteresis.py (4)
196-197: LGTM: Custom attribute registration before model construction.Correctly registers solver-specific Dahl plasticity attributes before building the model, which aligns with the
SolverVBD.register_custom_attributesAPI documented in the relevant code snippets.
279-283: LGTM: Complete kinematic body setup.Zeroing both mass/inv_mass and inertia/inv_inertia ensures the obstacle bodies are fully kinematic. This is consistent with other cable examples in this PR.
306-311: LGTM: Per-model Dahl parameter configuration.The defensive
hasattrcheck andfill_()calls correctly configure per-joint Dahl parameters after model finalization, as required by the solver's attribute-based configuration pattern.
313-318: LGTM: Solver instantiation with Dahl friction enabled.The solver is correctly instantiated with
rigid_enable_dahl_friction=with_dahl, relying on the model'svbdattributes for Dahl parameters rather than constructor arguments.newton/tests/test_up_axis.py (2)
37-38: LGTM: Required coloring for SolverVBD compatibility.Adding
builder.color()beforefinalize()is necessary for SolverVBD, which requires coloring information for both particles and rigid bodies as documented in the ModelBuilder.color description.
64-64: VBD solver integration follows the existing pattern.The
vbdsolver entry at line 64 is correctly formatted and consistent with the other solver entries in the test matrix.README.md (1)
197-228: LGTM: Well-structured Cable Examples documentation.The new Cable Examples section follows the established README format with proper table layout, image links, and run commands. The examples align with the new cable functionality introduced in this PR.
newton/examples/cable/example_cable_bend.py (5)
55-56: LGTM: Clarified docstring.The updated docstring accurately describes the return values and notes that
edge_indicesare not used byadd_rod().
161-161: LGTM: Per-cable body tracking.Adding
cable_bodies_listenables robust testing by storing the actual body indices returned byadd_rod()for each cable, rather than assuming contiguous allocation.
194-198: LGTM: Complete kinematic body setup and body list storage.Zeroing both inertia and inv_inertia for the first body ensures proper kinematic behavior. Storing
rod_bodiesincable_bodies_listenables accurate per-cable testing.
281-295: LGTM: Robust cable connectivity test.The test now correctly iterates over the stored
cable_bodies_listto check segment connectivity, which is more reliable than assuming body index patterns.
312-318: LGTM: Improved hanging test using stored body indices.Using
self.cable_bodies_list[0]to access the first cable's anchored and free ends is more robust than hardcoded index calculations.newton/examples/cable/example_cable_ball_joints.py (5)
37-74: LGTM: Ball joint error computation kernel.The kernel correctly computes world-space distance between parent and child anchor points, providing a clear verification mechanism for BALL joint constraint satisfaction. The docstring accurately describes the computation and its purpose.
76-122: LGTM: Kinematic anchor animation kernels.The
advance_timeandmove_kinematic_anchorskernels provide clean separation of time advancement and anchor motion. The smoothstep ramp prevents sudden velocity spikes at simulation start.
124-140: LGTM: Helper utilities.
_auto_scaleand_make_straight_cable_downare well-implemented utilities for mesh scaling and cable geometry generation.
142-408: LGTM: Well-structured BALL joint example.The
Exampleclass demonstrates proper usage of:
- Kinematic body creation with complete mass/inertia zeroing (Lines 284-287)
- BALL joint attachment between kinematic drivers and cable first segments
- Manual articulation assembly with
wrap_in_articulation=Falsefollowed byadd_articulation()- Proper capture pattern for CUDA graph optimization
The solver configuration with high
rigid_joint_linear_keandrigid_joint_linear_k_startensures stiff ball joint behavior.
465-489: LGTM: Ball joint validation test.The
test_finalmethod correctly validates ball joint constraint satisfaction using thecompute_ball_anchor_errorkernel. The 1.0e-3 tolerance is appropriate for stiff penalty-based constraints.The static analysis hint (TRY003) about the exception message is acceptable here—descriptive error messages in test assertions aid debugging.
newton/examples/cable/example_cable_fixed_joints.py (4)
37-85: LGTM: Fixed joint error computation kernel.The kernel correctly computes both position and angular error between joint frames:
- Position error: world-space anchor distance
- Angular error: using quaternion difference with proper handling of the double-cover (using
abs(dq[3])to get shortest-angle measure)The docstring clearly documents the computation approach.
87-151: LGTM: Shared kernel and helper implementations.The
advance_time,move_kinematic_anchors,_auto_scale, and_make_straight_cable_downimplementations are consistent with the BALL joint example.
153-430: LGTM: Well-structured FIXED joint example.The
Exampleclass properly demonstrates:
- FIXED joint attachment preserving both position and orientation
- Correct joint frame setup using
parent_frame_q = rod_quats[0]to match the cable's initial orientation- Additional
rigid_joint_angular_keparameter for angular stiffness on FIXED joints- Complete test buffer setup including frame quaternions
The structure mirrors the BALL joint example appropriately, with necessary extensions for angular constraint validation.
488-523: LGTM: Fixed joint validation test.The
test_finalmethod validates both position (tol=1.0e-3) and angular (tol=1.0e-2 rad ≈ 0.57°) constraint satisfaction. The tolerances are appropriate for stiff penalty-based constraints.The static analysis hint (TRY003) about the exception message is acceptable—the detailed error reporting aids debugging.
newton/examples/cable/example_cable_twist.py (5)
35-58: LGTM: Dual-buffer kinematic transform update.The kernel now correctly writes the updated kinematic transform to both
body_q0andbody_q1. This is necessary because:
- The kernel runs before
solver.step()- After
solver.step(), states are swapped- Both buffers must have the correct kinematic pose to maintain consistency across the state swap
This prevents the kinematic body from reverting to stale transforms after each swap.
79-80: LGTM: Clarified docstring.Updated return description accurately reflects that
edge_indicesare not used byadd_rod().
238-239: LGTM: Complete kinematic body setup.Zeroing both inertia and inv_inertia ensures the first body is fully kinematic, consistent with other cable examples.
275-275: LGTM: Explicit device check.Using
self.solver.device.is_cudais more explicit and correct thanwp.get_device().is_cuda, as it references the solver's actual device rather than the global default.
292-292: LGTM: Dual-buffer kernel output.The
outputsparameter now correctly lists both state buffers, matching the kernel's dual-write pattern.newton/_src/solvers/vbd/solver_vbd.py (2)
1177-1362: Optional contacts plumbing is consistent for rigid-body paths; dual updates remain safe.
The pattern “skip contact adjacency/warmstart/accumulation/dual-updates whencontacts is None” is applied consistently, while joint solving/dual updates still run. That’s the right shape for “joint-only rigid solve” scenarios.Also applies to: 1639-1895
573-619: This is intentional design for new Cable VBD rigid body functionality, not a breaking change.Lines 596–607 raise
NotImplementedErrorfor unsupported joint types (only CABLE, BALL, FIXED, and FREE are supported). This is by design—the docstring explicitly states "Any other joint type will raiseNotImplementedError." This is part of new rigid body VBD functionality (PR #1114) with documented constraints. Current tests (e.g.,test_cable.py) only use BALL and FIXED joints, which are supported. No existing code is broken by this guard.newton/tests/test_cable.py (2)
32-116: Test helpers + kinematic pose kernel are clean and readable.
The helpers make failure modes much more diagnosable (context-rich messages), and_set_kinematic_body_poseis a good “single source of truth” for pose+velocity reset.Also applies to: 123-132, 139-153
279-349: The codebase already uses the established pattern of.joint_parent.numpy()directly throughout (test_cable.py, test_coloring.py, ik_objectives.py, solver_mujoco.py, etc.), and no CI flakiness has been reported. Model metadata arrays likejoint_parentandjoint_X_pare CPU-resident and immutable; the.to("cpu").numpy()pattern is appropriately reserved for mutable warp arrays likebody_q. The code is also already withinwp.ScopedDevice("cpu")context, which handles device placement correctly. No change needed.newton/_src/solvers/vbd/rigid_vbd_kernels.py (3)
261-367: Isotropic linear/angular constraint evaluators are consistent with existing contact Jacobian/Hessian conventions.
TheH_alskew-sign and the “flip force/torque but not Hessian for parent/child” handling match what you already do for rigid contacts, which should keep the 6×6 block system symmetric/PSD under the same assumptions.Also applies to: 370-471
848-1051: Per-constraint-slot indexing for CABLE/BALL/FIXED is clear and consistently applied.
joint_constraint_start+joint_penalty_k/kdusage is straightforward, and the constraint-slot layout matches what SolverVBD documents/constructs.Also applies to: 2079-2205
1383-1499: Dahl envelope now correctly keys off the bend cap (constraint slot 1) instead of DOF indexing.
Usingjoint_penalty_k_max[c_start + 1]as the effective bend stiffness target aligns with the per-constraint penalty refactor and avoids reintroducing DOF-vs-constraint ambiguity.Also applies to: 2406-2532
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/solvers/vbd/solver_vbd.py (1)
1092-1113: Particle self-contact is incorrectly skipped whencontacts is None(contradicts docstring).
Insolve_particle_iteration(), whenself.particle_enable_self_contactis True, the self-contact accumulation kernel is only launched underif contacts is not None, which disables self-contact entirely when callers intentionally passcontacts=None. This conflicts with the method doc (“particle self-contact … does not depend on this argument”) and is likely a real behavior bug.Proposed fix (keep self-contact working with contacts=None by passing empty soft-contact buffers)
class SolverVBD(SolverBase): @@ def _init_particle_system( @@ if model.particle_count == 0: return + + # Empty soft-contact buffers for cases where `contacts=None` (self-contact still supported). + # These allow launching the self-contact kernel without requiring a Contacts object. + self._empty_soft_contact_count = wp.array([0], dtype=wp.int32, device=self.device) + self._empty_soft_contact_max = 0 + self._empty_soft_contact_particle = wp.empty(shape=(0,), dtype=wp.int32, device=self.device) + self._empty_soft_contact_shape = wp.empty(shape=(0,), dtype=wp.int32, device=self.device) + self._empty_soft_contact_body_pos = wp.empty(shape=(0,), dtype=wp.vec3, device=self.device) + self._empty_soft_contact_body_vel = wp.empty(shape=(0,), dtype=wp.vec3, device=self.device) + self._empty_soft_contact_normal = wp.empty(shape=(0,), dtype=wp.vec3, device=self.device) @@ def solve_particle_iteration( self, state_in: State, state_out: State, contacts: Contacts | None, dt: float, iter_num: int ): @@ for color in range(len(model.particle_color_groups)): # Accumulate contact forces if self.particle_enable_self_contact: - if contacts is not None: + # Self-contact does not require rigid contacts; use empty soft-contact buffers when contacts is None. + soft_contact_particle = ( + contacts.soft_contact_particle if contacts is not None else self._empty_soft_contact_particle + ) + soft_contact_count = contacts.soft_contact_count if contacts is not None else self._empty_soft_contact_count + soft_contact_max = contacts.soft_contact_max if contacts is not None else self._empty_soft_contact_max + soft_contact_shape = contacts.soft_contact_shape if contacts is not None else self._empty_soft_contact_shape + soft_contact_body_pos = ( + contacts.soft_contact_body_pos if contacts is not None else self._empty_soft_contact_body_pos + ) + soft_contact_body_vel = ( + contacts.soft_contact_body_vel if contacts is not None else self._empty_soft_contact_body_vel + ) + soft_contact_normal = ( + contacts.soft_contact_normal if contacts is not None else self._empty_soft_contact_normal + ) + wp.launch( kernel=accumulate_contact_force_and_hessian, dim=self.collision_evaluation_kernel_launch_size, inputs=[ @@ # body-particle contact model.particle_radius, - contacts.soft_contact_particle, - contacts.soft_contact_count, - contacts.soft_contact_max, + soft_contact_particle, + soft_contact_count, + soft_contact_max, self.body_particle_contact_penalty_k, self.body_particle_contact_material_kd, self.body_particle_contact_material_mu, @@ model.shape_body, body_q_for_particles, body_q_prev_for_particles, body_qd_for_particles, model.body_com, - contacts.soft_contact_shape, - contacts.soft_contact_body_pos, - contacts.soft_contact_body_vel, - contacts.soft_contact_normal, + soft_contact_shape, + soft_contact_body_pos, + soft_contact_body_vel, + soft_contact_normal, ], outputs=[ self.particle_forces, self.particle_hessians, ], device=self.device, max_blocks=model.device.sm_count, )Also applies to: 1363-1485
🧹 Nitpick comments (10)
newton/_src/sim/builder.py (2)
4638-4645: Call out the add_rod semantics change loudly (material-like inputs vs per-joint inputs).The doc now states
add_rod(..., stretch_stiffness, bend_stiffness)are material-like (EA, EI) and converted by dividing by segment length. Since the public API name didn’t change and onlyadd_rodhas this “material-like” interpretation (vsadd_joint_cabletaking effective per-joint values), I’d strongly recommend an explicit migration note/example showing equivalence between the two APIs (e.g., “to match old behavior passEA = k_per_joint * L”).Also applies to: 4671-4672
4789-4809: Verify damping scaling choice (stiffness is length-scaled, damping is not).You pre-scale
stretch_stiffnessandbend_stiffnessby1/Lbut passstretch_damping/bend_dampingthrough unchanged. If users think*_dampingis paired with the material-like stiffness inputs, they may reasonably expect analogous scaling (or at least a doc note that damping is already “per-joint”).Possible adjustment (only if damping is intended to be material-like too)
seg_len = segment_lengths[parent_idx] stretch_ke_eff = stretch_stiffness / seg_len bend_ke_eff = bend_stiffness / seg_len +stretch_kd_eff = stretch_damping / seg_len +bend_kd_eff = bend_damping / seg_len joint = self.add_joint_cable( parent=parent_body, child=child_body, parent_xform=parent_xform, child_xform=child_xform, bend_stiffness=bend_ke_eff, - bend_damping=bend_damping, + bend_damping=bend_kd_eff, stretch_stiffness=stretch_ke_eff, - stretch_damping=stretch_damping, + stretch_damping=stretch_kd_eff, key=joint_key, collision_filter_parent=True, enabled=True, )newton/_src/solvers/vbd/rigid_vbd_kernels.py (3)
261-367: Good refactor: isotropic linear/rotational constraint blocks are consistent and map once.
The “local force/Hessian then map” structure looks coherent, and the sign handling viais_parentis straightforward. Only minor robustness: both functions assumedt > 0(multiple1.0 / dtsites). If there’s any chance ofdt==0in callers (even in tests), add a cheap guard in the caller or here.Also applies to: 370-472
847-1051: Per-constraint-slot joint indexing looks correct; consider a debug-only bounds sanity check onc_start.
Givenjoint_constraint_start[j]drives indexing intojoint_penalty_k/joint_penalty_kd, a bad layout would become silent OOB on device. If you have a host-side init path, it’d be worth assertingc_start + expected_dim <= joint_penalty_k.shape[0]during solver setup.
2078-2205: Dual updates for CABLE/BALL/FIXED are clear; minor duplication could be trimmed later.
The per-type violation magnitudes and per-slot updates are readable. If you want to reduce repetition, you could factor out the commonx_p/x_cextraction for BALL+FIXED linear updates.newton/_src/solvers/vbd/solver_vbd.py (1)
642-656: Ruff TRY003: long RuntimeError messages could be shortened or moved to exception subclasses.
Not correctness-critical, but Ruff is flagging theseRuntimeError(...)strings. If you want to keep TRY003 clean, consider shorter messages here or custom exception types.newton/examples/cable/example_cable_bundle_hysteresis.py (1)
306-311: Safer guard when authoringmodel.vbd.*: check attributes, not just namespace.
hasattr(self.model, "vbd")may be true for other reasons; consider also checkinghasattr(self.model.vbd, "dahl_eps_max")/"dahl_tau"beforefill_()to avoid attribute errors if someone runs the example without registering custom attrs.newton/examples/cable/example_cable_twist.py (1)
234-240: Kinematic endpoint should also normalizerot_new(tiny drift over long runs).
rot_new = wp.mul(dq, rot)will accumulate numerical error; considerrot_new = wp.normalize(rot_new)in the kernel.newton/examples/cable/example_cable_ball_joints.py (2)
132-140: Anchor placement logic is careful for primitives; mesh (“bear”) anchor likely needs a surface-based placement too.
Sphere/capsule/box anchors are projected onto the surface, but the mesh case keeps a genericattach_offsetand az0-based anchor that can float relative to the mesh (sincezis adjusted for the bear). If the goal is “anchor at the bottom surface”, consider computing a mesh-derived local anchor from the scaled AABB (or at least basinganchor_world.zonz, notz0).Also applies to: 289-366
471-489: Ruff TRY003: long AssertionError message.
If you’re keeping Ruff clean, this would likely be flagged similarly to the solver file—optional to adjust.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/images/examples/example_cable_bend.jpgis excluded by!**/*.jpgdocs/images/examples/example_cable_bundle_hysteresis.jpgis excluded by!**/*.jpgdocs/images/examples/example_cable_twist.jpgis excluded by!**/*.jpg
📒 Files selected for processing (11)
README.mdnewton/_src/sim/builder.pynewton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_bend.pynewton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_fixed_joints.pynewton/examples/cable/example_cable_twist.pynewton/tests/test_cable.py
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 Learning: 2025-12-03T09:33:27.083Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/examples/sim/example_sim_basics_cartpole.py:388-392
Timestamp: 2025-12-03T09:33:27.083Z
Learning: In the Kamino ModelBuilder class located at newton/_src/solvers/kamino/core/builder.py, the attributes `gravity`, `bodies`, `joints`, `collision_geoms`, and `physical_geoms` are implemented as property decorated methods, so they should be accessed as attributes (e.g., `builder.gravity`) rather than called as methods (e.g., `builder.gravity()`).
Applied to files:
newton/examples/cable/example_cable_bundle_hysteresis.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_fixed_joints.pynewton/tests/test_cable.pynewton/_src/solvers/vbd/solver_vbd.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/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_ball_joints.pynewton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_fixed_joints.pynewton/examples/cable/example_cable_twist.pynewton/examples/cable/example_cable_bend.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/examples/cable/example_cable_bend_damping.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because the BVH query kernels that use them always write the count to query_results[0, tid] = 0 first before populating any data, ensuring no undefined reads occur.
Applied to files:
newton/examples/cable/example_cable_bend_damping.py
📚 Learning: 2025-10-29T14:55:48.171Z
Learnt from: gdaviet
Repo: newton-physics/newton PR: 940
File: newton/examples/mpm/example_mpm_granular.py:92-101
Timestamp: 2025-10-29T14:55:48.171Z
Learning: In newton/examples/mpm/example_mpm_granular.py, the capture() method intentionally advances the simulation state once during CUDA graph capture. This is expected behavior because CUDA graph capture records exact memory addresses, so the state arrays (state_0, state_1) cannot be reset or reallocated after capture without invalidating the graph.
Applied to files:
newton/examples/cable/example_cable_bend_damping.pynewton/examples/cable/example_cable_twist.py
📚 Learning: 2025-08-12T17:58:16.929Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/tests/test_ik.py:25-26
Timestamp: 2025-08-12T17:58:16.929Z
Learning: In the Newton physics engine codebase, it's acceptable for tests to import and use private `_src` internal modules and functions when needed for testing purposes, even though this breaks the public API boundary. This is an intentional project decision for test code.
Applied to files:
newton/tests/test_cable.py
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/geometry.py:16-22
Timestamp: 2025-08-12T17:51:37.474Z
Learning: In the Newton public API refactor, common geometry symbols like ParticleFlags and ShapeFlags are now exposed at the top level in newton/__init__.py rather than through newton.geometry. The newton/geometry.py module is intentionally focused only on broad-phase collision detection classes (BroadPhaseAllPairs, BroadPhaseExplicit, BroadPhaseSAP).
Applied to files:
newton/tests/test_cable.py
📚 Learning: 2025-08-12T05:17:34.423Z
Learnt from: shi-eric
Repo: newton-physics/newton PR: 521
File: newton/examples/example_cloth_hanging.py:36-36
Timestamp: 2025-08-12T05:17:34.423Z
Learning: The Newton migration guide (docs/migration.rst) is specifically for documenting how to migrate existing warp.sim functionality to Newton equivalents. New Newton-only features that didn't exist in warp.sim do not need migration documentation.
Applied to files:
newton/tests/test_cable.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/tests/test_cable.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-11-28T11:12:40.805Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1146
File: newton/examples/basic/example_basic_joints.py:206-213
Timestamp: 2025-11-28T11:12:40.805Z
Learning: In Newton's standard solvers (XPBD, SemiImplicit, SolverMuJoCo), spatial vectors in State.body_qd use the ordering (linear, angular): wp.spatial_top(qd) returns linear velocity and wp.spatial_bottom(qd) returns angular velocity. This is the opposite of typical Modern Robotics spatial twist notation where (angular, linear) is used.
Applied to files:
newton/tests/test_cable.pynewton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is properly re-exported from the spatial module via "from .spatial import (..., transform_twist, ...)" and included in __all__, making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/examples/cable/example_cable_twist.py
📚 Learning: 2025-08-12T17:58:00.815Z
Learnt from: nvlukasz
Repo: newton-physics/newton PR: 519
File: newton/_src/solvers/featherstone/kernels.py:18-20
Timestamp: 2025-08-12T17:58:00.815Z
Learning: In newton/_src/core/__init__.py, the function transform_twist is re-exported from the spatial module via "from .spatial import transform_twist", making it available for import as "from ...core import transform_twist" from other modules.
Applied to files:
newton/examples/cable/example_cable_twist.py
📚 Learning: 2025-08-18T15:56:26.587Z
Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 552
File: newton/_src/solvers/mujoco/solver_mujoco.py:0-0
Timestamp: 2025-08-18T15:56:26.587Z
Learning: In Newton's MuJoCo solver, when transforming joint axes from Newton's internal frame to MuJoCo's expected frame, use wp.quat_rotate(joint_rot, axis) not wp.quat_rotate_inv(joint_rot, axis). The joint_rot represents rotation from joint-local to body frame, so forward rotation is correct.
Applied to files:
newton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-09-25T16:14:22.033Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_objectives.py:0-0
Timestamp: 2025-09-25T16:14:22.033Z
Learning: In NVIDIA Warp's Newton physics library, wp.transform supports direct numerical indexing (e.g., body_tf[0], body_tf[1], body_tf[2] for position and body_tf[3], body_tf[4], body_tf[5], body_tf[6] for quaternion components) to access its elements. This is the standard API used throughout the codebase.
Applied to files:
newton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/examples/cable/example_cable_twist.pynewton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/solvers/vbd/rigid_vbd_kernels.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-11-24T08:05:21.390Z
Learnt from: adenzler-nvidia
Repo: newton-physics/newton PR: 1107
File: newton/_src/solvers/mujoco/kernels.py:973-974
Timestamp: 2025-11-24T08:05:21.390Z
Learning: In Newton's MuJoCo solver integration (newton/_src/solvers/mujoco/), the mapping between Newton joints and MuJoCo joints is not 1-to-1. Instead, each Newton DOF maps to a distinct MuJoCo joint. This means that for multi-DOF joints (like D6 with 6 DOFs), there will be 6 corresponding MuJoCo joints, each with its own properties (margin, solimp, solref, etc.). The mapping is done via dof_to_mjc_joint array, ensuring each DOF's properties are written to its own MuJoCo joint without overwriting.
Applied to files:
newton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2025-12-01T16:21:36.581Z
Learnt from: jvonmuralt
Repo: newton-physics/newton PR: 1160
File: newton/_src/sim/articulation.py:0-0
Timestamp: 2025-12-01T16:21:36.581Z
Learning: In Newton's Featherstone solver, during forward kinematics for FREE/DISTANCE joints, the computed spatial velocity v_wc follows Featherstone's spatial twist convention where the linear component (spatial_top) represents the velocity at the world origin (0,0,0), not at the joint anchor or COM. To convert to State.body_qd (which stores COM velocity), use v_com = v_origin + ω × x_com, where x_com is the absolute world position of the COM.
Applied to files:
newton/_src/solvers/vbd/rigid_vbd_kernels.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
🧬 Code graph analysis (4)
newton/examples/cable/example_cable_bundle_hysteresis.py (1)
newton/_src/solvers/vbd/solver_vbd.py (2)
SolverVBD(97-1994)register_custom_attributes(801-828)
newton/examples/cable/example_cable_ball_joints.py (2)
newton/examples/__init__.py (2)
get_asset(34-35)init(344-387)newton/_src/usd/utils.py (1)
get_mesh(638-850)
newton/examples/cable/example_cable_twist.py (1)
newton/_src/utils/import_utils.py (1)
transform(139-140)
newton/_src/solvers/vbd/rigid_vbd_kernels.py (1)
newton/_src/sim/joints.py (1)
JointType(20-47)
🪛 Ruff (0.14.10)
newton/examples/cable/example_cable_ball_joints.py
488-488: Avoid specifying long messages outside the exception class
(TRY003)
newton/examples/cable/example_cable_fixed_joints.py
518-522: Avoid specifying long messages outside the exception class
(TRY003)
newton/_src/solvers/vbd/solver_vbd.py
647-650: Avoid specifying long messages outside the exception class
(TRY003)
653-655: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (30)
README.md (1)
197-228: LGTM!The new Cable Examples section follows the established documentation pattern consistently with other example sections. The table structure, image links, and
uv runcommands all match the existing conventions.newton/examples/cable/example_cable_bend_damping.py (5)
52-53: LGTM!The clarified docstring accurately documents that
edge_indicesare not used byadd_rod(), helping prevent confusion for future readers.
167-167: LGTM!The generalized comment avoids hardcoding a specific cable count and stays accurate regardless of how
bend_damping_valuesis modified.
194-200: Good fix for complete kinematic body behavior.Previously, only mass was zeroed which would make the body kinematic for translation but potentially allow rotational drift. Setting both
body_inertiaandbody_inv_inertiato zero ensures the anchor body is fully fixed.
228-234: Good improvement for device consistency.Using
self.solver.device.is_cudainstead ofwp.get_device().is_cudaensures the CUDA graph capture decision is based on the solver's actual device rather than the global default, which could differ if explicitly configured.
325-332: LGTM!Using
self.cable_bodies_list[0]with index accessors is more robust than assuming fixed body indices. This correctly validates that the free end of the cable hangs below the kinematic anchor.newton/_src/solvers/vbd/rigid_vbd_kernels.py (2)
1382-1499: Dahl envelope now derived from bend cap: matches the new per-constraint cap semantics.
Usingjoint_penalty_k_max[c_start + 1](bend slot) to computesigma_maxlines up with the “cap defines envelope” framing, and early-outs are sensible.Also applies to: 2406-2532
1800-2054:solve_rigid_body()output-to-temp + copy-back is fine; ensure aliasing can’t happen at call sites.
Because the kernel now takes bothbody_qandbody_q_new, correctness relies on them not being the same underlying array. The current pattern insolver_vbd.py(writestate_out.body_q, then copy back) is good—just worth keeping invariant/documented.Also applies to: 2056-2076
newton/_src/solvers/vbd/solver_vbd.py (3)
145-306: Constructor/API updates for non-cable joint tuning are clear.
The split between solver-global rigid caps (rigid_joint_*) vs cable per-DOF tuning (model.joint_target_ke/kd) is well documented and matches the PR objectives.Also applies to: 417-450
573-619: Joint per-constraint layout + penalty init is a solid direction.
_init_joint_constraint_layout()+_init_joint_penalty_k()correctly decouple solver constraint scalars from model DOF indexing and keepk_min <= k0 <= k_max.Also applies to: 620-750
799-829: Custom model attributes registration for Dahl params looks good.
Namespace choice (vbd) and per-joint defaults are sensible, and matches the example usage.newton/examples/cable/example_cable_bundle_hysteresis.py (1)
195-198: Nice: explicit kinematic obstacles now fully zero mass + inertia.
Settingbody_inertia/body_inv_inertiato zero in addition tobody_mass/body_inv_massavoids accidental inertial contributions from shapes and is consistent with other updated examples.Also applies to: 279-284
newton/examples/cable/example_cable_bend.py (2)
55-57: Good test robustness improvement by trackingself.cable_bodies_list.
This removes brittle assumptions about body indexing/layout and makes the connectivity and endpoint assertions much less likely to break as examples evolve.Also applies to: 161-199, 281-318
190-196: Consistent kinematic anchoring: inertia/inv_inertia zeroing is a good add.
Matches the updated kinematic-body handling pattern elsewhere in the PR.newton/examples/cable/example_cable_twist.py (1)
34-59: Fix is sound: writing twist updates into both state buffers prevents swap-related desync.
Updating bothbody_q0andbody_q1in the spin kernel matches the solver’s double-buffered state flow.Also applies to: 282-317
newton/examples/cable/example_cable_ball_joints.py (2)
76-122: Kinematic driving kernels are graph-friendly and clear.
Using device-side time + smoothstep ramp and explicitly zeroingbody_qdis a clean way to keep kinematic anchors well-behaved under contact/joint coupling.Also applies to: 417-450
349-360: Articulation creation ordering rationale is good—confirmedadd_articulation()enforces contiguity/monotonicity constraints.
Theadd_articulation()implementation at newton/_src/sim/builder.py:1237 explicitly validates that joint indices must be contiguous and monotonically increasing, raisingValueErrorif violated. The code correctly creates rod joints first, then the ball joint, ensuring the indices satisfy these constraints when passed to the builder.newton/tests/test_cable.py (8)
32-116: LGTM!The assertion helpers are well-structured with clear docstrings, appropriate tolerances, and descriptive error messages. The edge case handling for near-zero segment length in
_assert_capsule_attachments(lines 72-75) is correct.
123-132: LGTM!The kernel correctly sets the kinematic body pose and zeros the velocity.
139-154: LGTM!The geometry helper correctly creates a straight cable along +X with proper quaternion rotation from local +Z to world +X.
185-187: LGTM!Good refactoring to use the shared
_make_straight_cable_along_xhelper for consistency.
279-349: LGTM!The compute helpers correctly calculate joint errors in world space. The quaternion sign normalization at line 345 (
wp.abs(q_rel[3])) properly handles the quaternion double-cover for robust angular error measurement.
902-1023: LGTM!The BALL joint test is well-structured with comprehensive validation:
- Kinematic anchor motion with proper substep handling
- Per-substep anchor error verification
- Final state checks for surface attachment, ground clearance, and capsule connectivity
The use of
wrap_in_articulation=Falsefollowed by explicitbuilder.add_articulation([*rod_joints, j_ball])correctly groups the joints.
1025-1156: LGTM!The FIXED joint test appropriately stiffens the solver parameters (lines 1092-1099) to achieve near-rigid constraint behavior. The combined translation and rotation driving (lines 1110-1113) provides good coverage of the FIXED joint's 6-DOF constraint enforcement.
1198-1209: LGTM!Test registrations follow the established pattern.
newton/examples/cable/example_cable_fixed_joints.py (5)
37-133: LGTM!The kernels are well-designed:
compute_fixed_joint_errorcorrectly handles the quaternion double-cover withwp.abs(dq[3])for robust angular error computationmove_kinematic_anchorsuses a proper smoothstep ramp (line 110) to avoid velocity discontinuities at simulation start
135-151: LGTM!The helper functions are correct.
_auto_scaleproperly guards against zero dimensions, and_make_straight_cable_downcorrectly uses a 180° rotation about X to align local +Z with world -Z.
163-430: LGTM!The
Example.__init__is well-structured:
- Kinematic bodies are properly configured with zero mass and inertia (lines 297-300)
- Surface attachment point calculations correctly handle sphere, capsule, and box geometries (lines 312-339)
- Device-side buffers are properly allocated for CUDA graph capture compatibility
- The articulation grouping
[*rod_joints, j_fixed]correctly includes all joints in monotonically increasing order
432-486: LGTM!The simulation methods follow established patterns:
- CUDA graph capture is properly guarded for CPU fallback
- Contact recomputation every 10 substeps (via
update_step_interval) is an intentional design choice per project conventions- The
set_rigid_history_updateflag correctly synchronizes with the contact update cadence
488-523: LGTM!The
test_finalmethod provides appropriate validation of FIXED joint constraint satisfaction. The tolerances are reasonable for soft constraints.Regarding the static analysis hint (TRY003): the inline exception message is acceptable for example/test code where creating a custom exception class would be over-engineering.
…. Added comment clarity.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
newton/_src/sim/builder.py (2)
2924-2928: Clarify damping units + “target_k” semantics for cable joints.*The docstring now states stiffness is stored as
target_keand damping astarget_kd, but only stiffness has units. Consider explicitly documenting the damping units (likely[N*s/m]for stretch and[N*m*s](or torque·s/rad) for bend) and that these are PD gains consumed bySolverVBD(not material parameters).
4789-4799: Pre-scaling uses parent segment length only; confirm that’s the intended convention (and consider guardrails).You scale by
seg_len = segment_lengths[parent_idx]and applyk_eff = k / Lfor both stretch and bend. For non-uniform segments, using only the parent length is a choice; if the constraint conceptually lives “between” two segments, you may wantavg(L_parent, L_child)(or similar) instead. Also, sincestretch_stiffness/bend_stiffnesscan now be interpreted as material-like, consider validating they’re non-negative to prevent hard-to-debug solver instability.Proposed validation (optional)
@@ - seg_len = segment_lengths[parent_idx] - stretch_ke_eff = stretch_stiffness / seg_len - bend_ke_eff = bend_stiffness / seg_len + seg_len = segment_lengths[parent_idx] + if stretch_stiffness < 0.0 or bend_stiffness < 0.0: + raise ValueError( + "add_rod: stiffness values must be non-negative " + f"(stretch_stiffness={stretch_stiffness}, bend_stiffness={bend_stiffness})" + ) + stretch_ke_eff = stretch_stiffness / seg_len + bend_ke_eff = bend_stiffness / seg_lenAlso applies to: 4805-4808
newton/_src/solvers/vbd/solver_vbd.py (1)
645-658: Consider extracting exception messages to constants or a custom exception class.The static analysis flags long exception messages (TRY003). While the current messages are informative and aid debugging, extracting them to a custom exception class (e.g.,
SolverVBDConfigurationError) would improve maintainability and satisfy the linter.This is a low-priority style suggestion and can be deferred.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:03:18.367Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
🧬 Code graph analysis (1)
newton/_src/solvers/vbd/solver_vbd.py (1)
newton/_src/sim/model.py (3)
Model(100-909)ModelAttributeAssignment(31-46)ModelAttributeFrequency(49-74)
🪛 Ruff (0.14.10)
newton/_src/solvers/vbd/solver_vbd.py
650-653: Avoid specifying long messages outside the exception class
(TRY003)
656-658: Avoid specifying long messages outside the exception class
(TRY003)
705-710: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 / Run GPU Benchmarks on AWS EC2
- 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)
🔇 Additional comments (12)
newton/_src/solvers/vbd/solver_vbd.py (12)
170-176: LGTM!The new joint constraint parameters are well-organized with sensible defaults and clear inline documentation differentiating their purposes (initial seeds vs. caps for different constraint types).
417-419: LGTM!Caching an empty
Contactsobject at initialization is a clean pattern to enable particle self-contact processing when the caller passescontacts=None, avoiding repeated allocations and simplifying downstream logic.
449-454: LGTM!Non-negative clamping for stiffness and damping parameters is appropriate defensive programming.
535-542: LGTM!Graceful fallback when
model.vbdcustom attributes aren't registered ensures backward compatibility. The recommendation in the comment to callregister_custom_attributesis helpful.
576-622: LGTM!The per-constraint layout logic correctly maps joint types to their scalar constraint counts. The cumulative indexing approach is sound, and the
NotImplementedErrorfor unsupported joint types provides clear feedback.
700-724: LGTM!The bounds checking for CABLE joint DOF indices (lines 704-710) correctly validates that both required DOF entries exist in
joint_target_ke/kd, preventing out-of-bounds access. The clamping logic at lines 717-721 properly ensures the invariantk_min <= k0 <= k_max.
725-754: LGTM!The BALL and FIXED joint constraint initialization correctly uses solver-level parameters (as documented) rather than model DOF parameters, differentiating them from cable constraints. The index arithmetic for FIXED's two constraint slots is correct.
810-839: LGTM!The custom attribute registration follows the established pattern (similar to MuJoCo solver) and correctly sets per-joint frequency with model assignment. The defaults (0.5, 1.0) match the fallback values used when attributes aren't registered.
1230-1291: LGTM!The conditional logic correctly separates concerns: contact warmstart is skipped when
contacts=None(line 1232), while joint warmstart runs independently (lines 1280-1291). This allows joint constraints to function without requiring collision detection.
1374-1381: LGTM!The fallback to
_empty_contactsis a clean solution that avoids scattering None checks throughout the method. The empty contacts have zero counts, so contact processing loops execute correctly with no iterations.
1838-1906: LGTM!The contact-dependent dual updates are correctly guarded with
contacts is not None, while the joint penalty update (lines 1889-1906) correctly runs unconditionally since joints don't depend on theContactsbuffer.
1279-1291: LGTM!The
warmstart_jointskernel correctly usesjoint_constraint_countas the launch dimension (line 1289), matching the per-constraint indexing scheme. The new kernel inputs (joint_penalty_k_max,joint_penalty_k_min) are consistent with the per-constraint layout.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @newton/_src/sim/builder.py:
- Around line 2924-2928: Update the docstring entries for stretch_damping and
bend_damping to explicitly state they are unitless Rayleigh-style damping
(dimensionless) rather than physical N·s/m or N·m·s/rad; mention they are stored
in target_kd so readers know the mapping, and ensure the same unitless wording
is applied consistently in any other VBD-related parameter descriptions (e.g.,
anywhere else referencing target_kd or cable damping semantics).
🧹 Nitpick comments (1)
newton/_src/sim/builder.py (1)
4637-4648:add_rod()uses “EA/EI-style” stiffness whileadd_joint_cable()uses per-joint stiffness — consider de-confusing the API.
The docstring correctly explains the conversion and the “pre-scaled by segment length” behavior. The main footgun is that the same parameter names (stretch_stiffness,bend_stiffness) now imply different units/meaning depending on which helper is used. If you want to keep signature stability, consider adding clearer alias kwargs (e.g.,axial_stiffness_EA,bending_stiffness_EI) and deprecating the ambiguous names over time, or at least add a prominent warning sentence in the docstring header. Also, please explicitly note damping is unitless per VBD convention. Based on learnings, damping should be described as unitless Rayleigh-style.Also applies to: 4673-4675
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/sim/builder.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 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/_src/sim/builder.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2026-01-13T03:11:40.555Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.555Z
Learning: In newton/_src/sim/builder.py, follow VBD conventions: treat radians as dimensionless. Prefer wording 'torque per radian' instead of writing 'N·m/rad' in any docs, strings, or generated text within this file. Ensure consistent usage in docstrings, comments, and documentation strings associated with this module.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/sim/builder.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). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (1)
newton/_src/sim/builder.py (1)
4792-4811: Length pre-scaling implementation is correct/safe, but please verify the chosen length (parent segment) and edge-case behavior.
Division byseg_lenis guarded by the earliersegment_length > 0.0check, and wiring*_ke_effintoadd_joint_cable()is coherent. Two things worth verifying: (1) using the parent segment length for the joint between segments (vs avg/min of adjacent) matches the intended discrete rod model; and (2) extremely small segments will explodek_eff—consider a small lower bound or warning if that’s a realistic input.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
newton/_src/sim/builder.py (1)
4641-4652: Rod parameter semantics are clearer; consider explicitly repeating “unitless Rayleigh-style damping” here too.
add_rod()now clearly documents EA/EI-like inputs and the “divide by segment length” conversion to effective per-joint stiffness, which is great. One small follow-up: sincestretch_damping/bend_dampingare ultimately passed into VBD cable joints, it may be worth explicitly stating here as well that they’re treated as dimensionless Rayleigh-style coefficients inSolverVBD(mirroringadd_joint_cable()), to prevent users from assuming physical units. Based on learnings, keeping that phrasing consistent helps avoid confusion.Also applies to: 4677-4679
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/sim/builder.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 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/_src/sim/builder.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2026-01-13T03:11:40.555Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.555Z
Learning: In newton/_src/sim/builder.py, follow VBD conventions: treat radians as dimensionless. Prefer wording 'torque per radian' instead of writing 'N·m/rad' in any docs, strings, or generated text within this file. Ensure consistent usage in docstrings, comments, and documentation strings associated with this module.
Applied to files:
newton/_src/sim/builder.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). (4)
- GitHub Check: Run GPU Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
🔇 Additional comments (2)
newton/_src/sim/builder.py (2)
2924-2932: Docstring semantics look consistent with VBD conventions (unitless damping; “torque per radian” wording).Nice clarification that cable
*_dampingis dimensionless (Rayleigh-style) underSolverVBD, and that bend stiffness is expressed as torque per radian (treating radians as dimensionless). Based on learnings, this matches the repo’s VBD damping/unit conventions and preferred wording.
4796-4815: No double-scaling risk; the pre-scaling is intentional and documented.The pre-scaled stiffness values (
stretch_ke_eff = stretch_stiffness / seg_len,bend_ke_eff = bend_stiffness / seg_len) are used directly by the VBD solver without further normalization. The rigid_vbd_kernels.py at line 955 explicitly documents: "For rods created byModelBuilder.add_rod(), this stiffness is pre-scaled by dividing by segment length." The solver directly assigns these pre-scaled values tojoint_k_max_np(solver_vbd.py lines 712–714) without re-normalizing. Using the parent segment length for the joint is the correct convention for rod topology (connects end of parent segment to start of child).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
newton/examples/cable/example_cable_bundle_hysteresis.py (1)
308-320: LGTM!The Dahl friction parameters are correctly authored via model.vbd attributes after finalization, aligning with the updated SolverVBD API that reads these per-joint values when
rigid_enable_dahl_friction=True.The
hasattrcheck is defensive—sinceregister_custom_attributesis called earlier,model.vbdshould always exist. Consider using an assertion instead if you want to catch misconfiguration earlier:assert hasattr(self.model, "vbd"), "VBD custom attributes not registered"
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/examples/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_pile.py
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 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/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_pile.py
📚 Learning: 2025-12-03T09:33:27.083Z
Learnt from: camevor
Repo: newton-physics/newton PR: 1019
File: newton/_src/solvers/kamino/examples/sim/example_sim_basics_cartpole.py:388-392
Timestamp: 2025-12-03T09:33:27.083Z
Learning: In the Kamino ModelBuilder class located at newton/_src/solvers/kamino/core/builder.py, the attributes `gravity`, `bodies`, `joints`, `collision_geoms`, and `physical_geoms` are implemented as property decorated methods, so they should be accessed as attributes (e.g., `builder.gravity`) rather than called as methods (e.g., `builder.gravity()`).
Applied to files:
newton/examples/cable/example_cable_bundle_hysteresis.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/examples/cable/example_cable_bundle_hysteresis.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/cable/example_cable_bundle_hysteresis.pynewton/examples/cable/example_cable_pile.py
🧬 Code graph analysis (1)
newton/examples/cable/example_cable_bundle_hysteresis.py (1)
newton/_src/solvers/vbd/solver_vbd.py (2)
SolverVBD(97-2006)register_custom_attributes(812-839)
⏰ 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 / Run GPU Benchmarks on AWS EC2
- 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)
🔇 Additional comments (10)
newton/examples/cable/example_cable_pile.py (1)
147-148: LGTM!The contact margin of 0.05 is appropriate for this dense cable pile (roughly 4× the cable radius of 0.012), enabling earlier contact detection to improve stability. The value matching
segment_lengthmay be intentional—consider adding a brief comment if so, to clarify the relationship for future readers.newton/examples/cable/example_cable_bundle_hysteresis.py (9)
37-93: LGTM!The triangle wave obstacle motion kernel handles all three phases (load, hold, release) correctly. The phase-based logic using device-side time is well-suited for CUDA graph capture.
95-101: LGTM!Separating time advancement into its own single-threaded kernel avoids race conditions and maintains graph-capture compatibility.
104-159: LGTM!The bundle position calculation correctly handles various cable counts with proper geometric spacing to prevent overlap.
195-206: LGTM!The builder setup correctly registers VBD-specific custom attributes before adding bodies/joints, ensuring the Dahl friction parameters will be available on the finalized model.
222-245: LGTM!Cable bundle creation with proper alignment and consistent orientations looks correct.
281-285: LGTM!Explicitly zeroing both mass and inertia tensors ensures the obstacles behave as proper kinematic bodies.
322-352: LGTM!State initialization and kinematics setup are correct. Using a device-side time array for graph-capture compatibility is the right approach.
363-418: LGTM!The simulation loop correctly handles:
- Obstacle kinematic motion with separate time advancement
- Configurable contact recomputation cadence
- Rigid history update synchronization
- Proper state swapping
443-466: LGTM!The main block provides useful CLI arguments for experimenting with Dahl friction parameters, with sensible defaults and proper conditional enabling.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/solvers/vbd/solver_vbd.py (1)
1411-1492: AttributeError whencontacts=Nonewith particles present.Both branches unconditionally access
contacts.soft_contact_particle,contacts.soft_contact_count, etc., butcontactscan beNoneper the updated signature. This will crash with anAttributeError.The docstring at lines 1120-1122 states particle self-contact should work when
contacts=None, but this code path doesn't guard against it.Suggested approach
Guard the contact-dependent kernel inputs, or split the kernel launch to handle self-contact separately from body-particle contacts:
# Accumulate contact forces if self.particle_enable_self_contact: + # Body-particle soft contact data (use fallback when contacts=None) + soft_contact_particle = contacts.soft_contact_particle if contacts is not None else wp.empty(0, dtype=wp.int32, device=self.device) + soft_contact_count = contacts.soft_contact_count if contacts is not None else wp.zeros(1, dtype=wp.int32, device=self.device) + soft_contact_max = contacts.soft_contact_max if contacts is not None else 0 + # ... similar for other contact fields wp.launch( kernel=accumulate_contact_force_and_hessian, ...Alternatively, create a cached empty
Contactsobject (as the comment on line 417 suggests) and use it as a fallback.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/solvers/vbd/solver_vbd.py
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 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/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:03:18.367Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
🪛 Ruff (0.14.10)
newton/_src/solvers/vbd/solver_vbd.py
649-652: Avoid specifying long messages outside the exception class
(TRY003)
655-657: Avoid specifying long messages outside the exception class
(TRY003)
704-709: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 / Run GPU Benchmarks on AWS EC2
- 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)
🔇 Additional comments (7)
newton/_src/solvers/vbd/solver_vbd.py (7)
575-620: LGTM!The per-constraint layout computation is correct. The prefix-sum for
joint_constraint_startand dimension mapping for CABLE/BALL/FIXED/FREE joint types align with the documented behavior. TheNotImplementedErrorfor unsupported types provides a clear failure mode.
622-759: LGTM!The per-constraint penalty initialization correctly handles CABLE (using model DOF-indexed
joint_target_ke/kd), BALL (single linear constraint), and FIXED (linear + angular constraints). The invariantk_min <= k0 <= k_maxis properly enforced viamin()clamping.The detailed RuntimeError messages (flagged by static analysis) are appropriate here since they provide actionable debugging information for model configuration errors.
809-838: LGTM!The
register_custom_attributesclassmethod correctly registersdahl_eps_maxanddahl_tauas per-joint model attributes in thevbdnamespace. The defaults (0.5 and 1.0) are consistent with the fallback in_init_dahl_params.
534-541: LGTM!Safe fallback pattern for reading Dahl parameters from
model.vbd. Thegetattr/hasattrchecks ensure backward compatibility when the custom attributes aren't registered.
1229-1290: LGTM!The contact warmstart logic is correctly guarded with
if contacts is not None:(line 1231), while joint warmstart (lines 1277-1290) runs independently of contacts. This matches the intended behavior where joint constraints work without external collision data.
1751-1902: LGTM!The rigid body iteration correctly guards contact-related operations with
if contacts is not None:checks, while the joint penalty dual update (lines 1884-1902) runs unconditionally as expected.
170-248: LGTM!The new constructor parameters and their docstrings clearly document the distinction between cable constraints (using per-DOF
model.joint_target_ke/kd) and non-cable constraints like BALL/FIXED (using the solver-levelrigid_joint_*parameters).
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/solvers/vbd/solver_vbd.py (1)
1456-1490: Same null dereference issue in non-self-contact path.The
elsebranch (lines 1468-1483) also accessescontacts.soft_contact_*without checking ifcontactsisNone. This affects particles interacting with rigid bodies even when self-contact is disabled.
🤖 Fix all issues with AI agents
In @newton/_src/solvers/vbd/solver_vbd.py:
- Around line 1409-1454: The kernel launch in solve_particle_iteration
(accumulate_contact_force_and_hessian) unconditionally reads fields on contacts
(e.g., contacts.soft_contact_particle, contacts.soft_contact_count,
contacts.soft_contact_max, contacts.soft_contact_shape,
contacts.soft_contact_body_pos, contacts.soft_contact_body_vel,
contacts.soft_contact_normal) and crashes when contacts is None; fix by guarding
or providing a minimal fallback: before the launch, if contacts is None create
an empty/zeroed Contacts-like structure with appropriate empty arrays and zero
counts (or skip passing body-particle contact arguments and the body-particle
accumulation when contacts is None), then use that fallback for the kernel
inputs so accumulate_contact_force_and_hessian always receives valid arrays and
counts; ensure all references in the surrounding code (the kernel inputs list
and any later uses) use the fallback variable instead of raw contacts.
🧹 Nitpick comments (1)
newton/_src/sim/builder.py (1)
4671-4682: Potential confusion:add_rod()stiffness inputs use EA/EI-style units whileadd_joint_cable()uses per-joint units.
The docstring explains the conversion, but the parameter names are identical (stretch_stiffness,bend_stiffness) across APIs with different physical interpretations, which is easy to misuse. Consider emphasizing this difference even more explicitly (e.g., first line of each param: “For rods, interpret as EA/EI…”).Also applies to: 4707-4709
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
🧰 Additional context used
📓 Path-based instructions (4)
newton/_src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
The
newton/_src/directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)
Files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
newton/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Any user-facing class/function/object added under
_srcmust be exposed via the public Newton API through re-exports
Files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use prefix-first naming for classes:ActuatorPD,ActuatorPID(notPDActuator,PIDActuator)
Use prefix-first naming for methods:add_shape_sphere()(notadd_sphere_shape())
Method names must usesnake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values
Files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
CLI arguments must use
kebab-case(e.g.,--use-cuda-graph, not--use_cuda_graph)
Files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
🧠 Learnings (10)
📓 Common learnings
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: Repo: newton-physics/newton — VBD damping semantics
- Joint/cable damping in SolverVBD is Rayleigh-style and treated as unitless; same convention applies across VBD constraints.
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:638-648
Timestamp: 2025-09-22T21:03:18.367Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently only supports additive updates, meaning it assumes n_coords == n_dofs. Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work.
📚 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/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: In finalize() validation logic (e.g., newton/_src/sim/builder.py and similar files), keep the validation lightweight. Do not add union-find/connected-components validation for loop joints. The current behavior—allowing non-articulated joints when the child is in any articulation—is acceptable. Avoid overengineering here.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2025-09-22T21:03:39.624Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/_src/sim/ik/ik_lbfgs_optimizer.py:739-752
Timestamp: 2025-09-22T21:03:39.624Z
Learning: The L-BFGS optimizer in newton/_src/sim/ik/ik_lbfgs_optimizer.py currently intentionally only supports additive updates (assuming n_coords == n_dofs). Velocity space integration for joints with mismatched coordinate/DOF dimensions (like free/ball joints) is planned for future work and should not be flagged as an issue in current reviews.
Applied to files:
newton/_src/sim/builder.pynewton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2026-01-13T03:11:40.555Z
Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.555Z
Learning: In newton/_src/sim/builder.py, follow VBD conventions: treat radians as dimensionless. Prefer wording 'torque per radian' instead of writing 'N·m/rad' in any docs, strings, or generated text within this file. Ensure consistent usage in docstrings, comments, and documentation strings associated with this module.
Applied to files:
newton/_src/sim/builder.py
📚 Learning: 2026-01-12T10:11:39.312Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py:6061-6091
Timestamp: 2026-01-12T10:11:39.312Z
Learning: Repo: newton-physics/newton PR: 1314
File: newton/_src/sim/builder.py (ModelBuilder.finalize)
Learning: The project prefers to keep finalize() validation lightweight; do not add union-find/connected-components validation for loop joints. Current behavior (allow non-articulated joints when the child is in any articulation) is acceptable; avoid overengineering here.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering, despite the general warp documentation suggesting copies are made by default.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-12-12T08:45:51.908Z
Learnt from: nvtw
Repo: newton-physics/newton PR: 1221
File: newton/examples/example_sdf.py:277-287
Timestamp: 2025-12-12T08:45:51.908Z
Learning: In Newton examples (e.g., example_sdf.py), computing contacts once per frame and reusing them across multiple substeps is an intentional design choice, not a bug. The contacts are computed before the substep loop and intentionally kept constant throughout all substeps within that frame.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-26T05:58:21.013Z
Learnt from: WenchaoHuang
Repo: newton-physics/newton PR: 835
File: newton/_src/solvers/style3d/collision/collision.py:49-53
Timestamp: 2025-09-26T05:58:21.013Z
Learning: In newton/_src/solvers/style3d/collision/collision.py, the broad_phase buffers (broad_phase_ee, broad_phase_ef, broad_phase_vf) created with wp.array() don't need zero-initialization because they follow a write-first-read-later pattern: BVH query kernels in frame_begin() populate these buffers completely (write-only operations), and collision kernels only read from them afterward in accumulate_contact_force(), ensuring no undefined reads occur.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
📚 Learning: 2025-09-22T21:08:31.901Z
Learnt from: dylanturpin
Repo: newton-physics/newton PR: 806
File: newton/examples/ik/example_ik_franka.py:121-123
Timestamp: 2025-09-22T21:08:31.901Z
Learning: In the newton physics framework, when creating warp arrays for IK solver joint variables using wp.array(self.model.joint_q, shape=(1, coord_count)), the resulting array acts as a reference/pointer to the original model's joint coordinates, so updates from the IK solver automatically reflect in the model's joint_q buffer used for rendering.
Applied to files:
newton/_src/solvers/vbd/solver_vbd.py
🧬 Code graph analysis (1)
newton/_src/solvers/vbd/solver_vbd.py (4)
newton/_src/sim/contacts.py (1)
Contacts(23-114)newton/_src/sim/model.py (3)
Model(100-909)ModelAttributeAssignment(31-46)ModelAttributeFrequency(49-74)newton/_src/sim/builder.py (2)
ModelBuilder(70-6870)CustomAttribute(355-519)newton/_src/solvers/vbd/rigid_vbd_kernels.py (1)
to(496-504)
🪛 Ruff (0.14.10)
newton/_src/solvers/vbd/solver_vbd.py
647-650: Avoid specifying long messages outside the exception class
(TRY003)
653-655: Avoid specifying long messages outside the exception class
(TRY003)
702-707: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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 Tests / Run GPU Unit Tests on AWS EC2
- GitHub Check: Run GPU Benchmarks / Run GPU Benchmarks on AWS EC2
- GitHub Check: run-newton-tests / newton-unittests (ubuntu-latest)
- GitHub Check: run-newton-tests / newton-unittests (windows-latest)
🔇 Additional comments (9)
newton/_src/sim/builder.py (1)
2953-2961: Docstring: damping + “torque per radian” wording is consistent with VBD conventions.
The updated descriptions correctly call out VBD’s dimensionless (Rayleigh-style)target_kdsemantics and avoid “N·m/rad” in favor of “torque per radian”. Based on learnings, this is the right convention to document here.newton/_src/solvers/vbd/solver_vbd.py (8)
25-34: LGTM!The expanded imports for
Contacts,JointType,ModelAttributeAssignment, andModelAttributeFrequencycorrectly support the new per-constraint joint handling and custom attribute registration.
170-176: LGTM!The new per-constraint AVBD parameters (
rigid_joint_linear_ke,rigid_joint_angular_ke,rigid_joint_linear_kd,rigid_joint_angular_kd) are well-documented and correctly distinguish between cable and non-cable joint constraint handling.
532-539: LGTM!The Dahl parameter loading safely checks for the
model.vbdnamespace and falls back to sensible defaults when custom attributes aren't registered. This provides good backward compatibility.
573-618: LGTM!The per-constraint layout logic correctly maps joint types to constraint dimensions (CABLE: 2, BALL: 1, FIXED: 2, FREE: 0) and builds cumulative start indices. The
NotImplementedErrorfor unsupported joint types provides clear guidance.
698-721: LGTM!The CABLE joint constraint initialization correctly validates DOF bounds before accessing
model.joint_target_ke/kd. The constraint cap and damping values are properly sourced from the model's per-DOF arrays.
807-836: LGTM!The
register_custom_attributesclassmethod correctly declares per-joint Dahl friction parameters in thevbdnamespace. The frequency (JOINT) and assignment (MODEL) are appropriate, and defaults align with the fallback values in_init_dahl_params.
1094-1121: LGTM!The
stepmethod signature correctly reflects thatcontactsis optional. The docstring clearly explains that rigid contact handling is skipped whencontacts=Nonewhile particle self-contact (if enabled) remains independent.
1945-1958: LGTM!The
finalize_rigid_bodiesmethod correctly uses the per-constraint arrays (joint_constraint_start,joint_penalty_k_max) for the Dahl state update kernel.
gdaviet
left a comment
gdaviet
left a comment
There was a problem hiding this comment.
Looks good to me -- it's great that you've been able to implement the FIXED and BALL joints by reusing the CABLE energies!
Minor question about the parametrization -- its there a particular reason for using stiffnesses rather than elastic moduli in Pa (then computing EA / EI according to the cable thickness) ? Or is it mostly for consistency with other solvers ?
Thanks @gdaviet for the great input and I am also curious about your preference. :-) |
Right, it is true that cables are rarely homogeneous, so if that's our main use case, let's keep using stiffnesses. Maybe we can have an helper for the case of homogeneous rods that computes the stiffness from elastic moduli. |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for BALL and FIXED joint types in the SolverVBD system, enabling cable attachment to kinematic anchors. The implementation introduces per-constraint penalty parameter tracking, Dahl friction model configuration via custom model attributes, and comprehensive test coverage for the new joint types.
Changes:
- Extended SolverVBD to support BALL (point-to-point) and FIXED (welded) joints alongside existing CABLE joints
- Refactored joint constraint handling from per-DOF to per-scalar-constraint indexing for more flexible penalty adaptation
- Moved Dahl friction parameters from solver constructor to model custom attributes for better scene authoring
- Added per-segment length normalization for rod stiffness values in
ModelBuilder.add_rod()
Reviewed changes
Copilot reviewed 12 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| newton/_src/solvers/vbd/solver_vbd.py | Core solver changes: constraint layout refactor, BALL/FIXED joint support, custom attribute registration |
| newton/_src/solvers/vbd/rigid_vbd_kernels.py | Kernel updates: unified isotropic constraint force/hessian evaluation, BALL/FIXED constraint implementations |
| newton/_src/sim/builder.py | Rod builder enhancements: automatic stiffness scaling, input validation, improved documentation |
| newton/tests/test_cable.py | Comprehensive test suite: connectivity, joint attachment verification, helper utilities |
| newton/examples/cable/*.py | Visual test examples for BALL/FIXED joints with kinematic anchors, improved existing examples |
| README.md | Documentation: added cable examples showcase |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Thanks @gdaviet! I created an issue (#1360), and I will follow up soon. |
@eric-heiden @gdaviet Would adding a rod util under _src/utils/ work? and add rod helpers there? |
2 participants
Description
Newton Migration Guide
Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.
docs/migration.rstis up-to dateBefore your PR is "Ready for review"
newton/tests/test_examples.py)pre-commit run -aSummary by CodeRabbit
Release Notes
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.