[Task Clean-up][Newton] Dexterous Part 1/8: Fix cloner rows, cubric fallback, and visualizer teardown by hujc7 · Pull Request #6411 · isaac-sim/IsaacLab

hujc7 · 2026-07-08T11:36:29Z

Review Map

Exact changes: a stacked PR's page shows the cumulative diff of its dependency chain; the link pins the commit range that is the PR's own contribution.
Links pin specific SHAs and can go stale after a branch update — the table on [DO-NOT-MERGE][Task Clean-up] Dexterous: lumped validation branch (series reference) #6324 is refreshed first.

PR	Depends on	Exact changes
#6410 [Docs] Environment overview regen	—	—
📌 #6411 Part 1/8: Newton cloner/cubric/visualizer fixes (this PR)	—	—
#6412 Part 2/8: OVPhysX articulation + manager runtime	—	—
#6413 Part 3/8: Reorient Direct, torch	—	—
#6414 Part 4/8: MARL-to-single-agent fix + handover/camera Direct	#6413	changes
#6418 Part 5/8: Reorient manager counterparts	#6413	changes
#6421 Part 6/8: Handover + camera manager counterparts	#6413, #6414, #6418	changes
#6415 Part 7/8: Benchmark success-rate utilities + docs	#6413, #6414, #6418, #6421	changes
#6582 Part 8/8: Warp variants → experimental (draft; merges last)	#6413	changes
#6324 [DO-NOT-MERGE] Lumped validation reference	ALL	—

Summary

Fixes Newton cloner label rows, the cubric IAdapter version audit (exact-match fallback to the CPU hierarchy path), and visualizer teardown.
Retains an in-tree ignore_paths workaround for custom-frequency USD traversal; it becomes redundant once the Newton pin advance (Pin Newton to v1.4.0 and override the Isaac Sim MuJoCo pins #6584) merges — this PR then only needs a rebase.

Stacking

Independent; based on develop.

Review history

Approved. The Newton pin + MuJoCo overrides were split out to Pin Newton to v1.4.0 and override the Isaac Sim MuJoCo pins #6584 via revert commits (2026-07-17) so this PR's CI runs against develop's pins.

Cloner imports no longer create empty MuJoCo custom-frequency rows from ignored environment subtrees; the physics manager falls back from unvalidated cubric adapter versions that produced detached articulation links under Newton with Isaac RTX; visualization markers are torn down before interpreter shutdown to avoid destructor errors. Validated by full dexterous training runs on the Newton backend as part of the Task Clean-up campaign.

greptile-apps · 2026-07-08T11:41:55Z

Greptile Summary

This PR fixes three Newton backend bugs found during dexterous training: (1) empty MuJoCo custom-frequency rows created from ignored environment subtrees during USD import, (2) detached articulation links caused by accepting unvalidated cubric IAdapter minor versions, and (3) crashes during interpreter shutdown when close() re-queried a potentially-torn-down SimulationContext.

Cloner fix: _add_global_stage_to_builder monkey-patches builder.add_custom_frequency during add_usd to scope MuJoCo-frequency prim filters to exclude ignored subtrees; a try/finally block guarantees filter restoration on failure.
Cubric fix: _verify_iadapter_version now requires an exact version match (minor != _IA_EXPECTED_MINOR) instead of the previous semver-compatible check (minor < _IA_EXPECTED_MINOR), since higher minor versions produced incorrect transform behavior with Isaac RTX.
Visualizer fix: NewtonVisualizationMarkers caches the registry reference at __init__ time so close() never calls SimulationContext.instance(); double-close is made idempotent. All three fixes have dedicated regression tests.

Confidence Score: 4/5

Safe to merge; all three fixes are targeted, regression-tested, and have been validated by full dexterous training runs.

The three fixes are well-scoped and backed by dedicated tests. The only notable concern is in _add_global_stage_to_builder, where re.compile(path).match(prim_path) is used for path filtering without anchoring at the end or escaping metacharacters — this could cause /World/envs to mistakenly exclude prims under a path like /World/envs_global. In the current usage patterns of IsaacLab this doesn't trigger, but it's a subtle correctness gap worth addressing.

source/isaaclab_newton/isaaclab_newton/cloner/newton_clone_utils.py — the regex path filter in _scope_filter deserves a second look for edge-case scenes with prim paths that share a prefix with the ignored subtrees.

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/cloner/newton_clone_utils.py	Adds _add_global_stage_to_builder which patches builder.add_custom_frequency to scope MuJoCo-frequency filters during USD import; restore logic via try/finally is correct, but the regex-based path filter uses re.match without escaping or end-anchoring which can over-exclude prims.
source/isaaclab_newton/isaaclab_newton/cloner/replicate.py	Swaps builder.add_usd for _add_global_stage_to_builder to fix empty custom-frequency rows from ignored env subtrees; straightforward call-site change.
source/isaaclab_newton/isaaclab_newton/physics/_cubric.py	Tightens IAdapter version check from semver-compatible (minor >=) to exact-match (minor ==), removing the proceed-under-semver-contract path that produced detached articulation links with Isaac RTX.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Applies the same _add_global_stage_to_builder fix for the non-clone single-builder path; the import is done inline inside the else block rather than at the module top.
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py	Caches vis_marker_registry at init time instead of re-querying SimulationContext during close(), preventing AttributeError when module globals are torn down during interpreter shutdown; double-close is also made idempotent.
source/isaaclab_newton/test/cloner/test_rename_builder_labels.py	Adds TestIgnoredCloneCustomFrequencies with two tests: one verifying tendon rows are not created from ignored env subtrees, another verifying filter restoration after import failure; coverage is thorough.
source/isaaclab_newton/test/physics/test_cubric.py	New regression test for the exact-match version change; constructs an in-memory ctypes plugin descriptor and asserts that v0.2 is rejected while v0.1 is accepted.
source/isaaclab_visualizers/test/test_newton_adapter.py	Adds test_newton_marker_close_does_not_query_simulation_context verifying the shutdown-safe close() path; double-close and idempotency are both exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_add_global_stage_to_builder(builder, stage, ignore_paths, schema_resolvers)"]
    A --> B["Snapshot original usd_prim_filter\nfor each custom_frequency"]
    B --> C["Wrap each existing filter\nwith _scope_filter (ignores ignored_paths)"]
    C --> D["Intercept builder.add_custom_frequency\nvia instance setattr"]
    D --> E["builder.add_usd(stage, ignore_paths,\nschema_resolvers)"]
    E -->|New frequency registered| F["_add_scoped_custom_frequency:\nwrap filter, record original"]
    E -->|Existing frequency re-registered| G["Check callbacks match;\nreturn early if same"]
    E -->|Import error| H["Exception propagates to finally"]
    E --> I["finally block:\nrestore all usd_prim_filter refs\ndelete instance override"]
    H --> I
    F --> I
    G --> I
    I --> J["Return stage_info"]

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["_add_global_stage_to_builder(builder, stage, ignore_paths, schema_resolvers)"]
    A --> B["Snapshot original usd_prim_filter\nfor each custom_frequency"]
    B --> C["Wrap each existing filter\nwith _scope_filter (ignores ignored_paths)"]
    C --> D["Intercept builder.add_custom_frequency\nvia instance setattr"]
    D --> E["builder.add_usd(stage, ignore_paths,\nschema_resolvers)"]
    E -->|New frequency registered| F["_add_scoped_custom_frequency:\nwrap filter, record original"]
    E -->|Existing frequency re-registered| G["Check callbacks match;\nreturn early if same"]
    E -->|Import error| H["Exception propagates to finally"]
    E --> I["finally block:\nrestore all usd_prim_filter refs\ndelete instance override"]
    H --> I
    F --> I
    G --> I
    I --> J["Return stage_info"]

_{Reviews (1): Last reviewed commit: "Fix Newton cloner rows, cubric fallback,..." | Re-trigger Greptile}

greptile-apps · 2026-07-08T11:41:58Z

+    ignored_patterns = tuple(re.compile(path) for path in ignore_paths)
+    original_filters = {
+        frequency_key: frequency.usd_prim_filter for frequency_key, frequency in builder.custom_frequencies.items()
+    }
+
+    def _scope_filter(callback):
+        if callback is None:
+            return None
+
+        def _filtered(prim: Usd.Prim, context: dict[str, Any]) -> bool:
+            prim_path = str(prim.GetPath())
+            if any(pattern.match(prim_path) for pattern in ignored_patterns):


Regex path matching can produce false-positive exclusions

re.compile(path) uses the raw prim-path string as a regex pattern, and pattern.match(prim_path) anchors only at the start (not the end). This means /World/envs also matches /World/envs_global/..., and /World/envs/env_1 matches /World/envs/env_10/.... Any global prim whose path begins with one of the ignore_paths strings as a substring would be silently dropped from the custom-frequency traversal, even though Newton's own add_usd excludes only the intended exact subtrees.

For a correct prefix check, replace the regex compile and match with a plain string prefix comparison that also requires a / separator or an exact match (e.g. prim_path == path or prim_path.startswith(path + "/")), or at minimum use re.compile(re.escape(path) + r"(/|$)") to prevent mid-segment matches.

greptile-apps · 2026-07-08T11:41:59Z

            # Load everything except the env subtrees (ground plane, lights, etc.)
            ignore_paths = [path for _, path in env_paths]
-            builder.add_usd(stage, ignore_paths=ignore_paths, schema_resolvers=schema_resolvers)
+            from isaaclab_newton.cloner.newton_clone_utils import _add_global_stage_to_builder


Inline import of private cross-module function

_add_global_stage_to_builder is now used in three separate modules (replicate.py, visualization_builder.py, and here), which gives it de-facto public cross-module status. The leading underscore and this inline from import are inconsistent with that reach. Moving the import to the top-level imports of newton_manager.py (alongside the other newton_clone_utils imports already used in the file) would make the dependency explicit, and promoting the function to a non-underscore name would align its visibility with its actual API surface.

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!

It is consumed by three modules, so the leading underscore and the physics manager's inline import were misleading; imports move to the module top.

The audit guard treated any newer major as compatible; require the exact expected major.minor instead.

newton-physics/newton#3406 landed the fix upstream: the custom-frequency USD traversal now honors ignore_paths, so the local import-scoping workaround (add_global_stage_to_builder) is removed and its three call sites use builder.add_usd directly. The pin bump pulls in MuJoCo and mujoco-warp 3.10. Also requires the exact expected interface major.minor in the cubric audit.

Newton moves to current main (9af5a9f4181, 1.5.0.dev0), whose custom-frequency USD traversal honors ignore_paths. The pin requires the MuJoCo 3.10 stack while isaacsim-core exact-pins mujoco-warp 3.8, so [tool.uv] override-dependencies force mujoco/mujoco-warp 3.10 for every requester, and the newton-usd-schemas floor rises to 0.4.0 per newton main's [importers] extra. The uv-run pyproject test asserts the new overrides mirror the versions table.

AntoineRichard

Couple of small things otherwise it looks good

AntoineRichard · 2026-07-14T13:09:07Z

+    # MuJoCo-stack overrides mirror the table (they outrank isaacsim-core's exact pins).
+    assert f"mujoco-warp{versions['mujoco_warp']}" in overrides
+    assert f"mujoco{versions['mujoco']}" in overrides
+


Is this needed? I can see why but does it belongs in this PR?

There's a fix on newton side that's merged: newton-physics/newton#3406. The main problem is that it was pinned to older mjcwarp and updating newton needs newer version.
This can be done on a separate PR to update to the latest, at least resolve the dep issue I think.

AntoineRichard · 2026-07-14T13:10:14Z

I didn't know that was a thing, that looks lovely /s

Changes are reasonable otherwise.

AntoineRichard · 2026-07-14T13:11:56Z

I think this is also in another PRs. Worth checking out like the solver coupling PR. https://github.com/isaac-sim/IsaacLab/pull/5834/changes#diff-1b68f4d0395901f033293c2fbb2f489b98135587b9e94bbe7fb560bff647498f

I think it's different context though in the same file.

…eanup-dex-part01 # Conflicts: # source/isaaclab_newton/test/cloner/test_rename_builder_labels.py

…' into jichuanh/task-cleanup-dex-part01

…nager runtime (#6412) ## Summary - Fixes OVPhysX actuator joint indices to follow the common actuator indexing contract. - Fixes OVPhysX initialization alongside Kit by reusing Kit's registered PhysX schema provider. - Fixes the OVPhysX manager to support both the declared public runtime API and the current runtime API. - Regression tests included. Validated by full dexterous training runs on the OVPhysX backend; split out of the lumped validation branch #6324 (Part 2 of 11). ## Dependencies - None. ## Series review map Full integrated diff + training/validation evidence: the lumped validation PR #6324 (DO-NOT-MERGE). | Part | PR | |---|---| | Docs: regenerate the environment overview table | #6410 | | Part 1/11: Newton runtime fixes (cloner rows, cubric fallback, viz teardown) | #6411 | | **Part 2/11: OVPhysX runtime fixes (this PR)** | #6412 | | Part 3/11: success-rate metrics for the Direct reorientation tasks | #6413 | | Part 4/11: RSL-RL training for the handover Direct task | #6414 | | Part 5/11: success-rate support in the benchmark utilities | #6415 | | Part 6/11: renderer presets for the Direct camera task | #6416 | | Part 7/11: OVPhysX presets for the dexterous tasks | #6417 | | Part 8/11: Allegro manager counterpart | #6418 | | Part 9/11: Shadow + OpenAI manager counterparts | #6419 | | Part 10/11: Shadow camera manager counterpart | #6420 | | Part 11/11: Shadow handover manager counterpart | #6421 | --- ### Exact changes in this PR - OVPhysX backend changes + tests: 1f7a433

This reverts commit e553e07.

This reverts commit 4337a76.

This reverts commit 345dd78.

github-actions Bot added the isaac-lab Related to Isaac Lab team label Jul 8, 2026

greptile-apps Bot reviewed Jul 8, 2026

View reviewed changes

hujc7 added 5 commits July 8, 2026 20:54

Reference the upstream Newton issue from the cloner workaround

f424e9c

Fix line length in the cloner workaround docstring

71a0fb5

Make the global-stage cloner helper public

504957a

It is consumed by three modules, so the leading underscore and the physics manager's inline import were misleading; imports move to the module top.

Match the interface-version audit exactly

28567d8

The audit guard treated any newer major as compatible; require the exact expected major.minor instead.

github-actions Bot added the infrastructure label Jul 9, 2026

AntoineRichard approved these changes Jul 14, 2026

View reviewed changes

hujc7 added 4 commits July 15, 2026 01:26

Add skip changelog fragment for the isaaclab test-only change

345dd78

Merge branch 'develop' into jichuanh/task-cleanup-dex-part01

78c157d

Merge remote-tracking branch 'upstream/develop' into jichuanh/task-cl…

51e8e77

…eanup-dex-part01 # Conflicts: # source/isaaclab_newton/test/cloner/test_rename_builder_labels.py

Merge remote-tracking branch 'origin/jichuanh/task-cleanup-dex-part01…

b921d6d

…' into jichuanh/task-cleanup-dex-part01

hujc7 changed the title ~~[Task Clean-up][Newton] Dexterous Part 1/11: Fix cloner rows, cubric fallback, and visualizer teardown~~ [Task Clean-up][Newton] Dexterous Part 1: Fix cloner rows, cubric fallback, and visualizer teardown Jul 17, 2026

hujc7 changed the title ~~[Task Clean-up][Newton] Dexterous Part 1: Fix cloner rows, cubric fallback, and visualizer teardown~~ [Task Clean-up][Newton] Dexterous Part 1/11: Fix cloner rows, cubric fallback, and visualizer teardown Jul 17, 2026

hujc7 mentioned this pull request Jul 17, 2026

Pin Newton to v1.4.0 and override the Isaac Sim MuJoCo pins #6584

Closed

hujc7 added 3 commits July 17, 2026 10:42

Revert "Pin the latest Newton and override the MuJoCo stack"

8a616a2

This reverts commit e553e07.

Revert "Bump Newton to the ignore_paths custom-frequency fix"

3794920

This reverts commit 4337a76.

Revert "Add skip changelog fragment for the isaaclab test-only change"

adb8998

This reverts commit 345dd78.

hujc7 changed the title ~~[Task Clean-up][Newton] Dexterous Part 1/11: Fix cloner rows, cubric fallback, and visualizer teardown~~ [Task Clean-up][Newton] Dexterous Part 1/8: Fix cloner rows, cubric fallback, and visualizer teardown Jul 17, 2026

hujc7 mentioned this pull request Jul 17, 2026

[Task Clean-up] Dexterous Part 8/8: Move the warp Direct task variants to isaaclab_tasks_experimental #6582

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Task Clean-up][Newton] Dexterous Part 1/8: Fix cloner rows, cubric fallback, and visualizer teardown#6411

[Task Clean-up][Newton] Dexterous Part 1/8: Fix cloner rows, cubric fallback, and visualizer teardown#6411
hujc7 wants to merge 14 commits into
isaac-sim:developfrom
hujc7:jichuanh/task-cleanup-dex-part01

hujc7 commented Jul 8, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 8, 2026

Uh oh!

greptile-apps Bot Jul 8, 2026

Uh oh!

greptile-apps Bot Jul 8, 2026

Uh oh!

AntoineRichard left a comment

Uh oh!

AntoineRichard Jul 14, 2026

Uh oh!

hujc7 Jul 14, 2026

Uh oh!

AntoineRichard Jul 14, 2026

Uh oh!

AntoineRichard Jul 14, 2026

Uh oh!

AntoineRichard Jul 14, 2026

Uh oh!

hujc7 Jul 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hujc7 commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Map

Summary

Stacking

Review history

Uh oh!

greptile-apps Bot commented Jul 8, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard left a comment

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

hujc7 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

AntoineRichard Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

hujc7 Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hujc7 commented Jul 8, 2026 •

edited

Loading