[Exp] Cherry-pick manager-based warp env infrastructure from dev/newton

[hujc7]

Summary

Cherry-pick of warp manager-based env infrastructure from dev/newton, refactored for develop.

isaaclab_experimental

• Added warp-compatible manager implementations (ActionManager, ObservationManager, EventManager,
  CommandManager, TerminationManager, RewardManager) with Warp kernel execution and CUDA graph
  capture support.
• Added ManagerCallSwitch utility for per-manager eager/captured dispatch, configured via
  MANAGER_CALL_CONFIG env var.
• Added ManagerBasedEnvWarp and ManagerBasedRLEnvWarp orchestration env classes.
• Added warp MDP terms (observations, rewards, terminations, events, joint actions).
• Added utility modules: buffers (circular buffer), modifiers, noise models, warp kernels/helpers.
• Added experimental SceneEntityCfg with warp joint mask/ids for kernel-level joint selection.
• Generalized configclass default materialization in ManagerBase for automatic SceneEntityCfg resolution.

isaaclab_tasks_experimental

• Added Isaac-Cartpole-Warp-v0 task as reference environment for warp manager-based workflow.

isaaclab_rl

• Updated rsl_rl, rl_games, sb3, skrl wrappers to accept ManagerBasedRLEnvWarp and DirectRLEnvWarp.

isaaclab

• Fixed SettingsManager to catch RuntimeError when carb is unavailable.
• Minor comment cleanup in ObservationManager.

Dependencies

Must be merged after:

1. [Exp] Cherry-pick direct warp envs from dev/newton #4905 (merged)

Validated base

Validated against develop at 7588fa9ed5f.

Known limitations

• Scene_write_data_to_sim capped to mode=1 (eager) via MAX_MODE_OVERRIDES — articulation
  _apply_actuator_model uses wp.to_torch + torch indexing, not CUDA graph capture-safe.

Test plan

• Isaac-Cartpole-Warp-v0 training (4096 envs, 300 iters, mode=2): converges (reward 4.95, ep_len 300)

[hujc7]

@greptile review

[hujc7]

WIP but put in review for bot rebiew.

[greptile-apps]

Greptile Summary

This PR cherry-picks and refactors the Warp manager-based RL environment infrastructure from dev/newton into develop, introducing Warp-compatible manager implementations (ActionManager, ObservationManager, EventManager, RewardManager, TerminationManager), CUDA graph capture/replay via ManagerCallSwitch and WarpGraphCache, orchestration classes ManagerBasedEnvWarp/ManagerBasedRLEnvWarp, Warp MDP terms, and the Isaac-Cartpole-Warp-v0 reference task.

Most issues raised in prior review rounds have been addressed:

• MAX_MODE_OVERRIDES now applied on both the default-config and env-var paths in _load_cfg
• TIMER_ENABLED_STEP correctly gates the outer step() @Timer decorator
• STABLE mode guard raises ValueError with an actionable message
• WarpGraphCache performs an eager warm-up run before capture so first-call GPU execution is correct
• assert → raise RuntimeError/ValueError fixes applied in events.py, terminations.py, event_manager.py, and manager_based_rl_env_warp.py
• Scene added to MANAGER_NAMES so its capped mode is visible in the init printout

Two remaining issues were identified:

• envs/mdp/rewards.py — joint_vel_l1 still passes asset_cfg.joint_mask directly to the Warp kernel without checking for None. The identical guard was applied to terminations.py earlier in this PR but not here.
• cartpole/mdp/rewards.py — joint_pos_target_l2 uses a bare assert that is silently disabled with -O and will raise a confusing AttributeError (rather than a clear ValueError) if joint_mask is None.

Confidence Score: 4/5

• Safe to merge after fixing the two joint_mask guard omissions in the MDP reward terms.
• The vast majority of prior review concerns are resolved. Two P1 issues remain: a missing null guard for joint_mask in joint_vel_l1 (same pattern already fixed in terminations.py) and an unsafe assert in the cartpole task's joint_pos_target_l2 that will produce a confusing AttributeError if a wrong config is passed. Both are small, targeted fixes with clear solutions.
• source/isaaclab_experimental/isaaclab_experimental/envs/mdp/rewards.py and source/isaaclab_tasks_experimental/isaaclab_tasks_experimental/manager_based/classic/cartpole/mdp/rewards.py

Important Files Changed

Filename	Overview
source/isaaclab_experimental/isaaclab_experimental/envs/mdp/rewards.py	Missing joint_mask null guard in joint_vel_l1 before Warp kernel launch — the same guard applied in terminations.py was not applied here.
source/isaaclab_tasks_experimental/isaaclab_tasks_experimental/manager_based/classic/cartpole/mdp/rewards.py	joint_pos_target_l2 uses a bare assert for runtime validation that is silently stripped with -O and will also crash with AttributeError if joint_mask is None.
source/isaaclab_experimental/isaaclab_experimental/utils/manager_call_switch.py	Per-manager call switch with STABLE/WARP_NOT_CAPTURED/WARP_CAPTURED modes; prior issues (STABLE crash guard, Scene cap visibility, default-config MAX_MODE_OVERRIDES) all addressed.
source/isaaclab_experimental/isaaclab_experimental/utils/warp_graph_cache.py	CUDA graph cache with eager warm-up + capture pattern; first-call execution issue resolved via warm-up run before capture.
source/isaaclab_experimental/isaaclab_experimental/envs/manager_based_rl_env_warp.py	RL env Warp entry point; timer enable=TIMER_ENABLED_STEP fix confirmed, assert→RuntimeError fix confirmed, action buffer stable-pointer design correct.
source/isaaclab_experimental/isaaclab_experimental/envs/manager_based_env_warp.py	Base Warp env; MAX_MODE_OVERRIDES application on both config paths confirmed, shared ENV_MASK latent issue acknowledged and tracked.
source/isaaclab_experimental/isaaclab_experimental/managers/reward_manager.py	Warp-compatible reward manager with single-kernel finalize; buffer layout (num_terms×num_envs for term_outs, num_envs×num_terms for step_reward) is consistent throughout.
source/isaaclab_experimental/isaaclab_experimental/managers/event_manager.py	assert→raise RuntimeError fixes confirmed in _apply_interval and _apply_reset; per-term captured event dispatch looks correct.
source/isaaclab_experimental/isaaclab_experimental/envs/mdp/terminations.py	joint_mask null guard and shape check correctly added via explicit raise ValueError; consistent with the fix pattern requested in prior review.

Sequence Diagram

sequenceDiagram
    participant RL as RL Library
    participant Env as ManagerBasedRLEnvWarp
    participant MCS as ManagerCallSwitch
    participant WGC as WarpGraphCache
    participant Mgr as Warp Managers

    RL->>Env: step(action)
    Env->>Env: wp.copy(_action_in_wp)
    Env->>MCS: call_stage("ActionManager_process_action")
    MCS->>Mgr: WARP_CAPTURED → WarpGraphCache.capture_or_replay()
    Note over WGC: 1st call: warm-up + capture<br/>2nd+ call: wp.capture_launch()

    loop decimation
        Env->>MCS: call_stage("ActionManager_apply_action")
        Env->>MCS: call_stage("Scene_write_data_to_sim")
        Note over MCS: Scene capped at WARP_NOT_CAPTURED
        Env->>Env: sim.step()
        Env->>Env: scene.update()
    end

    Env->>MCS: call_stage("TerminationManager_compute")
    Env->>MCS: call_stage("RewardManager_compute")
    Env->>Env: _reset_idx(reset_env_ids)
    Env->>MCS: call_stage("EventManager_apply_interval")
    Env->>MCS: call_stage("ObservationManager_compute_update_history")
    Env-->>RL: obs, reward, terminated, truncated, extras

Loading

_{Reviews (10): Last reviewed commit: "Add warm-up before CUDA graph capture an..." | Re-trigger Greptile}

[greptile-apps]

MAX_MODE_OVERRIDES not applied for default config.

When cfg_source is None (the common case without --manager_call_config), _load_cfg returns early at line 162 without applying MAX_MODE_OVERRIDES. The PR description and class comment both state that Scene_write_data_to_sim must be capped at WARP_NOT_CAPTURED (mode 1) because _apply_actuator_model is not CUDA-graph-capture-safe. With the default config {"default": 2}, get_mode_for_manager("Scene") will return 2, bypassing the cap entirely and allowing unsafe CUDA graph capture.

The fix is to apply MAX_MODE_OVERRIDES on the default-config path:

Suggested change

	def _load_cfg(self, cfg_source: str | None) -> dict[str, int]:
	if cfg_source is not None and not isinstance(cfg_source, str):
	raise TypeError(f"cfg_source must be a string or None, got: {type(cfg_source)}")
	if cfg_source is None or cfg_source.strip() == "":
	return dict(self.DEFAULT_CONFIG)
	def _load_cfg(self, cfg_source: str | None) -> dict[str, int]:
	if cfg_source is not None and not isinstance(cfg_source, str):
	raise TypeError(f"cfg_source must be a string or None, got: {type(cfg_source)}")
	if cfg_source is None or cfg_source.strip() == "":
	cfg = dict(self.DEFAULT_CONFIG)
	# Apply caps even for the default config
	default_mode = cfg[self.DEFAULT_KEY]
	for name, max_mode in self.MAX_MODE_OVERRIDES.items():
	resolved = cfg.get(name, default_mode)
	if resolved > max_mode:
	cfg[name] = max_mode
	return cfg

[hujc7]

Done. Restructured _load_cfg so both paths (default config and parsed env var) flow into the shared MAX_MODE_OVERRIDES application at the end.

[greptile-apps]

@Timer(enable=True) hardcoded to always-on for stable step().

The decorator is hardcoded to enable=True, meaning it will print timing output to stdout on every environment step during training. The module-level constant TIMER_ENABLED_STEP = False was clearly introduced to gate timing output globally, but this decorator bypasses it. All inner context-manager timers correctly respect TIMER_ENABLED_STEP (lines 184, 191, 209, etc.), making this inconsistency appear unintentional.

This will generate extremely verbose and undesirable output for all users of the stable environment.

Suggested change

	@Timer(name="env_step", msg="Step took:", enable=True, format="us")
	def step(self, action: torch.Tensor) -> VecEnvStepReturn:
	@Timer(name="env_step", msg="Step took:", enable=TIMER_ENABLED_STEP, format="us")
	def step(self, action: torch.Tensor) -> VecEnvStepReturn:

[hujc7]

Not in scope — this comment is about the stable env (source/isaaclab/isaaclab/envs/manager_based_rl_env.py), which is not modified in this PR.

[hujc7]

@greptileai Review

[greptile-apps]

Outer step() timer always enabled, ignores TIMER_ENABLED_STEP

The @Timer decorator on step() has enable=True hardcoded, which means the outer step timer prints "Step took: …" to stdout on every environment step during training — even when neither DEBUG_TIMER_STEP nor DEBUG_TIMERS env vars are set.

The module-level constant TIMER_ENABLED_STEP was specifically introduced to gate this output (DEBUG_TIMER_STEP = os.environ.get("DEBUG_TIMER_STEP", "0") == "1"), but it is ignored here. All inner timers (lines 230, 265, 274, etc.) correctly respect TIMER_ENABLED_STEP. The inconsistency appears unintentional and will produce extremely verbose training output for all users.

Suggested change

	@Timer(name="env_step", msg="Step took:", enable=True, format="us")
	@Timer(name="env_step", msg="Step took:", enable=TIMER_ENABLED_STEP, format="us")

[hujc7]

Done. Changed to enable=TIMER_ENABLED_STEP.

[greptile-apps]

STABLE mode (mode=0) crashes unconditionally — stable_call is never provided by call sites

When mode == ManagerCallMode.STABLE, _dispatch immediately calls self._run_call(stable_call). However, stable_call defaults to None in call_stage, and every call site in manager_based_rl_env_warp.py (e.g. ActionManager_process_action, ActionManager_apply_action, Scene_write_data_to_sim, TerminationManager_compute, RewardManager_compute, ObservationManager_compute_update_history, etc.) omits stable_call entirely.

_run_call(None) will immediately raise TypeError: 'NoneType' object is not subscriptable because it indexes call["fn"]. This means configuring any manager — or "default" — to mode 0 via MANAGER_CALL_CONFIG will crash the training loop, despite STABLE mode being documented as a supported execution path that routes to isaaclab.managers.

A guard should be added so the failure is caught early and the error message is actionable:

if mode == ManagerCallMode.STABLE:
    if stable_call is None:
        raise ValueError(
            f"Stage '{stage}' requested STABLE mode but no stable_call was provided."
        )
    call, result = stable_call, self._run_call(stable_call)

[hujc7]

Done. Added a guard that raises ValueError with an actionable message when stable_call is None.

[hujc7]

@greptileai Review

[greptile-apps]

assert for runtime validation is unsafe

These assert statements are used for runtime validation, but assertions are silently skipped when Python is run with the -O (optimize) flag. If a caller passes a regular isaaclab.managers.SceneEntityCfg (which lacks joint_ids_wp), or an env without rng_state_wp, the failure will produce a cryptic AttributeError instead of a clear message.

Replace with explicit exception raises:

Suggested change

	assert asset_cfg.joint_ids_wp is not None
	assert env.rng_state_wp is not None
	if asset_cfg.joint_ids_wp is None:
	raise ValueError(
	f"reset_joints_by_offset requires an experimental SceneEntityCfg with resolved joint_ids_wp, "
	f"but got None for asset '{asset_cfg.name}'. "
	"Use isaaclab_experimental.managers.SceneEntityCfg and ensure joint_names are set."
	)
	if not hasattr(env, "rng_state_wp") or env.rng_state_wp is None:
	raise AttributeError(
	"reset_joints_by_offset requires env.rng_state_wp to be initialized. "
	"Use ManagerBasedEnvWarp or ManagerBasedRLEnvWarp as the base environment."
	)

The same applies to reset_joints_by_scale at lines 153–154.

[hujc7]

Done. Replaced assert with explicit raise ValueError/AttributeError with actionable messages in both reset_joints_by_offset and reset_joints_by_scale.

[greptile-apps]

TYPE_CHECKING import inside contextlib.suppress leaves names undefined for type checkers

The contextlib.suppress(ImportError) pattern at runtime is correct, but under TYPE_CHECKING, static analysis tools (mypy, pyright) also evaluate this block literally. If the import fails in that context, DirectRLEnvWarp and ManagerBasedRLEnvWarp will be undefined when the type checker evaluates the unwrapped return annotation on line 132.

The same pattern appears in source/isaaclab_rl/isaaclab_rl/rl_games/rl_games.py, source/isaaclab_rl/isaaclab_rl/sb3.py, and source/isaaclab_rl/isaaclab_rl/skrl.py.

A cleaner alternative is to use a plain try/except ImportError that assigns stub type aliases when the import fails:

if TYPE_CHECKING:
    try:
        from isaaclab_experimental.envs import DirectRLEnvWarp, ManagerBasedEnvWarp, ManagerBasedRLEnvWarp
    except ImportError:
        from isaaclab.envs import ManagerBasedRLEnv as DirectRLEnvWarp  # type: ignore[assignment]
        from isaaclab.envs import ManagerBasedRLEnv as ManagerBasedEnvWarp  # type: ignore[assignment]
        from isaaclab.envs import ManagerBasedRLEnv as ManagerBasedRLEnvWarp  # type: ignore[assignment]

[hujc7]

Low priority — not fixing in this PR. The contextlib.suppress pattern is consistent with the other RL wrappers (rl_games, sb3, skrl) and works correctly at runtime.

[hujc7]

Latest changes

Addressed review feedback:

• Replaced assert with explicit raise ValueError/AttributeError in reset_joints_by_offset and reset_joints_by_scale (events.py)
• Switched Timer import from isaaclab_experimental.utils.timer to isaaclab.utils.timer (3 files) and fixed format= → time_unit= kwarg to match stable API
• Aligned manager-based Cartpole newton solver config with stable CartpolePhysicsCfg.newton (removed stale ls_iterations, ls_parallel)
• Dropped redundant inhand manipulation commit (already in base PR [Exp] Cherry-pick direct warp envs from dev/newton #4905)

Verification

Cartpole Warp (manager-based), 300 iterations, 4096 envs, newton==1.0.0:

• All managers running in mode 2 (WARP_CAPTURED)
• Final episode length: 300.00 (solved)
• Training time: 73s

[hujc7]

@greptileai Review

[hujc7]

@greptileai Review

[greptile-apps]

assert in hot-path methods — inconsistent with fixes applied elsewhere in this PR

_apply_interval (and _apply_reset at line 370) still use assert for internal invariant checks:

assert self._env.rng_state_wp is not None       # line 331
assert self._interval_global_rng_state_wp is not None  # line 337
assert self._scratch_term_mask_wp is not None   # line 370

Python silently strips all assert statements when running with the -O (optimize) flag, which is common in production training scripts. These are the exact same patterns that were already fixed in this PR (in events.py and terminations.py) by replacing them with explicit raise. The inconsistency is easy to miss since these live in the manager implementation while the prior fixes were in MDP term functions.

The conditions themselves are always True under normal construction order (all fields are set in __init__ before _apply_* can be called), so these won't trigger in practice. However, if initialization order changes during future refactoring, the failure will be a cryptic TypeError/AttributeError deep inside a Warp kernel rather than a clear message at the assertion site.

For consistency with the rest of this PR, consider replacing with explicit RuntimeError raises — e.g.:

if self._env.rng_state_wp is None:
    raise RuntimeError("EventManager._apply_interval: env.rng_state_wp is not initialized.")

The same applies to line 455 (_prepare_terms), which is a one-time init path but is still an assert.

[hujc7]

Done — all asserts already replaced with raise RuntimeError in the latest push.

[hujc7]

@greptileai Review

[greptile-apps]

"Scene" stages invisible in configuration printout despite having a hard cap

MAX_MODE_OVERRIDES enforces Scene → WARP_NOT_CAPTURED (mode 1), which is a critical constraint documented in the PR description. However, "Scene" is not in MANAGER_NAMES, so neither the default mode for Scene stages nor the cap ever appears in the printed configuration block. A user reviewing logs to understand why their Scene_write_data_to_sim stages aren't captured will find no evidence of the cap.

Consider appending the overrides section to the printout:

# After the manager-names loop:
if self._max_modes:
    print("  [MAX_MODE_OVERRIDES]")
    for name, max_mode in self._max_modes.items():
        resolved = int(self.get_mode_for_manager(name))
        print(f"  - {name}: {resolved} (cap={max_mode})")

[hujc7]

Done. Added "Scene" to MANAGER_NAMES so the cap is visible in the init printout.

[greptile-apps]

reset() bypasses _manager_call_switch for observation compute

ManagerBasedEnvWarp.reset() (inherited by ManagerBasedRLEnvWarp) calls observation_manager.compute() directly, bypassing the ManagerCallSwitch. This means the reset-time observation compute is always eager — even when ObservationManager is configured for WARP_CAPTURED (mode 2). The step() path correctly uses call_stage, but reset() does not.

This is inconsistent and could lead to subtle differences between reset and step time observations (e.g., no CUDA-graph warmup on the first reset). Wrapping this call in call_stage (or at minimum adding a code comment explaining the intentional bypass) would prevent confusion for future maintainers.

[hujc7]

Acknowledged — reset() calls observation_manager.compute() directly. This is the base env reset() path, called once at initialization. The RL env step() routes through ManagerCallSwitch correctly. Low risk, will align in a follow-up.

[hujc7]

@greptileai Review

[greptile-apps]

Missing joint_mask null guard before Warp kernel launch

joint_vel_l1 passes asset_cfg.joint_mask directly to the Warp kernel without checking whether it is None. If a caller passes a plain isaaclab.managers.SceneEntityCfg (which lacks joint_mask), this will crash inside the Warp runtime with an unhelpful error about a null array pointer rather than a clear message at the call site.

The same pattern was already fixed in terminations.py for joint_pos_out_of_manual_limit (lines 75-83), which explicitly checks asset_cfg.joint_mask is None and raises a descriptive ValueError. The same guard is needed here for consistency:

Suggested change

	def joint_vel_l1(env: ManagerBasedRLEnv, out, asset_cfg: SceneEntityCfg) -> None:
	"""Penalize joint velocities on the articulation using an L1-kernel. Writes into ``out``."""
	asset: Articulation = env.scene[asset_cfg.name]
	wp.launch(
	kernel=_sum_abs_masked_kernel,
	dim=env.num_envs,
	inputs=[asset.data.joint_vel, asset_cfg.joint_mask, out],
	device=env.device,
	)
	def joint_vel_l1(env: ManagerBasedRLEnv, out, asset_cfg: SceneEntityCfg) -> None:
	"""Penalize joint velocities on the articulation using an L1-kernel. Writes into ``out``."""
	asset: Articulation = env.scene[asset_cfg.name]
	if asset_cfg.joint_mask is None:
	raise ValueError(
	f"joint_vel_l1 requires SceneEntityCfg with resolved joint_mask, "
	f"but got None for asset '{asset_cfg.name}'."
	)
	wp.launch(
	kernel=_sum_abs_masked_kernel,
	dim=env.num_envs,
	inputs=[asset.data.joint_vel, asset_cfg.joint_mask, out],
	device=env.device,
	)

[greptile-apps]

Base step() bypasses ManagerCallSwitch entirely

ManagerBasedEnvWarp.step() (the base class, non-RL) calls action_manager.process_action, action_manager.apply_action, scene.write_data_to_sim, event_manager.apply, and observation_manager.compute all directly, without routing through self._manager_call_switch. This means any ManagerCallSwitch configuration (including WARP_CAPTURED mode) has no effect when this step() path is taken.

Since ManagerBasedRLEnvWarp overrides step() and does use ManagerCallSwitch, this only affects direct users of ManagerBasedEnvWarp. However, given the class name suggests Warp-mode support, a developer who instantiates it directly expecting WARP_CAPTURED behavior would be surprised to find it never captures. A comment explaining this limitation would prevent confusion:

def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]:
    """Execute one time-step of the environment's dynamics.

    Note:
        This base-class step runs all manager calls eagerly and does **not**
        route through :attr:`_manager_call_switch`. CUDA graph capture is only
        available via :class:`ManagerBasedRLEnvWarp` which overrides this method.
    ...

[hujc7]

Hi, @ooctipus. This PR still needs some attention to get merged.

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot — PR #4829

Summary

Cherry-pick of Warp manager-based env infrastructure from dev/newton. This is a large PR (48 files, ~8k lines) adding experimental Warp-first manager implementations, MDP terms, utilities, and a Cartpole reference task.

Overall Assessment: ✅ Approve with minor suggestions

The PR is well-structured: new code lives entirely under isaaclab_experimental (no stable code disruption), the ManagerCallSwitch design allows per-manager fallback to stable implementations, and the WarpGraphCache warm-up-before-capture pattern is a solid improvement. The cherry-pick is clean with no conflict artifacts visible.

CI Status

• ✅ labeler passed
• ⚠️ No pre-commit/lint/ruff checks found for the head commit — suggest verifying linting passes locally.

Key Findings

Architecture (positive)

• Clean separation: all experimental code in isaaclab_experimental, stable code only gets 2 minor comment/docstring cleanups.
• ManagerCallSwitch with env-var-driven mode selection (MANAGER_CALL_CONFIG) is flexible for benchmarking stable vs warp vs captured paths.
• WarpGraphCache now does eager warm-up before capture — correctly handles first-call allocations outside capture context.
• RL library wrappers (rsl_rl, rl_games, sb3, skrl) updated uniformly with graceful ImportError handling.

Potential Issues (see inline comments)

1. _reset_idx signature mismatch in ManagerBasedRLEnvWarp — the base ManagerBasedEnvWarp._reset_idx takes env_ids only, but the RL env override adds env_mask kwarg. The base reset() calls _reset_idx(env_ids) without env_mask, meaning the mask codepath in the RL env is never exercised from reset().
2. resolve_1d_mask allocates wp.array from Python list when ids are not torch/warp — this defeats capture-safety for the slice → list(range(...)) path.
3. Observation dim inference in _infer_term_dim_scalar is fragile — falls back to wp.to_torch(asset.data.joint_pos).shape[1] which does a warp→torch conversion at init time.
4. episode_length_buf setter uses self._episode_length_buf[:] = value which triggers a full copy — fine for correctness but worth documenting the intent (preserve warp linkage).
5. recorder_manager not reset with mask — in _reset_idx of ManagerBasedRLEnvWarp, recorder_manager gets env_ids (torch) while all other managers get env_mask (warp). This inconsistency could cause issues if recorder_manager is ever captured.
6. NoiseModelCfg.rng_state_wp is set by side-effect during _prepare_terms — this coupling between observation manager and noise config is implicit and could break if noise configs are shared across managers.

[isaaclab-review-bot]

Minor: RNG seed -1 when cfg.seed is None

When cfg.seed is None, seed_val is set to -1 which is passed to wp.rand_init(seed, env_id). The wp.rand_init function may handle negative seeds differently across Warp versions. Consider defaulting to 0 or 42 instead of -1, or documenting the intent.

seed_val = int(self.cfg.seed) if self.cfg.seed is not None else -1

[isaaclab-review-bot]

Signature mismatch: _reset_idx in RL env vs base env

The base ManagerBasedEnvWarp._reset_idx(self, env_ids) has no env_mask parameter, but this override adds env_mask as a keyword argument. When the base class reset() method calls self._reset_idx(env_ids), the env_mask kwarg is never passed, so the mask-based codepath in this method is never exercised from reset(). This is fine for now (base reset() only calls from external API), but worth a # NOTE to prevent confusion.

[isaaclab-review-bot]

Inconsistency: recorder_manager.reset gets env_ids (torch) while others get env_mask (warp)

All other manager resets in this method use env_mask (warp boolean mask), but recorder_manager.reset(env_ids=env_ids) still uses torch integer indices. This is noted as intentional (recorder is still env_ids-based), but it means recorder cannot be captured. Consider adding a # TODO to track migration.

[isaaclab-review-bot]

Capture-safety: wp.array(ids, ...) allocates during resolve_1d_mask

When ids is a Python list (e.g. from slice → list(range(...))), this line allocates a new wp.array, which is not safe during CUDA graph capture. The function comment says "No allocations happen inside this function" but that's not true for this path.

ids_wp = wp.array(ids, dtype=wp.int32, device=device)

Consider either:

1. Pre-allocating a scratch ids buffer alongside scratch_mask, or
2. Documenting that the slice → list path is not capture-safe.

[isaaclab-review-bot]

Fragile dim inference: wp.to_torch conversion at init

The fallback wp.to_torch(asset.data.joint_pos).shape[1] performs a warp→torch conversion just to get a shape. Consider using asset.data.joint_pos.shape[1] directly (Warp arrays have a .shape attribute), which avoids the conversion overhead:

return int(asset.data.joint_pos.shape[1])

[isaaclab-review-bot]

History + Warp: NotImplementedError raised at init

The raise NotImplementedError("History reshaping is not implemented yet for warp.") at line 695 will prevent any observation term with history_length > 0 from being used. This is fine as a guard, but the code above it (lines 681-694) still executes and creates a CircularBuffer that will never be used. Consider moving the raise before the buffer creation to avoid wasted allocation.

[isaaclab-review-bot]

Good improvement: eager warm-up before capture

The addition of fn(*args, **kwargs) before wp.ScopedCapture() is the right approach — it flushes first-call allocations (hasattr guards, lazy dtype casts) outside the capture context. This matches the pattern recommended by NVIDIA for CUDA graph capture.

One edge case: if fn has side effects that should only happen once (e.g. incrementing a counter), the warm-up + capture run will execute them twice. The PR description mentions this is intentional for the current use case, but worth a docstring note.

[isaaclab-review-bot]

RNG state mutation: single thread per env is correct for race-freedom

Good design choice — using 1 thread per env with a sequential loop over joints avoids RNG state races. The comment documents this well. However, for articulations with many joints (e.g. humanoids with 20+ joints), this serialization could become a bottleneck. Consider noting this as a known limitation for future optimization (e.g. per-env per-joint RNG states).

[isaaclab-review-bot]

_reward_finalize kernel: step_reward uses weighted but not dt-scaled values

The kernel stores weighted = raw * weight into step_reward (the "reward rate"), but then val = weighted * dt goes into episode_sums and total. The comment says // store weighted reward rate (matches old: value/dt) but the stored value is actually raw * weight, not raw * weight / dt. The naming/comment could be clarified to avoid confusion — it's weighted_per_step, not a rate.

[isaaclab-review-bot]

is_rendering check differs from base class

Base ManagerBasedEnvWarp.step() uses:

is_rendering = bool(self.sim.settings.get("/isaaclab/visualizer")) or self.sim.settings.get("/isaaclab/render/rtx_sensors")

But this RL env uses:

is_rendering = self.sim.is_rendering

The commit message mentions "align warp env rendering checks with stable env: use sim.is_rendering". The base class ManagerBasedEnvWarp.step() should likely be updated too for consistency, or the difference documented.

[hujc7]

@greptileai Review