[Cloner] Scope Newton global imports with clone plans - #7292
Conversation
Keep shared scene assets explicit without encoding them as no-op replication rows. Populate the paths from both scene-driven and direct-env plan construction so backend work can be scoped from an authoritative plan.
Greptile SummaryThe PR extends
Confidence Score: 5/5The PR appears safe to merge, with global asset ownership added without changing existing replication behavior. The new field defaults safely for existing constructors, is populated in every return path, preserves insertion order while removing duplicates, and remains separate from all replication-row structures. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Cfg[Scene asset configuration] --> Match{Path under environment template?}
Match -->|Yes, spawn exists| Row[Replication row]
Row --> Sources[sources and destinations]
Row --> Mask[clone_mask]
Row --> Ownership[cfg_rows]
Match -->|No| Global[global_paths]
Match -->|Yes, no spawn| Skip[Omitted from plan]
Global -. excluded .-> Sources
Global -. excluded .-> Mask
Global -. excluded .-> Ownership
Reviews (1): Last reviewed commit: "Record global assets in clone plans" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
Reviewed the addition of ordered, de-duplicated shared-asset paths to clone plans. Both plan-construction paths populate the new field while keeping global assets outside the replication rows, and the documentation, changelog, and focused tests reflect that contract.
- Design and architecture: Separating global asset ownership into
global_pathspreserves the invariant that each replication row represents a prototype-to-environment mapping. The main non-blocking tradeoff is that global ownership remains inferred by failure to match the environment destination template rather than by an explicit asset classification. - API:
global_pathsis an additive public dataclass field with an empty-tuple default and is placed after existing fields, preserving existing positional and keyword construction. Its uniqueness, purpose, and exclusion from replication rows are documented. - Implementation: Traced both
make_clone_planandclone_plan_from_env_0: each retains encounter order, removes duplicate paths, and leavessources,destinations,clone_mask, andcfg_rowssemantics unchanged. Tests cover homogeneous, heterogeneous, global-only, duplicate-path, and direct-environment construction paths.
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.
StafaH
left a comment
There was a problem hiding this comment.
Nice! I have 2 comments:
The type for global paths looks inconsistent, and also the conversion looks odd. Can we enfore a type that doesn't require conversion? If its tuple, then we know everywhere that user needs to pass tuple.
Second question is on global world. Let's verify (either in a test or manually), that the generated newton model has the global prim inside newton's global world (found at index -1).
| env_ids=torch.arange(num_clones, dtype=torch.long, device=device), | ||
| positions=positions, | ||
| cfg_rows=cfg_rows, | ||
| global_paths=tuple(dict.fromkeys(global_paths)), |
There was a problem hiding this comment.
Can we use a different input type then Iterable[str], so that we can avoid the conversions?
There was a problem hiding this comment.
Updated all clone/import global_paths annotations to tuple[str, ...] and now pass the tuple through unchanged. The normalization conversions in both clone-plan constructors are removed.
| replicate_priority = 100 | ||
|
|
||
| def __init__(self, stage: Usd.Stage): | ||
| def __init__(self, stage: Usd.Stage, global_paths: Sequence[str] = ()): |
There was a problem hiding this comment.
Updated this and the other replication contexts to use tuple[str, ...] consistently.
| else: | ||
| self._clone_valid_set = None | ||
| return cfgs | ||
| return cfgs, tuple(dict.fromkeys(global_paths)) |
There was a problem hiding this comment.
the main thing to try to improve is to avoide the tuple(dict.fromkeys( if we can with something cleaner.
There was a problem hiding this comment.
Removed tuple(dict.fromkeys(...)). InteractiveScene now constructs the ordered unique tuple directly and returns it unchanged.
|
Addressed the global-world verification in |
|
Re-ran the startup comparison with three alternating post-warm-up runs per commit at 4,096 environments (seed 42). The medians show no measurable end-to-end improvement: Anymal-D Rough kitless -0.65%, KukaAllegro kitless +0.16%, and KukaAllegro Camera/OVRTX -0.10% total startup. Scene-creation deltas were -0.05%, +0.32%, and +0.96%, respectively, all within run-to-run spread. I replaced the earlier 6–9% claim in the PR body because it was based on only one baseline and was sensitive to warm-up variance. This matches the expectation that kitless stages do not materialize the replicated USD env tree. A direct Kit/full-stage benchmark is still needed; Isaac Sim/Kit is not installed on this machine. |
## 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 #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`) 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>
## 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>
Description
Newton replication currently calls
ModelBuilder.add_usd()from the stage root and relies on ignore paths for replicated environments. This change moves global ownership to the scene composition root and carries it inClonePlan.global_paths.InteractiveScenereturns env-scoped clone configs and an orderedtuple[str, ...]of shared prim roots.ClonePlancarries that tuple without representing globals as clone rows.().root_path=....There is no stage-root guessing, global-path registry, or full-stage fallback in clone-plan replication.
Data flow
The scene owns classification:
The plan carries the declaration:
Newton performs narrow imports. For a plan declaring ground and light:
The physics-scene result remains the returned stage metadata; global-import result dictionaries are not merged. Globals stay outside
sources,destinations,clone_mask, andcfg_rows, so they create no clone work or per-environment solver/sensor structures.PhysX collision filtering remains a separate post-clone concern based on
collision_group == -1; global import ownership does not change that policy.Direct scenes
Only scenes with shared roots need an explicit argument:
Scenes without shared roots keep the compact default:
Startup benchmark
RTX 5090, 4,096 environments, seed 42, Newton MJWarp, three post-warm-up runs per commit in alternating order. The baseline is
bc8b7bdf005, immediately before scoped global import. Values are medians; positive deltas mean the PR is faster.Isaac-Velocity-Rough-AnymalD, kitlessIsaac-Lift-KukaAllegro, kitlessIsaac-Lift-KukaAllegro-Camera, OVRTX RGB64All differences are below 1%, so these workloads show no measurable gain. That is expected for the two kitless tasks because their USD stages do not contain materialized replicated environment trees. The warmed OVRTX workload also does not reproduce the earlier single-run estimate.
A full Kit-stage comparison could not be run because Isaac Sim/Kit is not installed on the benchmark machine. That is the case where avoiding traversal of a materialized replicated USD tree should matter, and it remains unmeasured here.
Testing
90 passedon the final API.85 passed, 8 xpassed.151 passed.shape_world == -1; the declared light correctly creates no Newton physics entity.The contact suite exposed the ownership boundary correctly: its ground had previously been spawned by
build_simulation_context()outside the scene config and therefore was absent from the clone plan. The fix declares the ground inContactSensorTestSceneCfg; the single intentionally groundless test setsterrain = None. No discovery fallback was added.Type of change
Checklist