[Newton] Streamline contact and raycast sensor startup - #7269
Conversation
Greptile SummaryThe PR reduces Newton startup work while correcting path-expression handling and preserving solver-specific USD attributes.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified. The changed builder, selector, and BVH lifecycles retain direct-stage, cloned, coupled, and post-finalization fallback paths, and the accompanying tests cover the principal behavior changes. Important Files Changed
Sequence DiagramsequenceDiagram
participant Scene as Scene construction
participant Sensor as Newton ray/contact sensor
participant Manager as Active Newton manager
participant Builder as ModelBuilder
participant Model as Finalized model
Scene->>Sensor: Instantiate configured sensors
Sensor->>Manager: Request contact data or BVH shape flags
Scene->>Manager: Create builder using active solver manager
Manager->>Builder: Register active solver attributes
Manager->>Builder: Import or replicate USD model
Manager->>Builder: Apply pending BVH requirements
Builder->>Model: Finalize once
Manager->>Sensor: Dispatch physics-ready initialization
Sensor->>Model: Bind regex-selected views and sensor buffers
Reviews (1): Last reviewed commit: "fix test" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
Reviewed the Newton startup-time changes across solver-specific builder attribute registration, pre-finalization ray-caster BVH configuration, and compiled full-path regex selectors for contact sensors and Newton views. No numbered candidate findings were supplied for acceptance.
- Design and architecture: The active manager now owns solver-specific attribute registration, including nested coupled solvers, while ray casters request collision-shape BVH coverage before model finalization and retain the existing late-registration rebuild fallback.
- API: Checked the compatibility surface for contact-sensor shape selectors, body selectors, and Newton view roots. The intentional full-path, full-match regex behavior is documented with migration examples in configuration docstrings and package changelog fragments; public configuration field names remain unchanged.
- Implementation: Traced custom-attribute registration through builder creation, stage import, coupled managers, and finalization. Also traced BVH requirements from ray-caster construction through finalization, fallback rebuilding, and manager cleanup, plus the selector path from expression compilation to Newton sensor/view construction. The added tests cover solver-registration ownership, BVH inclusion and fallback behavior, regex matching, malformed expressions, and coupled-manager registration.
No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.
Automated review; human maintainers own approval decisions.
Newton 1.4 deprecated the `sensing_obj_*` spellings on SensorContact. Newton's deprecated properties return the new attributes unchanged, so this is a pure rename with no behavior change, and it keeps the sensor working when Newton removes the aliases.
603e5f1 to
5d0e1b1
Compare
## Summary This PR is now the contact/raycast part of the Newton startup work: - compile contact-sensor full-path expressions once and match them directly, without regex-to-glob conversion or stage globbing; - use Newton's current contact-sensing API names; - declare raycast collision-shape requirements before model finalization, so the model builds one correctly configured BVH instead of rebuilding it during sensor initialization. The contact-selector and sensing-API commits retain Chris's (`camevor`) original authorship. The BVH lifecycle change incorporates the review from isaac-sim#7296 while keeping this as Chris's PR. The related ownership work is intentionally split by component: isaac-sim#7292 handles explicit global ownership, and isaac-sim#7295 handles model/articulation startup. This PR contains no inactive-solver registration or articulation-view changes and supersedes isaac-sim#7296. Production code is `+29/-56` (net `-27`) against `909cc5decc5`. ## Startup benchmark RTX 5090, CUDA device 1, 4096 environments, three fresh processes per revision/task. Values are median end-to-end startup wall time. Base: `909cc5decc5`. PR: `5d0e1b1595e`. | Task | Base | PR | Change | |---|---:|---:|---:| | `Isaac-Cartpole` | 8.316 s | 8.410 s | +1.1% | | `Isaac-Velocity-Rough-UnitreeGo2` | 16.363 s | 15.682 s | -4.2% | | `Isaac-Lift-KukaAllegro-Camera` | 39.473 s | 37.605 s | -4.7% | Cartpole has no contact/raycast workload here and is neutral within process-startup noise. The sensor-heavy tasks show the intended gain: | Median phase | Go2 base | Go2 PR | Kuka base | Kuka PR | |---|---:|---:|---:|---:| | `newton_contact_sensor` | 0.10 s | 0.04 s | 1.32 s | 0.24 s | | `simulation_start` | 6.36 s | 5.91 s | 11.48 s | 10.39 s | Raw end-to-end totals: - Cartpole base: 9.368, 8.316, 8.195 s; PR: 8.410, 8.370, 8.476 s. - Go2 rough base: 19.900, 16.363, 16.091 s; PR: 15.669, 15.682, 15.984 s. - Kuka camera base: 41.328, 38.625, 39.473 s; PR: 37.580, 37.605, 38.785 s. ## Test plan - `261 passed, 8 xpassed` across the Newton manager abstraction, contact-sensor, and raycast-sensor suites. - Repository formatting and pre-commit checks pass. - Architecture checks reject the removed regex-to-glob path, deprecated sensing names, duplicate BVH state, and late BVH fallback. - The three 4096-environment benchmark tasks also provide end-to-end Newton MJWarp startup coverage. ## Type of change - Performance improvement - Bug fix --------- Co-authored-by: Octi Zhang <zhengyuz@nvidia.com> (cherry picked from commit 21bc111)
## Summary This is the model/articulation part of the scoped Newton startup work. It keeps startup work with the component that owns it: - each Newton manager declares only the custom builder schema used by its active solver; - articulation target modes are resolved for the prototype and copied to its replicas; - the base physics manager owns the articulation-view registry; - articulations create and register their view, while joint-wrench sensors reuse it or create it when used independently; - root expressions remain regular expressions instead of taking a lossy regex-to-glob round trip. The active-solver and reusable-view findings originated in Chris's #7269. This PR isolates those model/articulation changes so #7269 can remain the contact/raycast change under Chris's PR. This replaces the cloner's unconditional MuJoCo + Kamino registration and the duplicate joint-wrench view. There is no view scan, manager-specific cache API, compatibility fallback, or duplicate registry. The production diff against the current base is `+57/-60` (net `-3`). The explicit-global-ownership part is #7292. The contact/raycast part remains in #7269. ## Startup benchmark RTX 5090, CUDA device 1, 4096 environments, three fresh processes per revision/task. Values are median end-to-end startup wall time; raw runs are included below. Base: `86cf66651bd`. PR: `947869af173`. | Task | Base | PR | Change | |---|---:|---:|---:| | `Isaac-Cartpole` | 8.316 s | 7.655 s | -8.0% | | `Isaac-Velocity-Rough-UnitreeGo2` | 17.525 s | 14.844 s | -15.3% | | `Isaac-Lift-KukaAllegro-Camera` | 41.691 s | 36.832 s | -11.7% | Raw totals: - Cartpole base: 10.595, 8.190, 8.316 s; PR: 7.655, 7.674, 7.469 s. - Go2 rough base: 17.568, 17.525, 16.426 s; PR: 14.852, 14.844, 14.724 s. - Kuka camera base: 47.590, 41.691, 38.724 s; PR: 36.832, 36.666, 37.032 s. The measured `env_creation` medians improve from 6.473 to 5.801 s for Cartpole, 15.558 to 12.863 s for Go2 rough, and 37.481 to 32.578 s for Kuka camera. ## Test plan - `247 passed` across the physics-manager lifecycle, Newton cloner, manager abstraction, coupled-manager, and joint-wrench reuse tests. - `test_rename_builder_labels.py`: `17 passed` after removing obsolete solver-registration mocks. - Ruff check and format pass on all changed Python files. - The three 4096-environment benchmark tasks provide end-to-end Newton MJWarp startup coverage.
## Summary This is the model/articulation part of the scoped Newton startup work. It keeps startup work with the component that owns it: - each Newton manager declares only the custom builder schema used by its active solver; - articulation target modes are resolved for the prototype and copied to its replicas; - the base physics manager owns the articulation-view registry; - articulations create and register their view, while joint-wrench sensors reuse it or create it when used independently; - root expressions remain regular expressions instead of taking a lossy regex-to-glob round trip. The active-solver and reusable-view findings originated in Chris's isaac-sim#7269. This PR isolates those model/articulation changes so isaac-sim#7269 can remain the contact/raycast change under Chris's PR. This replaces the cloner's unconditional MuJoCo + Kamino registration and the duplicate joint-wrench view. There is no view scan, manager-specific cache API, compatibility fallback, or duplicate registry. The production diff against the current base is `+57/-60` (net `-3`). The explicit-global-ownership part is isaac-sim#7292. The contact/raycast part remains in isaac-sim#7269. ## Startup benchmark RTX 5090, CUDA device 1, 4096 environments, three fresh processes per revision/task. Values are median end-to-end startup wall time; raw runs are included below. Base: `86cf66651bd`. PR: `947869af173`. | Task | Base | PR | Change | |---|---:|---:|---:| | `Isaac-Cartpole` | 8.316 s | 7.655 s | -8.0% | | `Isaac-Velocity-Rough-UnitreeGo2` | 17.525 s | 14.844 s | -15.3% | | `Isaac-Lift-KukaAllegro-Camera` | 41.691 s | 36.832 s | -11.7% | Raw totals: - Cartpole base: 10.595, 8.190, 8.316 s; PR: 7.655, 7.674, 7.469 s. - Go2 rough base: 17.568, 17.525, 16.426 s; PR: 14.852, 14.844, 14.724 s. - Kuka camera base: 47.590, 41.691, 38.724 s; PR: 36.832, 36.666, 37.032 s. The measured `env_creation` medians improve from 6.473 to 5.801 s for Cartpole, 15.558 to 12.863 s for Go2 rough, and 37.481 to 32.578 s for Kuka camera. ## Test plan - `247 passed` across the physics-manager lifecycle, Newton cloner, manager abstraction, coupled-manager, and joint-wrench reuse tests. - `test_rename_builder_labels.py`: `17 passed` after removing obsolete solver-registration mocks. - Ruff check and format pass on all changed Python files. - The three 4096-environment benchmark tasks provide end-to-end Newton MJWarp startup coverage. (cherry picked from commit c4a2759)
…7299) ## Summary Backports the following merged changes to `release/3.0.0` as separate provenance-preserving cherry-picks: - #7292 — scope Newton global imports with clone plans - #7285 — stabilize the sensor/PhysX video recording test - #7269 — streamline Newton contact and raycast sensor startup - #7119 — normalize non-finite depth display values safely - #7295 — avoid repeated Newton model and articulation startup work Each source squash commit was cherry-picked with `-x` and applied without conflicts. ## Validation - Stable patch IDs match all five source squash commits exactly. - File-by-file manifests match each source squash commit. - `git diff --check upstream/release/3.0.0..HEAD` - `uv run --frozen python tools/changelog/cli.py check backport-7285-7292-base` - `SKIP=check-changelog-fragments uv run --frozen isaaclab -f` - Cloner/Newton focused tests: 98 passed - Scene global-ownership tests: 2 passed - Simulator clone-plan tests: 4 passed - Video recording regression test: 1 passed - Newton BVH lifecycle tests: 2 passed - Newton contact-selector tests: 7 passed - Newton raycast BVH test: 4 passed - Non-finite depth display tests: 4 passed - #7295 physics lifecycle, cloner, manager, and coupling tests: 248 passed - #7295 Newton joint-wrench sensor tests: 11 passed - #7295 PhysX joint-wrench sensor tests: 16 passed PR #7121 remains open and is intentionally excluded; it will be backported from its final merge commit after merging. --------- Signed-off-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Co-authored-by: matthewtrepte <mtrepte@nvidia.com> Co-authored-by: camevor <camevor@nvidia.com> Co-authored-by: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Co-authored-by: Antoine RICHARD <antoiner@nvidia.com>
Summary
This PR is now the contact/raycast part of the Newton startup work:
The contact-selector and sensing-API commits retain Chris's (
camevor) original authorship. The BVH lifecycle change incorporates the review from #7296 while keeping this as Chris's PR.The related ownership work is intentionally split by component: #7292 handles explicit global ownership, and #7295 handles model/articulation startup. This PR contains no inactive-solver registration or articulation-view changes and supersedes #7296.
Production code is
+29/-56(net-27) against909cc5decc5.Startup benchmark
RTX 5090, CUDA device 1, 4096 environments, three fresh processes per revision/task. Values are median end-to-end startup wall time. Base:
909cc5decc5. PR:5d0e1b1595e.Isaac-CartpoleIsaac-Velocity-Rough-UnitreeGo2Isaac-Lift-KukaAllegro-CameraCartpole has no contact/raycast workload here and is neutral within process-startup noise. The sensor-heavy tasks show the intended gain:
newton_contact_sensorsimulation_startRaw end-to-end totals:
Test plan
261 passed, 8 xpassedacross the Newton manager abstraction, contact-sensor, and raycast-sensor suites.Type of change