[Newton] Synchronize actuator and model properties - #6799
[Newton] Synchronize actuator and model properties#6799AntoineRichard wants to merge 18 commits into
Conversation
Resolve target modes from existing actuator gains before Newton finalizes the model so solver actuator topology matches the configured controller. Refs isaac-sim#6649
Align sparse actuator gain dictionaries to every selected DOF before\ninferring Newton modes, and verify finalized MuJoCo actuator topology.\n\nRefs isaac-sim#6649
Bind passive Newton damping separately from actuator derivative gains and include it in actuator initialization and joint parameter randomization. Refs isaac-sim#6517
Exercise passive damping bindings and writers under nonidentity joint ordering, and keep the finalized-model writer test with the Newton articulation suite.
Update inverse mass and inertia whenever Isaac Lab changes Newton body inertial properties so solver notifications observe a consistent model. Refs isaac-sim#6518
Describe config-based actuator mode inference and record Newton damping and inertial consistency fixes in package changelog fragments. Refs isaac-sim#6649 Refs isaac-sim#6517 Refs isaac-sim#6518
Keep Newton interface mocks aligned with finalized inverse inertial arrays and share the inverse-update math between selector kernels. Remove the internal design artifact from the branch.\n\nRefs isaac-sim#6518
github-actions
Bot
added
documentation
Greptile SummarySynchronizes Newton’s runtime model properties with Isaac Lab configuration.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code defect was identified. The target-mode resolution, passive-damping bindings, randomization writes, and inverse inertial-property updates are consistently integrated across the affected Newton asset implementations and covered by focused regressions. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Isaac Lab actuator configuration] --> B[Resolve effective stiffness and damping]
B --> C[Set Newton joint target modes]
C --> D[Finalize Newton model and solver]
E[Runtime joint property writes] --> F[Static friction and passive viscous damping]
G[Runtime mass or inertia writes] --> H[Recompute inverse mass and inverse inertia]
F --> I[Notify Newton model of property changes]
H --> I
Reviews (1): Last reviewed commit: "Fix Newton inverse property integration" | Re-trigger Greptile |
![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
The Newton property synchronization is generally well integrated, but builder-time actuator target-mode inference does not identify articulations and DOFs consistently with the runtime articulation view. This can silently skip configured articulations or apply actuator modes to floating-base DOFs.
- Design and architecture: The MODEL_INIT callback is appropriate for pre-finalization configuration and is correctly deregistered. However, it matches builder labels against the configured prim path rather than the resolved articulation-root path, so assets whose ArticulationRootAPI is on a descendant can bypass target-mode inference.
- API: The new viscous-friction property and writers follow existing naming, selector, typing, and physical-unit documentation conventions. No material public API issue was identified.
- Implementation: The inverse inertial-property and viscous-damping plumbing is consistent with ordering and live model bindings. The builder target-mode pass must exclude FREE and FIXED joints to mirror the runtime view and avoid configuring floating-base DOFs.
Significant concerns. Posted 2 actionable findings inline.
Conservative automated review; human maintainers own approval decisions.
|
|
||
| def _configure_builder_joint_target_modes(builder, cfg: ArticulationCfg) -> None: | ||
| """Resolve configured actuator gains into Newton builder target modes before finalization.""" | ||
| articulation_ids, _ = resolve_matching_names(cfg.prim_path, builder.articulation_label, raise_when_no_match=False) |
There was a problem hiding this comment.
🟡 Warning · Design Architecture — Match resolved articulation root, not prim_path
Articulations are selected by matching cfg.prim_path against builder.articulation_label, whereas _initialize_impl deliberately resolves a separate root_prim_path_expr because the ArticulationRootAPI may sit on a descendant prim. For those assets no articulation matches and, with raise_when_no_match=False, target-mode inference silently does nothing. Reuse the same articulation-root resolution here so both paths agree on the articulation identity.
| joint_end = builder.articulation_end[articulation_id] | ||
| dof_ids: list[int] = [] | ||
| dof_names: list[str] = [] | ||
| for joint_id in range(joint_start, joint_end): |
There was a problem hiding this comment.
🟡 Warning · Implementation — Target-mode pass includes free-joint DOFs
This loop enumerates every builder joint in the articulation range, while the runtime view is built with exclude_joint_types=[JointType.FREE, JointType.FIXED]. For a floating-base robot the root free joint contributes six DOF names, so a common joint_names_expr=[".*"] actuator matches them and rewrites joint_target_mode for base DOFs that Isaac Lab never actuates. Skip FREE/FIXED joint types so the builder joint set mirrors the runtime one.
Retain the upstream selector changes while preserving Newton actuator viscous-friction propagation and both regression test sets.
Provide the synthetic articulation with a root-view count so shape validation can check the selected environment dimension.
Record the approved design for updating primary and inverse body\ninertial properties in one Newton kernel launch. Keep the shared\nordering utilities backend-neutral while preserving selector support.
Match builder articulations through the resolved root path and leave free\nand fixed joint modes unchanged.
Update primary and inverse body properties in one launch while preserving articulation ordering and signed selector widths.
Keep the pull request limited to product code, tests, documentation, and changelog fragments.
Normalize clone-plan view globs back to regular expressions before selecting Newton builder articulations.
2 participants
Description
Keep Newton actuator and model properties consistent with Isaac Lab runtime
configuration.
damping before solver construction. Explicit actuators use effort mode,
Nonegains preserve imported USD values, and unconfigured joints remainuntouched.
Model.joint_damping, separately fromPD damping, and include it in supported joint-parameter randomization.
rigid-object, and rigid-object-collection index/mask setters before solver
notification.
packages.
No new actuator configuration field or public inverse-inertia API is introduced.
Fixes #6649
Fixes #6518
Fixes #6517
Type of change
Screenshots
Not applicable.
Verification
./isaaclab.sh -f./isaaclab.sh -dThe CUDA- and Nucleus-dependent full asset suites were not run on this CPU-only
host.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there