skip usd cloning in pure newton path (#5743)#5872
Conversation
# Description Fixes Newton replicated-scene cloning so clone-plan source paths are available before sensor construction and asset USD replication is skipped when Newton handles physics replication. This also updates Newton frame views, ray caster and joint wrench sensor resolution, Newton camera preparation, deformable managers, and DexSuite point-cloud sampling to resolve against clone-plan source prims and Newton model labels instead of cloned destination USD prims. Fixes # N/A ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Screenshots N/A ## Validation - `./isaaclab.sh -f` was attempted. It passed ruff, formatting, whitespace, YAML/TOML, merge-conflict, private-key, debug-statement, codespell, license, and RST checks before failing in `end-of-file-fixer` with `PermissionError` on root-owned files in this checkout. The working tree remained clean after the attempt. - `git diff --check upstream/develop...HEAD` passed. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` -- CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
ooctipus
requested review from
Mayankm96,
jtigue-bdai,
kellyguo11 and
pascal-roth
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
Cherry-Pick Review: #5743 → release/3.0.0-beta2
This PR is a cherry-pick of #5743 (merged to develop on 2026-05-29) to the release/3.0.0-beta2 branch.
✅ Cherry-Pick Assessment
| Check | Status |
|---|---|
| Single commit (clean cherry-pick) | ✅ |
| Pre-commit checks | ✅ Passed |
| Changelog fragments added | ✅ Passed |
| CI checks | ⏳ In progress (critical checks passed) |
📋 Change Summary
This cherry-pick brings Newton replicated-scene cloning improvements to the release branch:
- Core fix: Clone-plan source paths available before sensor construction
- USD replication skip: Newton handles physics replication internally
- Updated components: Newton frame views, ray caster, joint wrench sensor, camera preparation, deformable managers, DexSuite point-cloud sampling
- 58 files changed across
isaaclab,isaaclab_newton,isaaclab_physx,isaaclab_ovphysx,isaaclab_contrib, andisaaclab_tasks
🔍 Release Branch Appropriateness
✅ Appropriate for release branch because:
- Bug fix type change (non-breaking)
- Already validated on
developbranch - Includes proper changelog fragments
- Has test coverage additions
Verdict
LGTM 👍 - Clean cherry-pick of a validated fix. Ready for release branch integration pending CI completion.
Automated review by Isaac Lab Review Bot
Greptile SummaryThis PR skips USD prim cloning in the pure-Newton (headless, no-Kit) code path, avoiding the overhead of replicating USD stage structure that Newton doesn't need for physics simulation. It also introduces three plan-aware prim-resolution helpers (
Confidence Score: 3/5The core USD-skip logic and new per_world site injection appear sound, but two concrete runtime crashes are present in the changed code paths that need fixing before merge. Newton multi-mesh ray casters with track_mesh_transforms=True will throw KeyError at runtime because _create_tracked_target_view now does a direct dict lookup whose key (built with * wildcards via resolve_clone_plan_source) never matches the keys passed by base_multi_mesh_ray_caster._build_mesh_records (built with .*); the previous .get() fallback that silently papered over this is gone. Additionally, resolve_matching_prims_from_source dereferences SimulationContext.instance() without a null guard. source/isaaclab_newton/isaaclab_newton/sensors/ray_caster/ray_caster.py (KeyError in _create_tracked_target_view) and source/isaaclab/isaaclab/sim/utils/queries.py (null deref in resolve_matching_prims_from_source) need fixes before merge. Important Files Changed
Reviews (1): Last reviewed commit: "skip usd cloning in pure newton path (#5..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _create_tracked_target_view(self: Any, target_prim_path: str | list[str]): | ||
| """Resolve dynamic multi-mesh target sites to raw Newton site indices.""" | ||
| target_key = tuple(target_prim_path) if isinstance(target_prim_path, list) else target_prim_path | ||
| labels = self._tracked_site_labels_by_expr.get(target_key) | ||
| if labels is None: | ||
| target_exprs = target_prim_path if isinstance(target_prim_path, list) else [target_prim_path] | ||
| labels = self._register_target_sites_for_exprs([_newton_body_pattern(expr) for expr in target_exprs]) | ||
| self._tracked_site_labels_by_expr[target_key] = labels | ||
| target_exprs = target_prim_path if isinstance(target_prim_path, list) else [target_prim_path] | ||
| labels = self._tracked_site_labels_by_target[tuple(target_exprs)] | ||
| site_indices = self._resolve_site_indices(labels, str(target_prim_path), self._num_envs) | ||
| return wp.array(site_indices, dtype=wp.int32, device=self._device) |
There was a problem hiding this comment.
KeyError when Newton + clone plan + tracked mesh transforms are combined
_create_tracked_target_view does a direct dict lookup with no fallback. The keys stored in _tracked_site_labels_by_target at __init__ time come from _resolve_target_owner_exprs, which produces destination expressions using bare * (via resolve_clone_plan_source → matching_template.replace("{}", "*")). However, base_multi_mesh_ray_caster._build_mesh_records builds tracked_target_exprs using destination_template.format(".*") (dot-star), and passes those as the lookup keys. A concrete example: stored key ("/World/envs/*/Robot",) vs lookup key ("/World/envs/.*/Robot",) → KeyError. In the legacy (no-plan) path, the same mismatch exists between the _newton_body_pattern output (env_.*) and the concrete path from reference_prim.GetPath(). The previous code used .get() with a lazy-registration fallback that masked this discrepancy; removing the fallback without fixing the format inconsistency makes this a hard crash for any Newton multi-mesh ray caster with track_mesh_transforms=True.
| plan = SimulationContext.instance().get_clone_plan() | ||
| resolved = resolve_clone_plan_source(path_expr, plan) if plan is not None else None |
There was a problem hiding this comment.
SimulationContext.instance() returns None before initialization; calling .get_clone_plan() directly on it will raise AttributeError. This guard matches the pattern used in sensor_base.py and elsewhere.
| plan = SimulationContext.instance().get_clone_plan() | |
| resolved = resolve_clone_plan_source(path_expr, plan) if plan is not None else None | |
| sim = SimulationContext.instance() | |
| plan = sim.get_clone_plan() if sim is not None else None | |
| resolved = resolve_clone_plan_source(path_expr, plan) if plan is not None else None |
| if (count := num_variants(cfg.spawn)) > 0: | ||
| groups.append((cfg, cfg.spawn, prim_path.replace(self.env_regex_ns, self.env_fmt), count)) |
There was a problem hiding this comment.
ValueError to silently skipping spawners that return zero variants removes a validation that was protecting against misconfigured spawn configs. A spawner unexpectedly returning 0 variants will now produce a scene that is missing an asset without any diagnostic error.
| if (count := num_variants(cfg.spawn)) > 0: | |
| groups.append((cfg, cfg.spawn, prim_path.replace(self.env_regex_ns, self.env_fmt), count)) | |
| if (count := num_variants(cfg.spawn)) <= 0: | |
| raise ValueError(f"Spawner at '{prim_path}' must have at least one variant.") | |
| groups.append((cfg, cfg.spawn, prim_path.replace(self.env_regex_ns, self.env_fmt), count)) |
|
|
||
| masking = torch.zeros((sum(group_sizes), num_clones), dtype=torch.bool, device=device) | ||
| masking[rows, cols] = True | ||
| return ClonePlan(sources=src, destinations=dest, clone_mask=masking) | ||
| return src, dest, masking |
There was a problem hiding this comment.
make_clone_plan
make_clone_plan previously returned a ClonePlan and is now changed to return a raw tuple[tuple[str, ...], tuple[str, ...], torch.Tensor]. All internal callers in the repo are updated, but the function is exported in cloner/__init__.pyi as part of the public API. Any external downstream packages that call make_clone_plan and access .sources, .destinations, or .clone_mask attributes on the result will receive an AttributeError at runtime with no compile-time warning. Consider keeping a thin ClonePlan-returning wrapper or clearly marking the deprecation.
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!
kellyguo11
merged commit e9fe73c
into
isaac-sim:release/3.0.0-beta2
2 participants
No description provided.