[hujc7]

Review Map

• A stacked PR's page shows the cumulative diff of its dependency chain; the "Exact changes" link opens that PR's Files changed page on its own changes (parent head to PR head).
• SHA links can go stale after a branch update; the copy on [Warp] Part 1/12: Resolve id and mask selections through shared helpers #8821 is refreshed first.

PR	Status	Depends on	Exact changes
#8821 Part 1/12: shared id and mask helpers, mask writers without host sync		—	—
#8824 Part 2/12: PhysX tendon mask setters through the masks		#8821	link
📌 #6646 Part 3/12: mask-correct partial resets		#8821	link
#8783 Part 4/12: shared Warp RNG state		#6646	link
#6607 Part 5/12: per-term capture, stable stepping parity		#8783	link
#8228 Part 6/12: Warp commands, Rough tasks, live term changes		#6607	link
#6582 Part 7/12: Shadow Warp twin, Allegro twin fixes		#6607	link
#7012 Part 8/12: --video on the Warp frontend		—	—
#8856 Part 9/12: stable SceneEntityCfg Warp selections, capture utilities		#8228	link

---
config:
  look: neo
  flowchart:
    curve: basis
---
flowchart TD
  P1("<b>#8821</b> · Part 1<br/>id/mask helpers")
  P2("<b>#8824</b> · Part 2<br/>PhysX tendon masks")
  P3("📌 <b>#6646</b> · Part 3<br/>masked resets")
  P4("<b>#8783</b> · Part 4<br/>WarpRng")
  P5("<b>#6607</b> · Part 5<br/>per-term capture")
  P6("<b>#8228</b> · Part 6<br/>commands, Rough")
  P7("<b>#6582</b> · Part 7<br/>Shadow twin")
  P8("<b>#7012</b> · Part 8<br/>--video, independent")
  P9("<b>#8856</b> · Part 9<br/>stable SceneEntityCfg, capture")
  P1 --> P2
  P1 --> P3 --> P4 --> P5
  P5 --> P6
  P5 --> P7
  P6 --> P9
  style P3 stroke-width:3px

Loading

Summary

Masked resets on Newton, OvPhysX and PhysX reset only the selected environments' actuator, wrench, sensor and deformable state (main resets every environment's), and no longer synchronize with the host, so the Warp frontend can record them in CUDA graphs.
InteractiveScene.reset takes the mask too, so the Warp environments use the stable scene.

Description

Update (2026-10-26): InteractiveScene.reset(env_mask=...) forwards the mask to every asset and sensor; InteractiveSceneWarp is removed. #8821 now owns the mask writers this PR used to change.

1. Fixes on main

1. Articulation.reset(env_mask=...) (Newton, OvPhysX, PhysX) reset the actuator state of every environment.
2. Newton and PhysX RigidObject / RigidObjectCollection resets ignored env_mask and cleared every environment's external wrenches.
3. InteractiveSceneWarp.reset(env_mask) reset deformables, surface grippers and sensors in every environment and skipped cable objects.
4. A contrib Multirotor reset with only env_mask reset every environment's thrusters.

2. Capture readiness

Actuator models, DelayBuffer and backend-native actuators reset from the mask without converting it to indices; masked ray-caster drift is resampled in a Warp kernel.

3. Breaking changes

1. ActuatorBase.reset takes env_mask; custom actuator models must reset from it without a host sync.
2. isaaclab_experimental.envs.InteractiveSceneWarp is removed; use isaaclab.scene.InteractiveScene.

4. Evidence (MEASURED, desk RTX PRO 6000)

• At 93daa9b9bf6: Newton native-actuator (40), articulation (39) and Warp frontend (29) tests pass; InteractiveScene.reset mask forwarding is a stub-scene unit test (test_interactive_scene.py); Warp Cartpole, Flat-Go2, Cartpole-Direct and torch Cartpole run with the stable scene (4096 envs, 150 steps with resets).
• New tests fail on main: actuator and wrench isolation, the OvPhysX reset, the PhysX wrench reset, and masked resets under capture (cudaErrorStreamCaptureImplicit). Cable-object forwarding and the surface-gripper mask reset have no test.

5. Follow-ups

• OvPhysX RigidObjectCollection mask writers still convert on the host: the OvPhysX wheel's writes wait on the legacy stream (Warp CUDA error 906).

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (existing functionality will not work without user modification)

Release backport

• Backport this pull request to the active release branch after it merges into main

Screenshots

Not applicable.

Checklist

Docker and GPU tests run on demand. Push the commits you want tested, then
comment run-ci on the pull request.

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab --format
• I have made corresponding changes to the documentation: the Warp guides already describe mask-based resets
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[hujc7]

run-ci

[greptile-apps]

Confidence Score: 3/5

[Medium impact] The changes affect reset behavior across several simulation backends and shared asset APIs.

Fix repeated tendon property refreshes and unselected gripper resets before merging.

Findings

1.  Replays leave tendon properties stale ▶
2.  Unselected grippers still open ▶
3.  Reverse slices stop working ▶
4.  Drift reset ignores copied mask ▶

Summary

Adds mask-based partial resets across actuators, assets and sensors, and shares selection helpers across backends.

• Actuator resets clear history only for selected environments.
• Warp scene resets forward the mask to the selected assets.
• Rigid-object resets clear wrenches only for selected environments.
• Ray-caster resets resample drift only for selected environments.
• Backend selection helpers resolve ids and masks in a shared way.

Acknowledged scope:

• hujc7 explicitly keeps tendon targets behind the host flag for performance; callers must set targets eagerly.
• hujc7 defers captured OVPhysX collection writes because the upstream wheel waits on the legacy stream.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Masked scene reset] --> B[Actuator state]
    A --> C[Wrench buffers]
    A --> D[Deformables and cables]
    A --> E[Sensors]
    A --> F[Surface grippers]
    B --> G[Selected environments]
    C --> G
    D --> G
    E --> G
    F --> H[Mask omitted: every gripper opens]

Loading

_{Reviews (1) · Last reviewed commit: "Write Newton tendon targets behind the h..." · Reviewed by Greptile}

[greptile-apps]

Replays leave tendon properties stale

write_fixed_tendon_properties_to_sim_mask records the copies into a CUDA graph, but add_model_change runs only in Python during capture. After the first replay and sim.step(), the queued notification is cleared. A second replay writes new properties without refreshing the solver, so physics keeps using the previous stiffness, damping and limits.

Make the solver refresh happen on every replay. Test two replays with different values, separated by physics steps.

[greptile-apps]

Unselected grippers still open

The scene reset fix still omits the mask for surface grippers. Calling scene.reset(env_mask=mask) reaches surface_gripper.reset(None), which opens every gripper. This releases objects in environments whose episodes are still running.

Call reset_mask(env_mask) when a mask is supplied, and add a scene test that keeps an unselected gripper closed.

[greptile-apps]

Reverse slices stop working

Replacing the experimental resolve_1d_mask drops negative-step slice support. For example, resolve_env_mask(env_ids=slice(None, None, -2)) previously expanded the slice with Python range. The shared helper instead assigns through a PyTorch slice, which raises because its step must be positive.

Preserve the range-based path for negative steps and add a regression test.

[greptile-apps]

Drift reset ignores copied mask

For a CUDA ray caster given a CPU Warp mask, _resolve_indices_and_mask copies the mask to CUDA and resets the sensor correctly. The new drift kernel still receives the original CPU env_mask, so its CUDA launch fails when drift is enabled.

Pass the device-correct reset_mask to this kernel instead.

Suggested change

	inputs=[
	env_mask,
	wp.vec2f(*self.cfg.drift_range),
	inputs=[
	reset_mask,
	wp.vec2f(*self.cfg.drift_range),

[isaaclab-review-bot]

Isaac Lab Review Bot

Needs attention: Compatibility and deprecation (1), Implementation (1), Style consistency (1), Test quality (1). Other reviewed categories look good.

• PR description: The title still claims a manager-construction fix absent from the production diff; retitle it, remove dated/reverted history and the unsupported 2–4% figure, and correct the surface-gripper reset claim.
• Design and architecture: Architecture looks good.
• Scope and over-engineering: No over-engineering issues.
• API: API changes look good.
• Compatibility and deprecation: Breaking changes: ActuatorCollection always calls custom ActuatorBase.reset with env_mask, so existing subclasses fail even on unmasked resets; preserve old calls and add a warned transition before requiring the keyword.
• Implementation: InteractiveSceneWarp promises mask precedence but calls surface_gripper.reset with env_ids only, resetting every gripper on mask-only resets; call reset_mask with env_mask.
• Style consistency: ImplicitActuator and IdealPDActuator narrow ActuatorBase.reset’s env_ids annotation by omitting slice; use the base signature in every override.
• Test quality: The Newton tendon capture test checks staged buffers, so it cannot detect a broken simulation-copy kernel; assert the sim_bind_fixed_tendon* buffers after replay.

Significant concerns. Posted 4 inline findings.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🔴 Critical · Compatibility — Actuator reset break lacks deprecation

env_mask is passed on every reset, so any custom ActuatorBase subclass declaring reset(env_ids) raises TypeError even for unmasked resets. Forward env_mask only when set, with a deprecation warning.

[isaaclab-review-bot]

🟡 Warning · Style Consistency — Reset overrides narrow base selector type

ActuatorBase.reset now accepts slice, and DelayedPDActuator keeps it, but these implicit and ideal overrides annotate Sequence[int] | None. Use the base signature in every override.

[isaaclab-review-bot]

🟡 Warning · Implementation — Masked scene reset skips grippers

This caution promises mask precedence, but surface_gripper.reset(env_ids) below still receives ids only, so a mask-only scene reset clears every environment's gripper. Dispatch through reset_mask(env_mask).

[isaaclab-review-bot]

🟡 Warning · Test Quality — Tendon capture test asserts staged buffers

Assertions read data.fixed_tendon_, the buffers the setters write, so a broken copy_fixed_tendon_properties_with_mask kernel still passes. Assert the sim_bind_fixed_tendon buffers after replay.

[hujc7]

run-ci

[AntoineRichard]

Experimental review tooling 🤖

Thanks for opening this PR to IsaacLab! We're currently experimenting with automated review tooling, and we'd appreciate a little human time to review the review! The reviewer is under review, too.

Please reply with feedback on useful findings, false positives, and anything the tool missed.

Open Code Review v1.13.3 with Sol6.1 (azure/openai/gpt-6.1-sol, via NVIDIA Inference Hub) reviewed 93daa9b9bf62af55010f9a0a892a6d48017c8f2e.

Reasoning effort: high. Review effort: medium.

Result: OCR reported 6 observations. 4 potential issues are posted inline and 2 additional suggestions are summarized below.

Additional suggestions from the tool

• source/isaaclab_newton/test/sensors/test_newton_raycast_sensor.py:231 (medium, test): Independent uniform samples can legitimately compare equal under torch.allclose's default tolerance. For the scalar z drift in [0.1, 0.2], even distinct samples within roughly 1–2e-6 trigger a failure. Neither this test nor its simulation fixture fixes the RNG seed, so this assertion and its graph-replay counterpart can fail intermittently despite correct resampling. Use explicitly seeded expected samples, or verify RNG-state advancement without requiring every scalar draw to differ.
• source/isaaclab_newton/changelog.d/jichuanh-id-mask.changed.rst:6 (low, documentation): The mask helpers use resolve_1d_mask, not resolve_ids. Please distinguish the two here: resolve_ids(mask=...) converts masks to indices and synchronizes the host, whereas the updated mask writers preserve masks.

Scope: 90 of 95 changed files selected, including Python tests and .rst files. This was a static review; simulation tests were not run.

Review time: 9m16s. Token usage: 3,392,693.

[AntoineRichard]

Experimental review tooling · medium · bug

This redirect regresses negative-step slice selections exposed by ManagerBasedEnvWarp.resolve_env_mask. The previous experimental helper expanded slices with slice.indices() and range(), whereas the core helper uses wp.to_torch(scratch_mask)[ids] = True. PyTorch rejects negative slice steps, so inputs such as env_ids=slice(None, None, -2) now raise ValueError instead of producing a mask. Preserve negative-step handling in the core helper before switching the export, and add a resolve_1d_mask regression test; the new negative-step tests currently cover only resolve_ids.

[AntoineRichard]

Experimental review tooling · high · bug

Mask support is added only to the actuator collection, but Multirotor.reset() still accepts only env_ids. The updated InteractiveScene.reset(env_mask=...) forwards that keyword to every articulation, so a scene containing a multirotor raises TypeError before reaching this code. Update Multirotor.reset() to accept and forward env_mask, and use the same selection when restoring thrust_target. Add a regression test through the public reset method; the collection-only test bypasses this failure.

[AntoineRichard]

Experimental review tooling · medium · bug

Pass reset_mask to this kernel instead of the original env_mask. _resolve_indices_and_mask() copies a Warp mask to the sensor's device when necessary, but this launch bypasses that normalized result. For example, resetting a CUDA sensor with a CPU Warp mask and nonzero drift now fails with a device mismatch after its timing buffers have already been reset.

[AntoineRichard]

Experimental review tooling · medium · bug

This notification runs only during CUDA-graph capture, not on replay. add_model_change() adds to a Python set that NewtonManager.step() clears after notifying the solver. After replay → step → change inputs → replay → step, the simulation-bound tendon buffers update again, but the solver's separate tendon properties remain stale. Make the notification replay-aware (or capture the solver refresh), and extend the CUDA test to check solver.mjw_model after two replays separated by physics steps.

[hujc7]

run-ci