Implicit MPM: Graph capture and two-way coupling example#940
Conversation
📝 WalkthroughWalkthroughMakes colliders body- and material-aware across the implicit MPM solver, rewrites SDF/rasterization to accept per-body transforms/velocities, moves mass/rigidity computations to body-based volumes and cell_volume, extends rheology to color-based per-color launches and rigidity coupling, and updates examples/tests including a two-way coupling demo. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Loop
participant Coll as Rasterized Collisions
participant MPM as ImplicitMPM Solver
participant Rigid as XPBD Rigid Solver
App->>Coll: rasterize_colliders(body_q, body_qd)
Coll-->>MPM: node fields (ids, sdf, grad, vel, material_id)
MPM->>MPM: assemble mass/rigidity using body_mass, body_inv_inertia, cell_volume
MPM->>App: collect_collider_impulses() → (impulses, positions, collider_ids)
App->>Rigid: compute_body_forces(dt, impulses, positions, body_ids, body_q, body_com)
Rigid->>Rigid: integrate rigid dynamics (XPBD)
App->>Rigid: subtract_body_force(...) -- optional sand revert
App->>MPM: project_outside_collider(updated body_q, body_qd)
MPM->>MPM: solve_rheology(color-based GS/Jacobi, rigidity coupling)
MPM->>App: updated state → repeat
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Focused review items:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
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/implicit_mpm/solve_rheology.py (1)
1233-1278: Replace device.is_capturing with Stream.is_capturing
Warp’s Device class does not exposeis_capturing; usewp.get_stream().is_capturingto guard capture logic—or simplify by always usingwp.ScopedCapture.
🧹 Nitpick comments (9)
pyproject.toml (1)
28-30: Avoid pinning a dev-only warp-lang in core [project] unless strictly requiredPinning to a specific dev build can break installs and downstream environments. Prefer a minimum constraint (>=) here and pin in a lockfile or a dev/test extra, unless you truly need this exact build for API compatibility.
Based on learnings
If exact pinning is required, please confirm this version is published on the configured "nvidia" index and documented in the migration guide.
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1313-1314: Minor cleanups: precedence buglet and unnecessary int cast
- Make intended precedence explicit for _NULL_COLOR.
- Drop redundant int().
-_NULL_COLOR = 1 << 31 - 1 # color for null nodes. make sure it is sorted last +_NULL_COLOR = (1 << 31) - 1 # color for null nodes. make sure it is sorted last - current_sum = int(0) + current_sum = 0Also applies to: 1324-1324
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
64-69: Clarify body_com/body_index docs (per body vs per collider).Collider.body_com is indexed by body_id (as used below), not by collider index. Update the docstring to avoid confusion.
- """Body center of mass of each collider""" + """Per-body center of mass; indexed by body_id"""
445-446: Typos in docstrings (“scalaing”).Fix spelling.
- cell_volume: Grid cell volume as scalaing factor to node_volumes. + cell_volume: Grid cell volume as scaling factor to node_volumes.Also applies to: 508-509
492-516: Docstring parameters out of date.The doc still mentions collider_coms/collider_inv_inertia; update to body_q, body_mass, body_inv_inertia, cell_volume to match the signature.
newton/examples/mpm/example_mpm_multi_material.py (1)
186-188: Minor: simplify mass computation.cell_volume is already a scalar; np.prod(cell_volume) is redundant.
- mass = np.prod(cell_volume) * density + mass = cell_volume * densitynewton/examples/mpm/example_mpm_2way_coupling.py (3)
61-83: Typo: substract_body_force → subtract_body_force (and update call sites).Keeps API clean and avoids future confusion.
-@wp.kernel -def substract_body_force( +@wp.kernel +def subtract_body_force( @@ - substract_body_force, + subtract_body_force,Also applies to: 305-317
303-317: Prefer using a scalar dim for launch.Use shape[0] for clarity.
- dim=self.sand_state_0.body_q.shape, + dim=self.sand_state_0.body_q.shape[0],
347-353: Unused variable: cid.Rename to underscore to satisfy Ruff RUF059.
- impulses, pos, cid = self.mpm_solver.collect_collider_impulses(self.sand_state_0) + impulses, pos, _ = self.mpm_solver.collect_collider_impulses(self.sand_state_0)
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(24 hunks)newton/_src/solvers/implicit_mpm/solve_rheology.py(11 hunks)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py(37 hunks)newton/examples/mpm/example_mpm_2way_coupling.py(1 hunks)newton/examples/mpm/example_mpm_anymal.py(7 hunks)newton/examples/mpm/example_mpm_grain_rendering.py(1 hunks)newton/examples/mpm/example_mpm_granular.py(2 hunks)newton/examples/mpm/example_mpm_multi_material.py(1 hunks)newton/tests/test_implicit_mpm.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-27T23:33:32.527Z
Learnt from: shi-eric
PR: newton-physics/newton#660
File: pyproject.toml:28-28
Timestamp: 2025-08-27T23:33:32.527Z
Learning: In Python projects, it's normal and correct to have different warp-lang version specifications across files: pyproject.toml should specify minimum compatible versions using >= for flexibility, while specific environments like asv.conf.json can pin exact versions using == for reproducibility. These serve different purposes and should not be "aligned" to the same version.
Applied to files:
pyproject.toml
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/mpm/example_mpm_anymal.py
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
📚 Learning: 2025-09-18T07:05:56.836Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/solve_rheology.py:969-986
Timestamp: 2025-09-18T07:05:56.836Z
Learning: In newton/_src/solvers/implicit_mpm/solve_rheology.py, transposed_strain_mat parameter cannot be None - the type signature was corrected to reflect this guarantee, eliminating the need for None checks when accessing transposed_strain_mat.offsets.
Applied to files:
newton/_src/solvers/implicit_mpm/solve_rheology.py
🧬 Code graph analysis (8)
newton/tests/test_implicit_mpm.py (1)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
collect_collider_impulses(1532-1546)
newton/examples/mpm/example_mpm_granular.py (3)
newton/examples/mpm/example_mpm_anymal.py (2)
capture(203-214)step(247-277)newton/examples/mpm/example_mpm_grain_rendering.py (2)
simulate(73-83)step(85-87)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (2)
step(1470-1483)project_outside(1490-1530)
newton/examples/mpm/example_mpm_grain_rendering.py (2)
newton/examples/diffsim/example_diffsim_drone.py (1)
state(447-448)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
enrich_state(1430-1467)
newton/examples/mpm/example_mpm_2way_coupling.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (4)
setup_collider(1102-1198)enrich_state(1430-1467)collect_collider_impulses(1532-1546)step(1470-1483)newton/_src/sim/builder.py (2)
add_ground_plane(2512-2532)add_shape_box(2567-2604)
newton/examples/mpm/example_mpm_multi_material.py (2)
newton/examples/diffsim/example_diffsim_drone.py (1)
state(447-448)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
enrich_state(1430-1467)
newton/examples/mpm/example_mpm_anymal.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (2)
SolverImplicitMPM(1367-2398)setup_collider(1102-1198)newton/examples/mpm/example_mpm_granular.py (1)
capture(89-97)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (6)
newton/_src/solvers/implicit_mpm/solve_rheology.py (1)
YieldParamVec(39-51)newton/_src/sim/model.py (2)
Model(29-649)state(440-474)newton/_src/geometry/types.py (2)
GeoType(25-64)Mesh(113-297)newton/_src/utils/mesh.py (4)
create_plane_mesh(405-446)create_sphere_mesh(22-83)create_capsule_mesh(86-153)create_cylinder_mesh(183-280)newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
rasterize_collider(282-342)allot_collider_mass(423-478)build_rigidity_matrix(481-570)newton/_src/sim/state.py (1)
State(21-112)
newton/_src/solvers/implicit_mpm/solve_rheology.py (3)
newton/_src/viewer/gl/gui.py (1)
is_capturing(261-265)newton/examples/mpm/example_mpm_anymal.py (1)
capture(203-214)newton/examples/mpm/example_mpm_granular.py (1)
capture(89-97)
🪛 Ruff (0.14.0)
newton/tests/test_implicit_mpm.py
86-86: Unpacked variable collider_ids is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
newton/examples/mpm/example_mpm_2way_coupling.py
347-347: Unpacked variable cid is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
625-625: Avoid specifying long messages outside the exception class
(TRY003)
1174-1174: Avoid specifying long messages outside the exception class
(TRY003)
1324-1324: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
2353-2353: 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (18)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
512-518: LGTM: world_position integrandUseful for collider rasterization; clear, minimal, and correct.
newton/examples/mpm/example_mpm_grain_rendering.py (1)
55-60: LGTM: state creation and enrichment moved to model.state() + enrich_stateMatches solver API expectations and keeps rendering data consistent.
newton/examples/mpm/example_mpm_granular.py (1)
90-97: ScopedCapture is record-only; no state reset needed
ScopedCapture records kernel launches into a CUDA graph without executing them, so callingsimulate()inside the capture block won’t advance simulation state.newton/_src/solvers/implicit_mpm/rasterized_collisions.py (6)
81-107: Rigid transform path in collision_sdf looks correct; please verify indexing and local/world usage.
- Local query via x_local and cp, then rotate grad/vel back to world and add rigid contribution using R*(closest_point - body_com[body_id]): consistent with body-local COM.
- Ensure collider.body_com is sized per-body and accessed by body_id (not per-collider).
Also applies to: 127-141
151-162: Volume accumulation kernel LGTM.Atomic accumulation gated by collider_id >= 0; scales by cell_volume as intended.
179-188: Division by collider volume: safe given registration, but add assertion if cheap.You divide by collider_volumes[collider_id]. This is fine because nodes tagged with a collider imply positive accumulated volume.
If helpful, add a debug assert to catch unexpected zeros in dev builds. Based on learnings.
321-327: Guard for OUTSIDE sentinel is correct.No uninitialized reads occur since you early-return for inactive nodes and still write collider_sdf.
389-414: Rigidity blocks: verify W definition/sign and indexing.
- W = skew(b_pos + Rcom - x) and IJtm[2i] = -m * I_inv_world @ W: sign convention seems intentional; please confirm it matches v_node = v + ω×(x - com).
- Accesses of body_inv_inertia[body_id] and body_mass[body_id] assume per-body arrays; OK if guaranteed by setup_collider.
567-569: Sparse mm composition looks good.bsr_mm with z=diag(Iphi) and beta=1.0 builds the final rigidity matrix in place with headroom via max_new_nnz.
newton/examples/mpm/example_mpm_multi_material.py (1)
80-85: State enrichment at init: good.Ensures required per-state fields exist before stepping.
Confirm that when collider_body_count == 0 the solver passes valid zero-length body_q/body_qd arrays to kernels (not None).
newton/examples/mpm/example_mpm_2way_coupling.py (2)
37-59: compute_body_forces: physics and indexing look correct.Impulse-to-force (impulse/dt) and torque r×F about COM are correct; mapping collider_ids→body_ids is guarded.
Ensure collider_ids values align with collider_body_id ordering from setup_collider (mesh index → body index).
247-256: Capture gating is fine; grid is fixed.CUDA graph capture over simulate() is appropriate since MPM uses a fixed grid in this example.
newton/_src/solvers/implicit_mpm/solve_rheology.py (6)
531-533: apply_stress_delta: correct orientation and update.delta_impulse = delta_stress @ B_local; velocity update scaled by inv_mass_matrix is consistent.
535-576: Per-color GS apply: structure looks sound.color-bucket iteration with stride=launch_dim avoids conflicts; J/D usage consistent.
Confirm color_offsets/indices guarantee no shared velocity nodes within a color.
579-651: Per-color GS solve: immediate delta application is correct.Local strain compute, solve, in-place stress update, and velocity update via cw_div(delta_stress, D) are consistent.
1049-1055: Color launch configuration: sensible defaults.SM-based block count and fixed block_dim=64 are fine; recorded launches reuse params per color.
Also applies to: 1058-1077, 1081-1116
1163-1182: Collider contact staging is correct.Warm-start impulse application, optional rigidity feedback, and per-iteration solve are sensibly ordered.
Also applies to: 1183-1197
833-837: Stopping condition: strict min-iterations check is OK.Using cur_it > min_iterations avoids premature termination at exactly min_iterations.
4a9486f to
c3408b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
newton/examples/mpm/example_mpm_anymal.py (1)
137-139: Non-fixed grids: avoid 0 active cells (use -1 for “unlimited”)Setting max_active_cell_count = 0 for non-fixed grids can disable partitions. Use -1 to mean “unlimited”.
- mpm_options.grid_padding = 50 if grid_type == "fixed" else 0 - mpm_options.max_active_cell_count = 1 << 15 if grid_type == "fixed" else 0 + mpm_options.grid_padding = 50 if grid_type == "fixed" else 0 + mpm_options.max_active_cell_count = (1 << 15) if grid_type == "fixed" else -1newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
930-932: Confirmcreate_cone_meshis exported innewton.utils.This code calls
newton.utils.create_cone_mesh, which was previously flagged as not being exported. Ensure the export has been added to avoid runtimeAttributeError.Based on learnings: See past review comments for the required fix in
newton/_src/utils/__init__.py.
🧹 Nitpick comments (17)
newton/_src/viewer/camera.py (1)
165-181: Guard against zero-length direction to avoid NaNsRare, but at extreme FOV/pitch combos direction could approach zero; add a safe fallback on normalization.
- direction = front + right * u * alpha * aspect_ratio + up * v * alpha - direction = direction / float(np.linalg.norm(direction)) + direction = front + right * u * alpha * aspect_ratio + up * v * alpha + norm = float(np.linalg.norm(direction)) + direction = (direction / norm) if norm > 0.0 else frontAlso, confirm the y-origin convention (Line 175 comment) matches the viewer's pixel coords to avoid flipped rays.
newton/_src/solvers/implicit_mpm/solve_rheology.py (5)
537-576: Color-launch striding is safe; add early-exit micro-guardThe per-color striding with launch_dim avoids races. For efficiency on small colors, consider bailing early when the thread’s first offset exceeds the color span.
- i = wp.tid() - color_beg = color_offsets[color] + i - color_end = color_offsets[color + 1] + i = wp.tid() + color_beg = color_offsets[color] + i + color_end = color_offsets[color + 1] + if color_beg >= color_end: + returnAlso, consider renaming inv_mass_matrix arg to inv_mass (or inv_w) to match use.
833-834: Stopping condition runs strictly more than min_iterationsYou changed to cur_it > min_iterations. If the intent is “at least min_iterations,” use >= for clarity and to avoid an extra iteration.
- stop = (wp.sqrt(residual[0]) < residual_threshold and cur_it > min_iterations) or cur_it > max_iterations + stop = (wp.sqrt(residual[0]) < residual_threshold and cur_it >= min_iterations) or cur_it > max_iterations
839-875: Rigidity coupling math is correct; tighten docstring and aliasing commentsD, J, IJtm decomposition with y += (D + J@IJtm) Δv matches the documented relation. Update the docstring to state that rigidity_mat is a tuple (D, J, IJtm). Add a brief note that prev_collider_velocity is reused as Δv (via array_axpy) to clarify aliasing.
-def apply_rigidity_matrix(rigidity_mat, prev_collider_velocity, collider_velocity, delta_body_qd): - """Apply collider rigidity feedback to the current collider velocities. +def apply_rigidity_matrix(rigidity_mat, prev_collider_velocity, collider_velocity, delta_body_qd): + """Apply collider rigidity feedback to the current collider velocities. @@ - Args: - rigidity_mat: Block-sparse rigidity operator (D + J @ IJtm) returned by - ``build_rigidity_matrix``. + Args: + rigidity_mat: Tuple of block-sparse matrices (D, J, IJtm) s.t. (D + J @ IJtm) is the coupling operator. @@ - # compute velocity delta, store in prev_collider_velocity + # compute velocity delta, store in prev_collider_velocity (reused as Δv)
1049-1079: GPU launch sizing: good defaults; add CPU-safe dim capSM-based sizing is reasonable. On CPU, consider dim = max(1, min(color_launch_dim, color_offsets[color+1]-color_offsets[color])) when setting per-color params to reduce idle threads, though correctness is unaffected.
Also applies to: 1082-1084, 1087-1117
1203-1225: Conditional graph capture is robust; minor init nitInitializing iteration_and_condition with ones works, but setting iteration=0 and condition=1 is slightly clearer.
- iteration_and_condition = fem.borrow_temporary(temporary_store, shape=(2,), dtype=int) - iteration_and_condition.array.fill_(1) + iteration_and_condition = fem.borrow_temporary(temporary_store, shape=(2,), dtype=int) + iteration_and_condition.array[0:1].fill_(0) # iteration + iteration_and_condition.array[1:2].fill_(1) # conditionVerbose summary block is helpful.
Also applies to: 1236-1273, 1274-1281
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (4)
81-142: Compose rigid-body velocity with mesh velocity instead of overwritingAfter rotating sdf_vel to world, you overwrite it with rigid velocity. If mesh vertices carry non-zero velocities, we should add both.
- sdf_vel = wp.quat_rotate(b_rot, sdf_vel) - sdf_grad = wp.quat_rotate(b_rot, sdf_grad) + sdf_vel = wp.quat_rotate(b_rot, sdf_vel) + sdf_grad = wp.quat_rotate(b_rot, sdf_grad) @@ - sdf_vel = b_v + wp.cross(b_w, wp.quat_rotate(b_rot, closest_point - collider.body_com[body_id])) + sdf_vel = (b_v + + wp.cross(b_w, wp.quat_rotate(b_rot, closest_point - collider.body_com[body_id])) + + sdf_vel)Also confirm collider.body_com is in body-local coordinates (it appears so).
178-187: Density from body mass: add defensive clamp (optional)Division by collider_volumes[collider_id] is expected to be safe given how ids are assigned. If you want belt-and-suspenders:
[Based on learnings]- return body_mass[body_id] / collider_volumes[collider_id] + vol = collider_volumes[collider_id] + return body_mass[body_id] / wp.max(vol, 1.0e-12)(Learning indicates zero-volume shouldn’t happen due to integrand logic.)
206-271: project_outside_collider: world-frame query and rigid velocity integration LGTMThe ACTIVE-bit test using bitwise complement works but is cryptic; consider explicit check for readability:
- if ~particle_flags[i] & newton.ParticleFlags.ACTIVE: + if (particle_flags[i] & newton.ParticleFlags.ACTIVE) == 0:
482-519: Return (D, J, IJtm) and dimensions consistentcols_of_blocks = 2*body_count matches the two (angular, linear) columns per body; diagonal D from Iphi is fine. Add a short note in the docstring that the tuple is returned, matching solve_rheology.apply_rigidity_matrix.
- Returns: - A tuple of ``warp.sparse.BsrMatrix`` (D, J, IJtm) representing the rigidity coupling operator (D + J @ IJtm) + Returns: + Tuple of ``warp.sparse.BsrMatrix`` (D, J, IJtm); the effective operator is (D + J @ IJtm).Also applies to: 521-566, 567-568
newton/examples/mpm/example_mpm_2way_coupling.py (5)
61-83: Typo: substract_body_force → subtract_body_forceRename for clarity and consistency.
-@wp.kernel -def substract_body_force( +@wp.kernel +def subtract_body_force( @@ - wp.launch( - substract_body_force, + wp.launch( + subtract_body_force,
160-171: Minor: avoid np.prod on scalarcell_volume is already scalar.
- mass = float(np.prod(cell_volume) * density) + mass = float(cell_volume * density)
250-257: Graph capture: consider mirroring granular example’s even-substeps guardFor MPM graph capture stability, some examples require even substeps. If applicable here, add the guard to reduce capture-time surprises.
303-317: Kernel dim: prefer .shape[0] for 1D launch clarityPassing dim=self.sand_state_0.body_q.shape works, but using the explicit length is clearer and avoids accidental multi-dim semantics.
- dim=self.sand_state_0.body_q.shape, + dim=self.sand_state_0.body_q.shape[0],
347-353: Unused variable ‘cid’cid isn’t used in render; prefix with underscore or drop the unpack to satisfy linters.
- impulses, pos, cid = self.mpm_solver.collect_collider_impulses(self.sand_state_0) + impulses, pos, _cid = self.mpm_solver.collect_collider_impulses(self.sand_state_0)newton/examples/mpm/example_mpm_anymal.py (1)
210-215: Mirror granular example’s even-substeps check for sand graph captureAdd the even-substeps guard to match example_mpm_granular and prevent capture inconsistencies.
- if wp.get_device().is_cuda and self.mpm_solver.grid_type == "fixed": - with wp.ScopedCapture() as capture: - self.simulate_sand() - self.sand_graph = capture.graph + if wp.get_device().is_cuda and self.mpm_solver.grid_type == "fixed": + if self.sim_substeps % 2 != 0: + wp.utils.warn("Sim substeps should be even for graph capture of MPM step") + else: + with wp.ScopedCapture() as capture: + self.simulate_sand() + self.sand_graph = capture.graphnewton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1328-1328: Minor: Remove unnecessaryintcast.The value
0is already an integer, soint(0)is redundant. Simply use0.Apply this diff:
- current_sum = int(0) + current_sum = 0As per static analysis hints: RUF046
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 4a9486f and c3408b2d469dc61a7685d8d7afd5b7a082680ec4.
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(25 hunks)newton/_src/solvers/implicit_mpm/solve_rheology.py(13 hunks)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py(37 hunks)newton/_src/viewer/camera.py(2 hunks)newton/examples/mpm/example_mpm_2way_coupling.py(1 hunks)newton/examples/mpm/example_mpm_anymal.py(7 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-27T23:33:32.527Z
Learnt from: shi-eric
PR: newton-physics/newton#660
File: pyproject.toml:28-28
Timestamp: 2025-08-27T23:33:32.527Z
Learning: In Python projects, it's normal and correct to have different warp-lang version specifications across files: pyproject.toml should specify minimum compatible versions using >= for flexibility, while specific environments like asv.conf.json can pin exact versions using == for reproducibility. These serve different purposes and should not be "aligned" to the same version.
Applied to files:
pyproject.toml
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/mpm/example_mpm_anymal.py
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🧬 Code graph analysis (4)
newton/_src/solvers/implicit_mpm/solve_rheology.py (3)
newton/_src/sim/builder.py (1)
color(4027-4068)newton/examples/mpm/example_mpm_anymal.py (1)
capture(203-214)newton/examples/mpm/example_mpm_granular.py (1)
capture(89-97)
newton/examples/mpm/example_mpm_anymal.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (2)
SolverImplicitMPM(1386-2432)setup_collider(1103-1202)newton/examples/mpm/example_mpm_granular.py (1)
capture(89-97)
newton/examples/mpm/example_mpm_2way_coupling.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (4)
setup_collider(1103-1202)enrich_state(1449-1486)collect_collider_impulses(1551-1565)step(1489-1502)newton/_src/sim/builder.py (2)
add_ground_plane(2512-2532)add_shape_box(2567-2604)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (6)
newton/_src/solvers/implicit_mpm/solve_rheology.py (1)
YieldParamVec(39-51)newton/_src/sim/model.py (1)
state(440-474)newton/_src/geometry/types.py (2)
GeoType(25-64)Mesh(113-297)newton/_src/utils/mesh.py (4)
create_plane_mesh(405-446)create_sphere_mesh(22-83)create_capsule_mesh(86-153)create_cylinder_mesh(183-280)newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
rasterize_collider(281-343)allot_collider_mass(423-478)build_rigidity_matrix(481-568)newton/_src/sim/state.py (1)
State(21-112)
🪛 Ruff (0.14.0)
newton/examples/mpm/example_mpm_2way_coupling.py
347-347: Unpacked variable cid is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
628-628: Avoid specifying long messages outside the exception class
(TRY003)
1328-1328: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
2387-2387: 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (27)
newton/_src/solvers/implicit_mpm/solve_rheology.py (3)
531-533: Dimensionality in BΔv update looks correctUsing delta_stress @ local_strain_mat_values[b] (vec6 @ mat63 → vec3) is consistent with BSR shapes; scaling by inv_mass_matrix per node is correct.
Please confirm inv_mass_matrix indeed represents 1/m (not 1/volume) here to avoid unit drift. If it’s 1/volume, rename for clarity in kernel signatures to prevent misuse later.
579-651: GS local-stress path aligns with coloring; residual bookkeeping OKImmediate application of wp.cw_div(delta_stress, D) through apply_stress_delta keeps GS consistent. Storing delta_correction for residuals is good.
1164-1169: Temporary allocations sized from J avoid shape mismatchesAllocating delta_body_qd with shape=J.shape[1] is a good safeguard; copying prev_collider_velocity before first apply maintains Δv semantics.
Also applies to: 1184-1184
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (7)
64-68: Body-aware collider fields are a good additionbody_index and body_com align the rasterized path with rigid bodies.
151-161: Volume accumulation kernel is straightforwardAtomic add per collider id scales node_volumes by cell_volume as intended.
197-204: Dynamic check tied to body mass is clearTreating mass<=0 as kinematic matches solver expectations.
281-345: rasterize_collider handles OUTSIDE sentinel and per-node materials wellGood to mark inactive with -1 friction. Velocity/normal assignments align with collision_sdf outputs.
348-365: Inverse mass uses node volume and density correctlyChecks for dynamic colliders and positive node volume prevent 1/0.
368-414: Rigidity matrices: world transforms and COM offset look right
- W built from (x_com_world - x_node) as skew is correct.
- world_inv_inertia via R @ I_inv @ R^T is correct.
- IJtm blocks scale with bc_mass and body_mass as expected.
423-465: Mass allotment pipeline is cleanTwo-phase (volume then inv mass) is clear and efficient.
newton/examples/mpm/example_mpm_2way_coupling.py (1)
50-59: Force accumulation is correct; atomic on spatial_vector assumed supportedThe force/torque about world COM is computed as F, r×F in world frame. Confirm wp.atomic_add supports spatial_vector on your target Warp version; if not, accumulate per-component.
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (16)
512-518: LGTM: Clean world position integrand.The new
world_positionintegrand correctly returns the world coordinates at sample points and is properly used in the rasterization step (line 1829-1834).
539-542: LGTM: Grid control options for capture mode.The new
grid_paddingandmax_active_cell_countoptions are well-documented and have sensible defaults for enabling graph capture with fixed grids.
574-617: LGTM: Scratchpad fields for dynamic colliders.The added fields (
grid,collider_velocity,collider_normal_field, etc.) properly support dynamic collider handling and lazy initialization.
618-640: LGTM: Clean basis space initialization.The separation of basis space creation from function space creation is a good design that supports grid reuse in capture mode.
706-754: LGTM: Smart field rebinding for dynamic grids.The guard against redundant recreation (lines 713-717) and the rebinding logic properly support both fixed and dynamic grid modes while avoiding unnecessary allocations.
760-808: LGTM: Consistent strain field management.The strain field management mirrors the velocity field pattern and properly handles both creation and rebinding scenarios.
809-846: LGTM: Temporaries allocation with coloring support.The addition of
max_colorsand conditional color array allocation properly supports the Gauss-Seidel coloring feature.
1103-1202: LGTM: Simplified collider setup with auto-mesh creation.The updated
setup_collidersignature simplifies two-way coupling by acceptingcollider_body_idsdirectly and auto-generating meshes from bodies when not provided. The length consistency checks (lines 1148, 1160-1163) ensure proper pairing.Note: This is a breaking API change as mentioned in the PR objectives.
1445-1487: LGTM: State enrichment for warmstart and coupling.The addition of
ws_impulse_fieldandws_stress_fieldfor warmstarting, plus automatic body array creation when needed, properly supports the new coupling features.
1551-1565: Breaking change:collect_collider_impulsesnow returns 3-tuple.The return signature changed from a single array to
(impulses, positions, collider_ids). Ensure calling code is updated accordingly.The negative sign on impulse values (line 1562) correctly represents action-reaction pairs for two-way coupling.
1722-1770: LGTM: Grid capture support with fixed/sparse/dense modes.The conditional grid rebuild (lines 1730-1739) enables graph capture for fixed grids, while sparse grids use Nanogrid partitions directly and dense grids build explicit active cell partitions (lines 1699-1720). This design cleanly supports all three modes.
1828-1867: LGTM: Enhanced collider rasterization with normals.The expanded rasterization now computes world positions (lines 1828-1834), rasterizes collider data (lines 1836-1853), computes gradient fields (lines 1855-1861), and normalizes normals (lines 1863-1867). This provides complete collider boundary information for the solver.
2156-2199: LGTM: Warmstart integration in solver loop.The warmstart fields are properly loaded before solving (line 2156) and saved after solving (line 2199) to provide initial guesses for the next step, improving convergence.
2296-2346: LGTM: Robust warmstart with non-conforming grids.The use of
NonconformingField(lines 2307-2308, 2316-2317) allows warmstart values to transfer correctly even when the grid changes between steps. The scatter kernels (lines 2328-2346) save the current step's values for the next warmstart.
905-941: All unhandledGeoTypeenum values correctly raiseNotImplementedError.The function properly handles all 7 mesh-compatible geometry types (PLANE, SPHERE, CAPSULE, CYLINDER, CONE, BOX, MESH). The four unhandled types—HFIELD (terrain), ELLIPSOID, SDF (signed distance field), and NONE (placeholder)—do not require mesh generation and correctly raise
NotImplementedErrorvia the fallback at line 941. The implementation is correct.
653-663: Conservative node count is an intentional, acknowledged design choice—no action needed.The code explicitly flags this as "overly conservative" (line 659), confirming developers are aware of the memory trade-off. This is sound practice for pre-allocated buffers: the conservative upper bound
MAX_NODES_PER_ELEMENT * element_count()ensures safety at the cost of modest over-allocation. The estimate is tied tomax_active_cell_count, further reducing practical wastefulness. No performance issues or optimization TODOs were found.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
905-942: Verify utility mesh factories are imported/exported (create_cone_mesh, create_box_mesh).This code assumes newton.utils exposes create_cone_mesh/create_box_mesh. A missing export will raise AttributeError at runtime. Prior comment already flagged cone; include box in the check.
#!/bin/bash # Verify mesh factories exist and are re-exported from newton.utils rg -nP --type=py '\bdef\s+create_(cone|box)_mesh\b' newton/_src | sed -n '1,120p' rg -nP --type=py 'from \.mesh import .*create_(cone|box)_mesh' newton/_src/utils/__init__.py -n
🧹 Nitpick comments (4)
newton/examples/mpm/example_mpm_2way_coupling.py (3)
61-71: Rename kernel and fix launch dim typing.Typos hinder grep and maintenance; and using a tuple for dim is fragile. Rename to subtract_body_force and use explicit length at the call site.
-@wp.kernel -def substract_body_force( +@wp.kernel +def subtract_body_force( dt: float, body_q: wp.array(dtype=wp.transform), body_qd: wp.array(dtype=wp.spatial_vector), body_f: wp.array(dtype=wp.spatial_vector), body_inv_inertia: wp.array(dtype=wp.mat33), body_inv_mass: wp.array(dtype=float), body_q_res: wp.array(dtype=wp.transform), body_qd_res: wp.array(dtype=wp.spatial_vector), ):And at launch (Line 305):
- substract_body_force, - dim=self.sand_state_0.body_q.shape, + subtract_body_force, + dim=self.sand_state_0.body_q.shape[0],
290-299: Avoid launching over 1<<20 slots; use active count from collect_collider_impulses.Currently compute_body_forces runs over a huge buffer and branches on -1. Track n_colliders and launch only that many threads.
def collect_collider_impulses(self): collider_impulses, collider_impulse_pos, collider_impulse_ids = self.mpm_solver.collect_collider_impulses( self.sand_state_0 ) self.collider_impulse_ids.fill_(-1) - n_colliders = min(collider_impulses.shape[0], self.collider_impulses.shape[0]) + n_colliders = min(collider_impulses.shape[0], self.collider_impulses.shape[0]) + self._active_impulse_count = n_colliders self.collider_impulses[:n_colliders].assign(collider_impulses[:n_colliders]) self.collider_impulse_pos[:n_colliders].assign(collider_impulse_pos[:n_colliders]) self.collider_impulse_ids[:n_colliders].assign(collider_impulse_ids[:n_colliders])And in simulate() (Lines 262-275):
- wp.launch( + wp.launch( compute_body_forces, - dim=self.collider_impulse_ids.shape[0], + dim=self._active_impulse_count, inputs=[Also applies to: 262-275
238-246: Right-size impulse buffers or assert capacity.Hard-coding max_nodes = 1<<20 risks OOM or silent truncation. At minimum, assert returned collider_impulses.shape[0] <= buffer size; ideally, allocate to exact size or grow-on-demand.
self.collider_impulses = wp.zeros(max_nodes, dtype=wp.vec3, device=self.model.device) ... - self.collect_collider_impulses() + self.collect_collider_impulses() + assert self.collider_impulses.shape[0] >= getattr(self, "_active_impulse_count", 0), \ + "Insufficient collider impulse buffer capacity"newton/examples/mpm/example_mpm_anymal.py (1)
210-215: Optional: Fall back gracefully when CUDA graph capture is unsupported.Consider warning when capture fails or when sim_substeps invalid for capture (as done in example_mpm_granular.py), to aid users.
Also applies to: 272-276
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between c3408b2d469dc61a7685d8d7afd5b7a082680ec4 and b8dd5d807462fea35ca4515fa0a755015aa3f9c0.
📒 Files selected for processing (5)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(24 hunks)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py(37 hunks)newton/examples/mpm/example_mpm_2way_coupling.py(1 hunks)newton/examples/mpm/example_mpm_anymal.py(7 hunks)newton/tests/test_implicit_mpm.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/tests/test_implicit_mpm.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/mpm/example_mpm_anymal.py
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🧬 Code graph analysis (3)
newton/examples/mpm/example_mpm_anymal.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
setup_collider(1103-1202)newton/examples/mpm/example_mpm_granular.py (1)
capture(89-97)
newton/examples/mpm/example_mpm_2way_coupling.py (3)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (5)
SolverImplicitMPM(1392-2445)setup_collider(1103-1202)enrich_state(1455-1492)collect_collider_impulses(1557-1571)step(1495-1508)newton/examples/mpm/example_mpm_anymal.py (4)
capture(203-214)step(247-277)simulate_sand(243-245)render(308-311)newton/_src/sim/builder.py (2)
add_ground_plane(2512-2532)add_shape_box(2567-2604)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (6)
newton/examples/mpm/example_mpm_anymal.py (1)
_merge_meshes(343-362)newton/_src/sim/model.py (2)
Model(29-649)state(440-474)newton/_src/geometry/types.py (1)
GeoType(25-64)newton/_src/utils/mesh.py (4)
create_plane_mesh(405-446)create_sphere_mesh(22-83)create_capsule_mesh(86-153)create_cylinder_mesh(183-280)newton/_src/solvers/implicit_mpm/rasterized_collisions.py (4)
collider_projection_threshold(190-193)rasterize_collider(281-343)allot_collider_mass(424-479)build_rigidity_matrix(482-569)newton/examples/mpm/example_mpm_2way_coupling.py (1)
collect_collider_impulses(290-298)
🪛 Ruff (0.14.0)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
628-628: Avoid specifying long messages outside the exception class
(TRY003)
1328-1328: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
2400-2400: 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 Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (5)
newton/examples/mpm/example_mpm_anymal.py (1)
135-139: Grid capture and active-cell settings look correct.grid_type, grid_padding, and max_active_cell_count are wired as expected; non-fixed uses -1 (unlimited). Capture is gated on fixed grids.
If tests exist for sparse/dense/fixed switching, please run them to confirm graph capture path parity.
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1753-1759: Active-cell partitioning: confirmmax_cell_countinteracts as intended with domain.element_count().Given
max_node_countis derived fromdomain.element_count(), ensureExplicitGeometryPartition(..., max_cell_count=...)actually limitsdomain.element_count(); otherwise space partitions may over-allocate.newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
80-92: Rigid-body transform flow in collision_sdf looks correct.Query in local, then rotate/compose velocity and normals back to world; lever arm uses body_com. Good optimization to do world transforms after closest hit.
Double-check body_com is in body-local coordinates (builder conventions) to avoid offsets.
Also applies to: 126-142
150-161: Volume accumulation and density: confirm no zero-volume cases slip through.You rely on node tagging to imply positive volume. This matches prior learnings, but please ensure collider_volumes is >0 for all dynamic colliders before dividing.
Based on learnings
Also applies to: 424-479
368-413: Rigidity blocks J/IJtm assembly: world inertia transform and lever arm W.R @ I_inv @ R^T and W from (x_com_world - node) are consistent. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
newton/examples/mpm/example_mpm_2way_coupling.py (1)
50-62: Guardbody_indexagainst OOB before indexing body arrays.Add bounds checks for
body_indexbefore usingbody_q/body_com/body_fto avoid OOB when mappings are inconsistent.- cid = collider_ids[i] - if cid >= 0 and cid < body_ids.shape[0]: - body_index = body_ids[cid] - if body_index == -1: - return + cid = collider_ids[i] + if cid >= 0 and cid < body_ids.shape[0]: + body_index = body_ids[cid] + if body_index < 0: + return + if body_index >= body_q.shape[0] or body_index >= body_f.shape[0]: + returnnewton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1458-1458: Precedence bug:_NULL_COLORequals1<<30, not2**31-1.Parenthesize the shift.
-_NULL_COLOR = 1 << 31 - 1 # color for null nodes. make sure it is sorted last +_NULL_COLOR = (1 << 31) - 1 # color for null nodes; ensure it sorts last
🧹 Nitpick comments (5)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (2)
101-107: Remove redundant casts and keep types simple.
global_face_id = int(0)is unnecessary; use a plain integer literal. Same forcollider_id = int(_GROUND_COLLIDER_ID)andmaterial_id = int(_GROUND_COLLIDER_MATERIAL_INDEX).Apply this diff:
- global_face_id = int(0) + global_face_id = 0 ... - collider_id = int(_GROUND_COLLIDER_ID) + collider_id = _GROUND_COLLIDER_ID - material_id = int(_GROUND_COLLIDER_MATERIAL_INDEX) + material_id = _GROUND_COLLIDER_MATERIAL_INDEX
248-253: Prefix unused unpacked var to appease linters.
collider_idisn’t used here; rename to_collider_idto silence RUF059.- sdf, sdf_gradient, sdf_vel, collider_id, material_id = collision_sdf(pos_adv, collider, body_q, body_qd) + sdf, sdf_gradient, sdf_vel, _collider_id, material_id = collision_sdf(pos_adv, collider, body_q, body_qd)newton/examples/mpm/example_mpm_granular.py (1)
115-129: Update test AABB to match collider placement.The cube test uses center z=0.5 but the collider is placed at z=0.9 and
hz=0.6. Adjust the AABB or derive it from the samexform/extents used for the collider to avoid false failures.newton/examples/mpm/example_mpm_2way_coupling.py (1)
261-277: Avoid launching over the entire preallocated buffer; use the active count.You allocate 1<<20 slots but only the first N contain valid collider data. Track
n_collidersfromcollect_collider_impulses()and launch kernels withdim=n_collidersto cut overhead.- self.collect_collider_impulses() + self.collect_collider_impulses() + self.active_impulse_count = min( + int(self.collider_impulses.shape[0]), + int((self.collider_impulse_ids != -1).sum().numpy()) + ) ... - wp.launch( + wp.launch( compute_body_forces, - dim=self.collider_impulse_ids.shape[0], + dim=self.active_impulse_count, inputs=[newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1469-1476: Drop redundant cast incompute_color_offsets.
current_sum = int(0)→current_sum = 0.- current_sum = int(0) + current_sum = 0
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between b8dd5d807462fea35ca4515fa0a755015aa3f9c0 and 307ff8b48dd36d4fc8f008689ab0e9ca7708082f.
📒 Files selected for processing (5)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(19 hunks)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py(38 hunks)newton/examples/mpm/example_mpm_2way_coupling.py(1 hunks)newton/examples/mpm/example_mpm_anymal.py(7 hunks)newton/examples/mpm/example_mpm_granular.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🧬 Code graph analysis (4)
newton/examples/mpm/example_mpm_2way_coupling.py (3)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (4)
setup_collider(1136-1343)enrich_state(1602-1639)collect_collider_impulses(1703-1717)step(1642-1655)newton/examples/mpm/example_mpm_anymal.py (4)
capture(203-214)step(247-277)simulate_sand(243-245)render(308-311)newton/_src/sim/builder.py (2)
add_ground_plane(2512-2532)add_shape_box(2567-2604)
newton/examples/mpm/example_mpm_anymal.py (3)
newton/_src/sim/builder.py (1)
body_count(515-519)newton/_src/geometry/flags.py (1)
ShapeFlags(30-42)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
setup_collider(1136-1343)
newton/examples/mpm/example_mpm_granular.py (3)
newton/_src/sim/builder.py (4)
add_shape_box(2567-2604)ModelBuilder(67-4483)ShapeConfig(141-193)add_ground_plane(2512-2532)newton/examples/mpm/example_mpm_2way_coupling.py (3)
capture(249-255)simulate(257-287)step(323-329)newton/examples/mpm/example_mpm_anymal.py (2)
capture(203-214)step(247-277)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (3)
newton/_src/geometry/types.py (2)
GeoType(25-64)Mesh(113-297)newton/_src/utils/mesh.py (4)
create_plane_mesh(405-446)create_sphere_mesh(22-83)create_capsule_mesh(86-153)create_cylinder_mesh(183-280)newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
rasterize_collider(273-335)allot_collider_mass(416-471)build_rigidity_matrix(474-561)
🪛 Ruff (0.14.0)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
102-102: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
250-250: Unpacked variable collider_id is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
633-633: Avoid specifying long messages outside the exception class
(TRY003)
986-986: Unused function argument: body_index
(ARG001)
1198-1200: Avoid specifying long messages outside the exception class
(TRY003)
1202-1204: Avoid specifying long messages outside the exception class
(TRY003)
1237-1237: Avoid specifying long messages outside the exception class
(TRY003)
1469-1469: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
2548-2548: 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (6)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (1)
180-189: OK to omit zero‑volume guards in density.Division by
collider_volumes[collider_id]is safe here because volumes are atomically accumulated only for registered collider nodes, guaranteeing non‑zero totals. Based on learningsnewton/examples/mpm/example_mpm_granular.py (1)
92-101: Nice: even‑substeps guard for graph capture.Even substeps ensure state swapping returns to the same buffer across captured replays. LGTM.
newton/examples/mpm/example_mpm_2way_coupling.py (1)
299-316: Matchdimto collider body count.Good: subtract only for bodies coupled to MPM by using
self.sand_state_0.body_q.shape. LGTM.newton/examples/mpm/example_mpm_anymal.py (2)
210-215: Separate capture for sand when grid is fixed — good pattern.Graph capture gating on
grid_type=="fixed"avoids shape changes. LGTM.
143-145: Correct default formax_active_cell_count(non‑fixed → unlimited).Using
-1for non‑fixed grids prevents empty partitions. Thanks for addressing this.newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
1882-1893: Grid rebuild logic is sound.Reusing grid for
grid_type=="fixed"and rebuilding otherwise aligns with graph capture assumptions.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
newton/_src/sim/builder.py (1)
3700-3706: Handle radius_mean=None correctly and align jitter doc with implementation.
- Bug: When radius_mean is None, radii becomes an object array and adding radius_std breaks. Pass None through to add_particles unless a numeric mean is provided.
- Doc says “maximum random offset,” but code produces offsets in [-jitter/2, +jitter/2]. Clarify docs or double the factor.
Apply this diff:
- jitter (float): Maximum random offset to apply to each particle position. + jitter (float): Random offset magnitude; actual offset is sampled in [-jitter/2, +jitter/2] per axis. @@ - radii = np.full(points.shape[0], fill_value=radius_mean) - if radius_std > 0.0: - radii += rng.standard_normal(radii.shape) * radius_std + radii = None + if radius_mean is not None: + radii = np.full(points.shape[0], fill_value=float(radius_mean), dtype=float) + if radius_std > 0.0: + radii += rng.standard_normal(radii.shape) * float(radius_std) @@ - radius=radii.tolist(), + radius=(radii.tolist() if radii is not None else None),Also applies to: 3711-3726
♻️ Duplicate comments (3)
newton/examples/mpm/example_mpm_granular.py (1)
40-56: Collider gating fix looks good.Explicitly skipping when options.collider == "none" prevents accidental shape creation. Matches earlier feedback.
newton/examples/mpm/example_mpm_anymal.py (1)
141-145: Fixed-grid defaults are sensible; non-fixed uses -1 for unlimited.This addresses the earlier issue where 0 disabled active cells on non-fixed grids.
newton/examples/mpm/example_mpm_twoway_coupling.py (1)
56-68: Add upper-bound guard for body_index to avoid OOB.A mismatched mapping can still yield body_index >= body_q.size. Guard before indexing.
Apply:
- if cid >= 0 and cid < body_ids.shape[0]: - body_index = body_ids[cid] - if body_index == -1: + if cid >= 0 and cid < body_ids.shape[0]: + body_index = body_ids[cid] + if body_index == -1: return + if body_index < 0 or body_index >= body_q.shape[0] or body_index >= body_f.shape[0]: + return
🧹 Nitpick comments (5)
newton/_src/sim/builder.py (1)
3731-3745: Nit: doc-code clarity for jitter range.If you prefer to keep the doc as “maximum offset,” change the implementation to use (rng.random(points.shape) - 0.5) * (2.0 * jitter). Otherwise the doc tweak above is sufficient.
newton/examples/mpm/example_mpm_granular.py (1)
121-129: Gate cube-specific test to only run when collider == "cube".Currently this runs regardless of chosen collider. Store options on self and conditionally assert.
Apply:
- def __init__(self, viewer, options): + def __init__(self, viewer, options): + self.options = options @@ - newton.examples.test_particle_state( + if getattr(self, "options", None) and self.options.collider == "cube": + newton.examples.test_particle_state( self.state_0, "all particles are outside the cube", lambda q, qd: not newton.utils.vec_inside_limits(q, cube_lower, cube_upper), - ) + )newton/examples/mpm/example_mpm_twoway_coupling.py (3)
200-217: Avoid launching over the full 1<<20 buffer; track actual collider count.Store the count in collect_collider_impulses and launch compute_body_forces for only n valid entries.
Apply:
def collect_collider_impulses(self): collider_impulses, collider_impulse_pos, collider_impulse_ids = self.mpm_solver.collect_collider_impulses( self.sand_state_0 ) self.collider_impulse_ids.fill_(-1) - n_colliders = min(collider_impulses.shape[0], self.collider_impulses.shape[0]) + n_colliders = min(collider_impulses.shape[0], self.collider_impulses.shape[0]) self.collider_impulses[:n_colliders].assign(collider_impulses[:n_colliders]) self.collider_impulse_pos[:n_colliders].assign(collider_impulse_pos[:n_colliders]) self.collider_impulse_ids[:n_colliders].assign(collider_impulse_ids[:n_colliders]) + self._impulse_count = n_colliders- wp.launch( + wp.launch( compute_body_forces, - dim=self.collider_impulse_ids.shape[0], + dim=int(getattr(self, "_impulse_count", 0)), inputs=[Also applies to: 232-241
193-199: Capturing simulate() mutates initial state; consider resetting after capture.Same concern as in granular example. Reinitialize states post-capture for deterministic startup.
274-285: Minor: silence linter by discarding unused qd in lambdas.Rename qd to
_for clarity.- lambda q, qd: q[2] > 0.45, + lambda q, _: q[2] > 0.45, @@ - lambda q, qd: q[2] > -0.05, + lambda q, _: q[2] > -0.05,
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
newton/_src/sim/builder.py(2 hunks)newton/examples/mpm/example_mpm_anymal.py(8 hunks)newton/examples/mpm/example_mpm_grain_rendering.py(2 hunks)newton/examples/mpm/example_mpm_granular.py(5 hunks)newton/examples/mpm/example_mpm_multi_material.py(2 hunks)newton/examples/mpm/example_mpm_twoway_coupling.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- newton/examples/mpm/example_mpm_multi_material.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/mpm/example_mpm_anymal.py
🧬 Code graph analysis (4)
newton/examples/mpm/example_mpm_grain_rendering.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
enrich_state(1613-1650)newton/_src/sim/builder.py (1)
add_particle_grid(3687-3760)
newton/examples/mpm/example_mpm_granular.py (2)
newton/_src/sim/builder.py (5)
add_shape_box(2536-2573)ModelBuilder(68-4506)ShapeConfig(142-194)add_ground_plane(2481-2501)add_particle_grid(3687-3760)newton/examples/mpm/example_mpm_anymal.py (2)
capture(203-214)step(247-277)
newton/examples/mpm/example_mpm_twoway_coupling.py (1)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (4)
setup_collider(1135-1342)enrich_state(1613-1650)collect_collider_impulses(1714-1728)step(1653-1666)
newton/examples/mpm/example_mpm_anymal.py (3)
newton/_src/sim/builder.py (2)
body_count(516-520)add_particle_grid(3687-3760)newton/_src/geometry/flags.py (1)
ShapeFlags(30-42)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
setup_collider(1135-1342)
🪛 GitHub Actions: Pull Request
newton/_src/sim/builder.py
[error] 1-1: Cannot resolve value for 'joint:transX_position' on prim '/joint_AD'; no authored value, no explicit default, and no solver mapping default.
[error] 1-1: Cannot resolve value for 'joint:transX_velocity' on prim '/joint_AD'; no authored value, no explicit default, and no solver mapping default.
[error] 1-1: Cannot resolve value for 'joint:nonexistent_attribute' on prim '/ant/joints/front_right_leg'; no authored value, no explicit default, and no solver mapping default.
🪛 Ruff (0.14.2)
newton/examples/mpm/example_mpm_twoway_coupling.py
279-279: Unused lambda argument: qd
(ARG005)
284-284: Unused lambda argument: qd
(ARG005)
⏰ 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (2)
newton/examples/mpm/example_mpm_grain_rendering.py (1)
55-60: State init + enrichment order looks correct.Using model.state() and enriching both states before sampling grains is the right sequence.
newton/examples/mpm/example_mpm_anymal.py (1)
81-86: Selective particle collision filtering is correct.Masking COLLIDE_PARTICLES on non-shank shapes reduces unnecessary sand contacts cleanly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
newton/examples/mpm/example_mpm_granular.py (1)
40-56: Correct collider guardExplicitly skipping "none" prevents accidental collider creation and undefined xform. Nicely fixed.
newton/examples/mpm/example_mpm_anymal.py (1)
141-145: Fixed-grid defaults are saneUsing 50 padding and 1<<15 active cells, else -1, avoids the zero-active-cells pitfall.
🧹 Nitpick comments (10)
newton/examples/mpm/example_mpm_grain_rendering.py (1)
131-144: Minor: simplify mass computation and guard radius randomness
- mass can use the scalar cell_volume directly; np.prod(cell_volume) is redundant.
- If radius_std will be used later, ensure radii remain positive.
- mass = np.prod(cell_volume) * density + mass = float(cell_volume) * densityIf you plan to pass radius_std to add_particle_grid, consider clamping radii inside the helper (see builder review) or here before passing.
newton/examples/mpm/example_mpm_multi_material.py (1)
182-201: Return particle indices: tiny readability tweakLogic is correct. For clarity, prefer the builder.particle_count property when computing begin/end.
- begin_id = len(builder.particle_q) + begin_id = builder.particle_count builder.add_particle_grid( ... ) - end_id = len(builder.particle_q) + end_id = builder.particle_count return np.arange(begin_id, end_id, dtype=int)newton/_src/sim/builder.py (1)
3731-3763: Robustness and clarity in grid generationThe implementation is correct; a few tweaks improve safety and clarity:
- Use indexing='ij' in meshgrid to make axis intent explicit.
- Force numpy dtypes to float32 to avoid accidental object dtypes from Warp types.
- Clamp radii to a small positive min to avoid negatives when radius_std > 0.
- px = np.arange(dim_x) * cell_x - py = np.arange(dim_y) * cell_y - pz = np.arange(dim_z) * cell_z - points = np.stack(np.meshgrid(px, py, pz)).reshape(3, -1).T + px = (np.arange(dim_x, dtype=np.float32) * np.float32(cell_x)) + py = (np.arange(dim_y, dtype=np.float32) * np.float32(cell_y)) + pz = (np.arange(dim_z, dtype=np.float32) * np.float32(cell_z)) + # explicit axis ordering for clarity + X, Y, Z = np.meshgrid(px, py, pz, indexing="ij") + points = np.stack((X, Y, Z), axis=-1).reshape(-1, 3) - rot_mat = wp.quat_to_matrix(rot) - points = points @ np.array(rot_mat).reshape(3, 3).T + np.array(pos) - velocity = np.broadcast_to(np.array(vel).reshape(1, 3), points.shape) + rot_mat = np.asarray(wp.quat_to_matrix(rot), dtype=np.float32).reshape(3, 3) + points = points @ rot_mat.T + np.asarray(pos, dtype=np.float32) + velocity = np.broadcast_to(np.asarray(vel, dtype=np.float32).reshape(1, 3), points.shape) - radii = np.full(points.shape[0], fill_value=radius_mean) + radii = np.full(points.shape[0], fill_value=radius_mean, dtype=np.float32) if radius_std > 0.0: - radii += rng.standard_normal(radii.shape) * radius_std + radii += rng.standard_normal(radii.shape).astype(np.float32) * np.float32(radius_std) + radii = np.clip(radii, a_min=1e-9, a_max=None)newton/examples/mpm/example_mpm_granular.py (4)
92-101: Capture: add a prerequisite check for fixed gridGraph capture is only valid with fixed grid and adequate padding/active-cell budget. Add a quick guard to warn once if padding <= 0 or max_active_cell_count <= 0 to save users from silent no-ops.
def capture(self): self.graph = None - if wp.get_device().is_cuda and self.solver.grid_type == "fixed": + if wp.get_device().is_cuda and self.solver.grid_type == "fixed": + # optional: sanity check common misconfigurations + try: + gp = getattr(self.solver, "grid_padding", None) + mac = getattr(self.solver, "max_active_cell_count", None) + if (gp is not None and gp <= 0) or (mac is not None and mac <= 0): + wp.utils.warn("Fixed-grid capture requires positive grid_padding and max_active_cell_count") + except Exception: + pass if self.sim_substeps % 2 != 0: wp.utils.warn("Sim substeps must be even for graph capture of MPM step") else:
121-123: Gate cube-specific test on selected colliderThe “outside the cube” assertion runs even for wedge/none. Consider gating to collider == "cube" (or adapting bounds per collider) to avoid confusing test semantics.
- cube_extents = wp.vec3(0.5, 2.0, 0.6) * 0.9 - cube_center = wp.vec3(0.75, 0, 0.9) + if getattr(self, "viewer", None) and hasattr(self.viewer, "options") and self.viewer.options.collider == "cube": + cube_extents = wp.vec3(0.5, 2.0, 0.6) * 0.9 + cube_center = wp.vec3(0.75, 0, 0.9) cube_lower = cube_center - cube_extents cube_upper = cube_center + cube_extents newton.examples.test_particle_state( self.state_0, "all particles are outside the cube", lambda q, qd: not newton.utils.vec_inside_limits(q, cube_lower, cube_upper), )Adjust to your options plumbing as needed.
155-168: Minor: simplify mass computationSame nit as in other examples: use the scalar cell_volume directly.
- mass=mass, + mass=float(cell_volume) * density,
197-200: CLI additions: good; please reflect in migration docsNew flags --grid-padding and --max-active-cell-count are key for fixed-grid capture; ensure docs/migration.rst calls this out for warp.sim users updating examples.
newton/examples/mpm/example_mpm_anymal.py (1)
320-333: Batch particle spawn via add_particle_gridVectorized spawn is correct; parameters align with helper. Consider passing flags=newton.ParticleFlags.ACTIVE explicitly if you ever make the helper’s default configurable.
newton/examples/mpm/example_mpm_twoway_coupling.py (2)
274-285: Unused lambda arguments in test assertions.Lines 279 and 284 define lambda functions with an unused
qdparameter. While not harmful, removing the unused parameter improves clarity.Apply this diff:
newton.examples.test_body_state( self.model, self.state_0, "all bodies are above the sand", - lambda q, qd: q[2] > 0.45, + lambda q, _qd: q[2] > 0.45, ) newton.examples.test_particle_state( self.sand_state_0, "all particles are above the ground", - lambda q, qd: q[2] > -0.05, + lambda q, _qd: q[2] > -0.05, )
56-68: Upper bounds check is optional—bounds are guaranteed by framework design for this example.The concern about verifying
body_index < body_q.shape[0]is valid in principle. However, the bounds are already guaranteed for this specific example:
- The example uses the default
setup_collider()(line 146), which generatescollider_body_idsfromrange(-1, model.body_count).- The
body_q,body_com, andbody_farrays are allocated in parallel during model building and thus always have the same length, sized tobody_count.- The static-body case (
body_index == -1) is already handled by the early return at lines 59–60.- All resulting indices are guaranteed to fall within
[0, body_count-1]by construction.Adding an explicit upper-bound check (as suggested in the original diff) is defensive programming and recommended as a best practice, even though it's not strictly necessary for this example:
cid = collider_ids[i] if cid >= 0 and cid < body_ids.shape[0]: body_index = body_ids[cid] if body_index == -1: return + if body_index >= body_q.shape[0]: + return
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
newton/_src/sim/builder.py(2 hunks)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py(43 hunks)newton/examples/mpm/example_mpm_anymal.py(8 hunks)newton/examples/mpm/example_mpm_grain_rendering.py(2 hunks)newton/examples/mpm/example_mpm_granular.py(5 hunks)newton/examples/mpm/example_mpm_multi_material.py(2 hunks)newton/examples/mpm/example_mpm_twoway_coupling.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-25T21:41:45.795Z
Learnt from: dylanturpin
PR: newton-physics/newton#634
File: newton/tests/test_examples.py:329-333
Timestamp: 2025-08-25T21:41:45.795Z
Learning: Newton examples use a centralized CLI argument parsing system in newton/examples/__init__.py. The create_parser() function defines common arguments like --device, --viewer, --output-path, and --num-frames, while init(parser) creates viewers based on parsed arguments. Individual example scripts don't need to define these flags themselves - they inherit them from the centralized system via the example_map and runpy execution.
Applied to files:
newton/examples/mpm/example_mpm_anymal.py
📚 Learning: 2025-10-27T14:07:25.228Z
Learnt from: gdaviet
PR: newton-physics/newton#940
File: newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py:1477-1482
Timestamp: 2025-10-27T14:07:25.228Z
Learning: In Warp kernels, variables must be explicitly cast (e.g., `current_sum = int(0)`) to be mutable. Without the cast, Warp interprets literal values as constants, which cannot be modified within the kernel.
Applied to files:
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
🧬 Code graph analysis (6)
newton/examples/mpm/example_mpm_grain_rendering.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
enrich_state(1613-1650)newton/_src/sim/builder.py (1)
add_particle_grid(3687-3763)
newton/examples/mpm/example_mpm_twoway_coupling.py (3)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (4)
setup_collider(1135-1342)enrich_state(1613-1650)collect_collider_impulses(1714-1728)step(1653-1666)newton/_src/sim/builder.py (2)
particle_count(530-534)add_particle_grid(3687-3763)newton/_src/sim/state.py (1)
clear_forces(64-75)
newton/examples/mpm/example_mpm_granular.py (3)
newton/_src/sim/builder.py (4)
add_shape_box(2536-2573)ShapeConfig(142-194)add_ground_plane(2481-2501)add_particle_grid(3687-3763)newton/examples/mpm/example_mpm_anymal.py (2)
capture(203-214)step(247-277)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (2)
step(1653-1666)project_outside(1673-1712)
newton/examples/mpm/example_mpm_multi_material.py (2)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
enrich_state(1613-1650)newton/_src/sim/builder.py (3)
add_particle_grid(3687-3763)flags(177-183)flags(186-191)
newton/examples/mpm/example_mpm_anymal.py (3)
newton/_src/sim/builder.py (2)
body_count(516-520)add_particle_grid(3687-3763)newton/_src/geometry/flags.py (1)
ShapeFlags(30-42)newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (1)
setup_collider(1135-1342)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (6)
newton/_src/solvers/implicit_mpm/solve_rheology.py (1)
YieldParamVec(39-51)newton/_src/sim/model.py (2)
Model(29-645)state(440-474)newton/_src/geometry/types.py (1)
GeoType(25-67)newton/_src/utils/mesh.py (4)
create_plane_mesh(405-446)create_sphere_mesh(22-83)create_capsule_mesh(86-153)create_cylinder_mesh(183-280)newton/_src/geometry/flags.py (2)
ShapeFlags(30-42)ParticleFlags(20-26)newton/_src/solvers/implicit_mpm/rasterized_collisions.py (3)
rasterize_collider(276-338)allot_collider_mass(419-474)build_rigidity_matrix(477-564)
🪛 Ruff (0.14.2)
newton/examples/mpm/example_mpm_twoway_coupling.py
279-279: Unused lambda argument: qd
(ARG005)
284-284: Unused lambda argument: qd
(ARG005)
newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py
633-633: Avoid specifying long messages outside the exception class
(TRY003)
1197-1199: Avoid specifying long messages outside the exception class
(TRY003)
1201-1203: Avoid specifying long messages outside the exception class
(TRY003)
1236-1236: Avoid specifying long messages outside the exception class
(TRY003)
1476-1476: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
2570-2570: 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (24)
newton/examples/mpm/example_mpm_grain_rendering.py (1)
55-60: State allocation/enrichment looks correctCreating states from self.model and enriching via solver is the right flow and matches enrich_state contract.
newton/examples/mpm/example_mpm_multi_material.py (1)
80-85: State creation and enrichment are consistentSwitch to self.model.state() and immediate enrichment is appropriate for the implicit MPM solver.
newton/_src/sim/builder.py (1)
3702-3726: API/doc: flags propagation clarified — goodAdding flags to add_particle_grid and documenting duplication across particles is sensible and aligns with finalize()’s flag_to_int handling.
newton/examples/mpm/example_mpm_granular.py (1)
109-113: Step: graph launch fallback is cleanFallback to simulate() when no captured graph keeps behavior consistent.
newton/examples/mpm/example_mpm_anymal.py (3)
81-86: Targeted particle-collision filteringMasking COLLIDE_PARTICLES for non-“SHANK” bodies reduces spurious sand contacts. Looks good.
Please confirm body_key naming covers all lower-leg variants across assets (e.g., “shank”, case differences).
210-215: Separate graph capture for sandGood separation of robot and sand captures; respects fixed-grid requirement.
341-341: CLI grid-type threading through to optionsPlumbing of --grid-type into Example and mpm_options is correct.
Also applies to: 358-359
newton/examples/mpm/example_mpm_twoway_coupling.py (4)
70-98: LGTM! Correct force subtraction logic.The kernel correctly computes the velocity and angular velocity changes induced by the previously applied sand forces and subtracts them from the body state. The launch dimension matches the body array size, ensuring
body_idis always in bounds.
101-191: LGTM! Proper initialization for graph-captured two-way coupling.The initialization correctly configures the MPM solver with
grid_type="fixed", sufficientgrid_padding, andmax_active_cell_countto enable graph capture. State buffers and coupling arrays are properly allocated.
242-264: Verify time step used in MPM solver.Line 261 calls
self.mpm_solver.step(..., dt=self.frame_dt), but the rigid-body simulation insimulate()usesself.sim_dtper substep (line 225). If the intent is to advance the MPM solver by one full frame, this is correct. However, if MPM should also substep, it should useself.sim_dtinstead.Confirm whether the MPM solver is meant to take a full frame step or match the rigid-body substeps. If substepping is desired, apply this diff:
- self.mpm_solver.step(self.sand_state_0, self.sand_state_0, contacts=None, control=None, dt=self.frame_dt) + self.mpm_solver.step(self.sand_state_0, self.sand_state_0, contacts=None, control=None, dt=self.sim_dt)
316-392: LGTM! Helper methods correctly emit rigid bodies and particles.The
_emit_rigid_bodiesmethod properly configures boxes with spatial offsets to avoid initial overlap. The_emit_particlesmethod correctly computes particle properties (radius, mass, cell size) from density and voxel size before callingadd_particle_grid.newton/_src/solvers/implicit_mpm/solver_implicit_mpm.py (13)
513-519: LGTM! Simple world position integrand.This new public integrand correctly returns the world-space position at a sample point, useful for rasterizing collider positions onto the grid.
540-543: LGTM! New options support graph capture and collider control.The added
grid_paddingandmax_active_cell_countoptions enable graph capture by pre-allocating a fixed grid. Thecollider_normal_from_sdf_gradientflag provides an alternative normal computation method.Also applies to: 571-573
576-872: LGTM! Scratchpad refactoring enables dynamic grid and field management.The addition of
self.gridand the newcreate_basis_spaces/ field rebinding methods allow the scratchpad to adapt to changing grid geometries and partitions. This is essential for supporting both sparse and fixed grid modes with graph capture.
1135-1342: LGTM! Major refactoring ofsetup_collideradds body-aware colliders.The updated
setup_collidermethod now acceptscollider_body_idsto map colliders to rigid bodies, enabling two-way coupling. The method properly:
- Validates that each collider has either a mesh or body ID (not both)
- Derives per-shape materials from the model when body IDs are provided
- Allows per-collider material overrides
- Assigns body-level properties (COM, mass, inertia) for compliant colliders
This is a substantial and necessary change to support the two-way coupling example.
1345-1393: LGTM! Improved bounds computation respects particle flags.The refactored
compute_boundskernel now correctly filters inactive particles when computing the bounding box, ensuring the grid only covers active particles.
1439-1547: LGTM! New utility kernels support active cell tracking and SDF-based normals.The
mark_active_cells,scatter_field_dof_values, andcollider_gradient_fieldkernels/integrands are well-designed additions that support the fixed-grid and collider normal computation features.
1593-1594: LGTM! Solver stores new options and initializes scratchpad.The solver correctly caches the new options and pre-builds the scratchpad during initialization, aligning with the refactored lifecycle.
Also applies to: 1609-1612
1613-1650: LGTM! Enhancedenrich_statesupports warmstarting and body coupling.The method now pre-allocates warmstart fields and body state arrays when needed, enabling both warmstarting across grid changes and two-way coupling with rigid bodies.
1652-1667: LGTM! Updated step lifecycle and collider impulse collection.The
stepmethod now properly rebuilds the scratchpad per step and releases temporaries afterward. Thecollect_collider_impulsesmethod correctly returns a tuple of impulse values, positions, and collider IDs, matching the updated API shown in the relevant code snippets.Also applies to: 1714-1728
1791-1892: LGTM! Grid allocation and partitioning refactored for dynamic support.The
_allocate_gridmethod now respectsparticle_flags, and the new_create_geometry_partitionmethod properly handles fixed-grid partitioning withmax_cell_count, essential for graph capture.
1893-1945: LGTM! Conditional grid rebuilding supports fixed-grid mode.The refactored
_rebuild_scratchpadmethod correctly skips grid rebuilding whengrid_type == "fixed", enabling graph capture by maintaining a stable grid across steps.
1993-2051: LGTM! Step implementation enhanced with position rasterization and warmstart save.The
_step_implmethod now:
- Interpolates world positions for collider nodes before rasterization
- Optionally computes collider normals from SDF gradients
- Properly saves warmstart fields after the solve
These changes integrate cleanly with the refactored scratchpad and field management.
Also applies to: 2376-2382
2447-2614: LGTM! Warmstart and coloring refactored for dynamic grid support.The new
_require_velocity_space_fieldsand_require_strain_space_fieldsmethods ensure fields are properly created and rebound as grids change. The_warmstart_fieldsand_save_for_next_warmstartmethods correctly transfer field data across nonconforming grids usingNonconformingFieldand scattering. The updated_compute_coloringlogic properly handles both DG and Q1 bases.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (1)
105-105: Remove unnecessaryintcast.The literal
0is already an integer, so theint()call is redundant.Apply this diff:
- global_face_id = int(0) + global_face_id = 0
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(19 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🪛 Ruff (0.14.2)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
105-105: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
🔇 Additional comments (6)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (6)
32-85: Well-structured refactoring to body- and material-aware colliders.The expanded
Colliderstruct and updated constants cleanly separate per-body, per-collider, and per-material properties. The field renamings (collider_mesh,collider_body_index,material_*arrays) provide clear semantics for the two-way coupling feature.
87-162: Excellent body-transform handling in collision_sdf.The two-stage approach—computing signed distance in local coordinates and then transforming normals/velocities to world frame—is both efficient and correct. The cross product term at line 159 properly accounts for rotational velocity, and the defensive checks for
body_id >= 0prevent issues with non-body colliders.
169-180: LGTM: Kernel-based volume accumulation.The new
collider_volumes_kernelcorrectly uses atomic operations for concurrent volume accumulation. This replaces the previous quadrature approach and aligns with the body-based mass semantics.
204-339: Collider projection and rasterization updated correctly.Both kernels properly integrate body transforms/velocities and use material-based properties (
material_friction,material_adhesion,material_projection_threshold). Thefem.OUTSIDEcheck at line 315 prevents processing nodes outside the active grid region.
362-417: Rigidity matrix assembly correctly handles body transforms and 6-DOF coupling.The skew matrix computation at line 396 correctly accounts for the vector from node to body COM, and the block-sparse structure properly maps rigid-body DOFs (angular and linear) to nodal velocities. World-space inertia transformation at line 404 is standard and correct.
477-564: build_rigidity_matrix API updated to return decomposed matrices.The function now returns a tuple
(D, J, IJtm)instead of a single matrix, giving callers more flexibility in how they use the rigidity coupling operators. The block-sparse construction correctly handles the 6-DOF rigid body representation with dimensions(N_nodes, 2*N_bodies).
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 (2)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (2)
218-238: Document missing parameters.The docstring is missing documentation for the
body_qandbody_qdparameters added on lines 211-212.Apply this diff to add the missing parameter documentation:
particle_flags: Per-particle flags (used to gate inactive particles). collider: Collider description and geometry. + body_q: Rigid body transforms. + body_qd: Rigid body velocities. dt: Timestep length. positions_out: Output particle positions.
289-310: Document missing parameters.The docstring is missing documentation for the
body_qandbody_qdparameters added on lines 277-278.Apply this diff to add the missing parameter documentation:
Args: collider: Collider description and geometry. + body_q: Rigid body transforms. + body_qd: Rigid body velocities. voxel_size: Grid voxel edge length (sets query/extrapolation band).
🧹 Nitpick comments (1)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (1)
105-105: Remove unnecessaryint()cast.The value
0is already an integer literal, so theint()cast is redundant.Apply this diff:
- global_face_id = int(0) + global_face_id = 0
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(19 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🪛 Ruff (0.14.2)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
105-105: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
⏰ 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 Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (8)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (8)
32-43: LGTM!The constant updates are appropriate for the body-aware collision system. The increased extrapolation distance and new epsilon constant for closest point normal calculation are reasonable additions.
46-85: LGTM!The restructured Collider data model effectively supports the new body- and material-aware collision system. The separation of per-collider, per-material, and per-body properties is well-designed for two-way coupling.
145-160: LGTM!The world-to-local coordinate transformation for body-attached colliders is correctly implemented. The rigid body velocity calculation at line 159 properly computes
v = v_linear + ω × r_world, where the position vector is first computed in body frame then transformed to world frame before the cross product.
169-179: LGTM!The new
collider_volumes_kernelcorrectly accumulates per-collider volumes using atomic operations to handle concurrent writes from multiple nodes to the same collider.
182-201: LGTM!The updated
collider_densityandcollider_is_dynamicfunctions correctly handle the body-based mass model, properly distinguishing between kinematic (infinite mass) and dynamic (finite mass) colliders.
340-358: LGTM!The kernel is correctly updated to use the body-based mass model, properly computing inverse masses for dynamic collider nodes.
361-415: LGTM!The rigidity matrix construction correctly implements the body-based coupling. The Jacobian blocks properly relate rigid body velocities (linear and angular) to nodal velocities using the skew-symmetric cross product matrix, and the mass-scaled inverse blocks are correctly computed in world frame.
476-563: LGTM!The refactored function correctly builds the decomposed rigidity coupling matrices (D, J, IJtm) for the body-based two-way coupling system. The change from returning a single fused matrix to a tuple of three matrices enables explicit rigid body coupling in the solver.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py(19 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-09T10:33:36.573Z
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
Applied to files:
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
🪛 Ruff (0.14.2)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py
105-105: Value being cast to int is already an integer
Remove unnecessary int call
(RUF046)
⏰ 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 (Pull Request)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (13)
newton/_src/solvers/implicit_mpm/rasterized_collisions.py (13)
32-43: LGTM! Constants are well-defined.The new
_CLOSEST_POINT_NORMAL_EPSILONand_GROUND_COLLIDER_MATERIAL_INDEXconstants improve clarity and enable material-based ground collision handling.
46-85: LGTM! Collider struct refactored for multi-body support.The transition to array-based collider data with per-body and per-material indexing enables the two-way coupling feature described in the PR objectives. The structure is well-documented with clear shape annotations.
164-166: LGTM!The collision activation threshold is correctly implemented.
169-179: LGTM! Volume accumulation is correctly implemented.The atomic accumulation per collider_id properly aggregates node volumes scaled by
cell_volume.
182-191: LGTM! Body-aware density calculation is correct.The function properly distinguishes between kinematic colliders (ground and static bodies) and dynamic colliders using body-based indexing.
194-201: LGTM! Dynamic collider detection is correct.The three-condition check properly identifies dynamic colliders that should participate in two-way coupling.
204-271: LGTM! Particle projection correctly integrates body transforms and material properties.The kernel properly queries colliders with body motion and applies material-specific friction and projection thresholds for accurate collision response.
274-337: LGTM! Collider rasterization handles inactive nodes and material properties correctly.The addition of the
fem.OUTSIDEcheck (lines 314-316) properly handles sparse grid scenarios, and material-based properties are correctly indexed and scaled.
340-358: LGTM! Inverse mass calculation uses body-based masses correctly.The kernel properly computes nodal inverse masses based on collider density derived from body masses.
361-416: LGTM! Rigidity matrix construction correctly implements body-based coupling.The Jacobian blocks (lines 387-388) now index into body DOFs rather than collider DOFs, and the kinematic relationships (lines 390-405) correctly model rigid body motion with proper mass and inertia scaling.
418-473: LGTM! Collider mass allocation properly integrates cell volume and body masses.The two-stage process (volume accumulation → inverse mass computation) correctly uses
cell_volumescaling and body-based masses.
476-563: LGTM! Rigidity matrix construction returns decomposed operators for flexible coupling.The refactored return type
(D, J, IJtm)(line 562) allows the caller to apply the rigidity couplingD + J @ IJtmin stages, and matrix dimensions correctly use2 * body_countfor body DOFs.
87-161: No issues found—body_comis correctly stored in local body coordinates.Verification confirms the velocity transformation at line 159 is correct. The
body_comarray in the Model documentation explicitly states it is in the local frame, and this is the source used by the Collider throughout the codebase. The formula computing world-frame velocity (b_v + cross(b_w, quat_rotate(b_rot, closest_point - body_com))) correctly assumes body-local coordinates.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newton/_src/sim/builder.py(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: gdaviet
PR: newton-physics/newton#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.
Learnt from: gdaviet
PR: newton-physics/newton#750
File: newton/_src/solvers/implicit_mpm/rasterized_collisions.py:133-138
Timestamp: 2025-09-09T10:33:36.573Z
Learning: In newton's implicit MPM rasterized collisions system, collider_density function doesn't need zero volume guards because collider_id being registered to a node implies non-zero volume through the collider_volumes integrand's atomic accumulation logic.
📚 Learning: 2025-08-12T17:51:37.474Z
Learnt from: nvlukasz
PR: newton-physics/newton#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/_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). (2)
- GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
- GitHub Check: Run GPU Benchmarks (Pull Request)
🔇 Additional comments (1)
newton/_src/sim/builder.py (1)
4109-4141: Efficient batch grid generation using vectorized operations.The refactored implementation effectively replaces per-particle loops with numpy batch operations. The approach correctly:
- Generates grid points via meshgrid
- Applies rotation using matrix multiplication
- Adds reproducible jitter with a seeded RNG
- Handles per-particle radius variation
- Supports the new optional flags parameter
This significantly improves performance for large grids.
…ics#940) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…ics#940) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
mpm_options.grid_padding) and predefining the maximum number of active grid cells (mpm_options.max_active_cell_count)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
New Features
Improvements
Tests
Documentation