Add Newton implicit MPM manager and demos#6089
Conversation
maxkra15
requested review from
Mayankm96,
jtigue-bdai,
kellyguo11 and
ooctipus
as code owners
github-actions
Bot
added
documentation
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot — PR #6089
Summary: Clean, well-structured integration of Newton's implicit MPM solver into the manager abstraction. The two new hooks (_register_builder_attributes, _supports_cuda_graph_capture) are additive and non-breaking. Test coverage is comprehensive with good parametrization. A few lifecycle and API-surface items below.
Findings
🟡 Missing _solver_specific_clear override — NewtonMPMManager._project_outside_colliders is a class-level flag set in _build_solver. When clear() tears down the manager and a subsequent start_simulation() rebuilds with a different MPMSolverCfg (e.g. project_outside_colliders=False), this flag will correctly be overwritten by the new _build_solver call. However, if clear() is called without a follow-up rebuild (teardown-only path), the stale flag persists on the class. Consider adding a _solver_specific_clear that resets it to False for hygiene, matching what other managers do for their solver-specific state.
🟡 Demos rely on private API _mark_transforms_dirty() — particle_pour.py calls NewtonManager._mark_transforms_dirty() directly. Since demos ship as user-facing examples, consumers may copy this pattern. Consider either promoting this to a public method (if the use-case of injecting kinematic poses before stepping is intentional API) or documenting that it's internal.
🔵 TemporaryStore is not retained as a class attribute — The TemporaryStore() created in _build_solver is passed to the solver but not stored on the manager. This works (the solver holds the reference), but if the solver's internal lifetime semantics ever change, the store could be GC'd. A defensive cls._temporary_store = ... line would make ownership explicit.
🔵 test_initialize_solver_populates_canonical_state — MPM particles lack custom_attributes — The add_particle_grid(...) call in the test doesn't pass custom_attributes={...} for MPM material params. This works because the solver apparently handles missing per-particle material data gracefully, but the test would be a more faithful smoke-test of the production path if it included at least mpm:young_modulus.
🔵 Changelog and docs look good — The RST additions clearly describe the new hooks and when they fire. No issues there.
Verdict: Solid work. The architecture cleanly extends the existing manager pattern. The two warnings above are minor lifecycle hygiene items; the suggestions are purely defensive. No blocking issues found.
Reviewed at: 8b76353
| NewtonManager._needs_collision_pipeline = False | ||
| cls._project_outside_colliders = solver_cfg.project_outside_colliders | ||
|
|
||
| @classmethod |
There was a problem hiding this comment.
🟡 Lifecycle hygiene: Consider adding a override that resets . While will overwrite it on rebuild, a teardown-only leaves this stale. Consistent with how other managers clean up their solver-specific state.
|
|
||
| if not stage.GetPrimAtPath("/World/TableVisual").IsValid(): | ||
| table_cfg = sim_utils.CuboidCfg( | ||
| size=(2.0 * TABLE_HALF_EXTENTS[0], 2.0 * TABLE_HALF_EXTENTS[1], 2.0 * TABLE_HALF_EXTENTS[2]), |
There was a problem hiding this comment.
🟡 Private API in user-facing demo: is a single-underscore private method. Since demos are often copied by users, consider either promoting this to public API or adding a comment explaining it is internal and subject to change.
| NewtonManager._solver = SolverImplicitMPM( | ||
| model, | ||
| solver_cfg.to_solver_config(), | ||
| temporary_store=TemporaryStore(), |
There was a problem hiding this comment.
🔵 Suggestion: Store the as a class attribute () so ownership is explicit and the object cannot be collected if the solver ever drops its reference.
Greptile SummaryThis PR integrates Newton's implicit MPM solver into Isaac Lab's Newton physics-manager abstraction by adding
Confidence Score: 5/5Safe to merge; new MPM manager and config are well-isolated, additive, and non-breaking for existing solvers. The core implementation correctly follows the established Newton manager pattern: shared state lives on the base class, hooks are no-ops by default, and dispatch goes through cfg.class_type. The CUDA-graph gating (sparse/dense opt-out via _supports_cuda_graph_capture) is tested with both monkeypatched unit tests and end-to-end simulation context tests. The only observations are a slightly opaque warning message and a missing mpm:poisson_ratio in the pour demo's fluid material — neither affects correctness of the manager abstraction or other solvers. particle_pour.py (missing mpm:poisson_ratio in fluid particle attributes, uses private _mark_transforms_dirty) and the SOLVER_MATRIX MPM entry in the test file (add_particle_grid without custom_attributes, flagged in a previous review). Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NewtonManager
participant NewtonMPMManager
participant SolverImplicitMPM
User->>NewtonManager: create_builder()
NewtonManager->>NewtonMPMManager: _register_builder_attributes(builder)
NewtonMPMManager->>SolverImplicitMPM: register_custom_attributes(builder)
Note over NewtonMPMManager: idempotent guard: has_custom_attribute(mpm:young_modulus)
User->>NewtonManager: "add_particle_grid(custom_attributes={mpm:*})"
User->>NewtonManager: set_builder(builder)
User->>NewtonManager: sim.reset() → start_simulation()
NewtonManager->>NewtonMPMManager: _register_builder_attributes(builder) [no-op]
NewtonManager->>NewtonMPMManager: initialize_solver → _build_solver(model, cfg)
NewtonMPMManager->>SolverImplicitMPM: SolverImplicitMPM(model, config, temporary_store)
NewtonMPMManager->>NewtonManager: "_solver=solver, _use_single_state=True, _needs_collision_pipeline=False"
NewtonManager->>NewtonMPMManager: _capture_or_defer_graph → _supports_cuda_graph_capture()
NewtonMPMManager->>SolverImplicitMPM: "check grid_type == fixed"
Note over NewtonManager: CUDA graph captured (fixed) or eager (sparse/dense)
loop Each sim.step()
NewtonManager->>NewtonMPMManager: _step_solver(state_0, state_0, control, None, dt)
NewtonMPMManager->>SolverImplicitMPM: step(state_0, state_0, control, None, dt)
opt "project_outside_colliders=True"
NewtonMPMManager->>SolverImplicitMPM: project_outside(state_0, state_0, dt)
end
end
Reviews (2): Last reviewed commit: "Add Newton implicit MPM manager and demo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| CONTAINER_LIFT_TIME = 3.0 | ||
|
|
||
| CONTAINER_BODY_PATH = "/World/ProceduralUtahTeapot" | ||
| CONTAINER_USD_URL = "omniverse://isaac-dev.ov.nvidia.com/Isaac/Props/Teapot/utah_teapot.usdc" |
There was a problem hiding this comment.
CONTAINER_USD_URL points to omniverse://isaac-dev.ov.nvidia.com, which is an internal NVIDIA development server inaccessible to external contributors or users. Anyone outside NVIDIA running this "public" demo will get a RuntimeError: Could not open source-container USD at startup. The asset needs to be either bundled locally, published to a publicly accessible Omniverse Nucleus server (e.g. omniverse://localhost/... for a local Nucleus, or an Isaac Sim CDN URL), or the URL must be overridable via a CLI flag so users can point at their own asset.
|
|
||
| .. code-block:: bash | ||
|
|
||
| ./isaaclab.sh -p scripts/demos/mpm/particle_pour.py --device cuda:0 --viz newton |
There was a problem hiding this comment.
--viz newton but the argparse attribute accessed throughout the demo is args_cli.visualizer (matching the full --visualizer flag added by add_launcher_args). The other demo (newton_mpm_granular.py) correctly documents --visualizer newton. While argparse allows unambiguous prefix abbreviations, the inconsistency in documentation is confusing and error-prone if another --viz* argument is added later.
| ./isaaclab.sh -p scripts/demos/mpm/particle_pour.py --device cuda:0 --viz newton | |
| ./isaaclab.sh -p scripts/demos/mpm/particle_pour.py --device cuda:0 --visualizer newton |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
maxkra15
force-pushed
the
max/newton-mpm-manager
branch
from
8b76353 to
134f1ab
Compare
| solver to dispatch to. | ||
| """ | ||
| cfg = PhysicsManager._cfg | ||
| if isinstance(cfg, NewtonCfg): |
There was a problem hiding this comment.
is it possible this is not newtonCfg? this looks extra defensive
| forward it. | ||
| """ | ||
|
|
||
| def to_solver_config(self) -> SolverImplicitMPM.Config: |
There was a problem hiding this comment.
lets try not add function to config class - let the implementation file import SolverImplicitMPM and construct from cfg
| """ | ||
|
|
||
| @classmethod | ||
| def _register_solver_custom_attributes(cls, builder: ModelBuilder) -> None: |
There was a problem hiding this comment.
this function seems like a shim layer, is this layer removable?
ooctipus
left a comment
ooctipus
left a comment
There was a problem hiding this comment.
This looks fine, there are few places could be better thought and refactored together with newton manager perhaps, but that takes thoughts and time, and also doable in follow up prs,
But left a few high level convention comments
maxkra15
force-pushed
the
max/newton-mpm-manager
branch
from
134f1ab to
f0a6319
Compare
maxkra15
force-pushed
the
max/newton-mpm-manager
branch
from
f0a6319 to
974688c
Compare
2 participants
Description
This PR integrates Newton's implicit MPM solver (
newton.solvers.SolverImplicitMPM) into Isaac Lab'sNewton physics-manager abstraction, plus two runnable demos.
What's added
MPMSolverCfg(isaaclab_newton.physics.MPMSolverCfg) — aNewtonSolverCfgwhose fields mapone-to-one onto
SolverImplicitMPM.Config.to_solver_config()forwards every Newton solver fieldexplicitly (no introspection), and
class_typeresolves toNewtonMPMManager, so users keep thestandard
NewtonCfg(solver_cfg=MPMSolverCfg(...))shape andSimulationContextdispatches to theright manager automatically.
NewtonMPMManager(isaaclab_newton.physics.NewtonMPMManager) — aNewtonManagerspecializationthat builds
SolverImplicitMPM, steps in place on a singleState, treats rigid geometry as colliders(so it does not consume Newton's rigid-body collision pipeline), and enables CUDA-graph capture only for fixed-grid MPM.
NewtonManagerhooks (additive, non-breaking for existing solvers):_register_builder_attributes()/_register_solver_custom_attributes()— lets a solver registerits Newton builder custom attributes before particles are added or the model is finalized. MPM uses
this to register
mpm:young_modulus,mpm:viscosity, … (a hardSolverImplicitMPMrequirement).The base implementation is a no-op, so other solvers are unaffected.
_supports_cuda_graph_capture()— lets a solver opt out of CUDA-graph capture._capture_or_defer_graphhonors it, so sparse/dense-grid MPM falls back to eager execution.
scripts/demos/mpm/):newton_mpm_granular.py— granular (sand-like) particles dropped onto colliders, on a fixed grid bydefault so the MPM step is CUDA-graph captured.
particle_pour.py— pouring viscous media into a container._supports_cuda_graph_capture()hook in the Newton manager abstraction page.Motivation / context for hooks
MPM is the first Isaac Lab Newton solver whose required inputs (per-particle material parameters) are
delivered through Newton's per-solver custom-attribute mechanism rather than built-in model fields, which
is why the two generic hooks above were needed.
Dependencies
Requires a Newton build that ships
SolverImplicitMPM(matches the bundled Newton). No new Python deps.Fixes # (N/A — no tracking issue)
Type of change
Screenshots
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there