Skip to content

Commit 838fb6d

Browse files
authored
[Fix] Set Warp's global determinism mode under --deterministic (#7561)
# Description `--deterministic` did not make camera observations reproducible. Training `Isaac-Cartpole-Camera --deterministic` on an RTX PRO 6000 Blackwell splits into two bitwise-identical outcome clusters; on an L40 it does not. Reported as NVBug 6658578 against Isaac Lab 3.0 GA. Newton's solvers accept a `deterministic` argument and apply it as a per-module option, so PR #7334 already covered the physics kernels. Its **sensor and geometry kernels take no such argument** and fall back to `warp.config.deterministic`, which stayed at `NOT_GUARANTEED` — Isaac Lab has never set it (`git log --all -S "config.deterministic"` is empty). The concrete path: `newton._src.geometry.bvh.compute_enabled_shapes` claims output slots with `wp.atomic_add`, so the enabled-shape order — and with it the primitive order the scene BVH is built over — varies between processes. Ray queries then break ties differently, and a tiled camera renders a few pixels differently from bit-identical simulation state. That is enough to make an image-observation policy diverge. `apply_env_overrides` now also requests `wp.config.deterministic = RUN_TO_RUN`. Warp reads the setting at module build time and the BVH is built during `ModelBuilder.finalize()`, so the request must land before the environment is created — `_apply_deterministic_request` at solver init is too late. An explicitly chosen mode such as `GPU_TO_GPU` is left alone. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Validation Where the divergence starts, from the real training path with every renderer input fingerprinted per frame (6 runs, Blackwell). All geometric inputs are bit-identical at the frame where the output already differs: ``` cam_xform / cam_rays / shape_transform / shape_body : never diverge body_q, BVH bounds : render 390 (after) rendered colour (output) : render 389 (first) ``` `Isaac-Cartpole-Camera --deterministic`, epoch-25 reward, unique run dirs, 8 runs per arm: | Branch | Configuration | Runs | Distinct outcomes | |---|---|---|---| | develop | before this change | 8 | 2 | | develop | `wp.config.deterministic` set externally | 8 | 1 | | develop | **this change**, plain `--deterministic` | 8 | **1** | | release/3.0.0 | before | 8 | 2 | | release/3.0.0 | `wp.config` set | 8 | 1 | Architecture control on L40 (4gpu), 12 runs, unique run dirs: all identical — the defect does not reproduce on Ada, which is why PR #7334's L40-only validation missed it. Unit tests: 31 pass in `test_entrypoints_common.py`, including three added here covering the request, an explicit mode being preserved, and no change without the flag. Fixes NVBug 6658578. ## Follow-up (not in this PR) Newton-side: sensor and geometry modules should accept a `deterministic` option like the solvers do, rather than depending on a process global; and `compute_enabled_shapes` produces order-dependent output regardless of that setting. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] 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 - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
1 parent 7483997 commit 838fb6d

8 files changed

Lines changed: 135 additions & 5 deletions

File tree

docs/source/features/reproducibility.rst

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,20 @@ for **RL-Games**, **skrl**, **RSL-RL**, and **Stable-Baselines3**: each calls
3535
so library initialization is not disturbed, then training proceeds with the requested global RNG and
3636
optional PyTorch deterministic algorithms. Whether the **rendering** half of the flag matters depends
3737
on the workload: **physics-only** simulation does not render at all; **RTX** rendering (non-minimal
38-
mode) needs it for reproducible imagery; **Newton** rendering is already deterministic.
38+
mode) needs it for reproducible imagery; **Newton** rendering needs Warp's global determinism mode,
39+
which the flag sets (see :ref:`below <reproducibility-warp-determinism>`).
40+
41+
.. _reproducibility-warp-determinism:
42+
43+
**Warp determinism.** Newton's solvers accept a ``deterministic`` argument that Isaac Lab supplies
44+
from :attr:`~isaaclab.physics.PhysicsCfg.deterministic`, and they apply it as a per-module option.
45+
Its sensor and geometry kernels take no such argument and fall back to ``warp.config.deterministic``,
46+
so the RL training and play entrypoints set that global to ``RUN_TO_RUN`` when
47+
``--deterministic`` is passed. A script that builds its own environment without those entrypoints
48+
must set ``warp.config.deterministic`` itself, before the environment is created. Without it the scene BVH is built
49+
over an atomically compacted shape list whose order varies between processes, and a tiled camera can
50+
render a few pixels differently from identical simulation state. An explicitly chosen mode, such as
51+
``GPU_TO_GPU``, is left untouched.
3952

4053
.. note::
4154

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed ``--deterministic`` not making camera observations reproducible. The flag configured the
5+
physics solver but left :attr:`warp.config.deterministic` at ``NOT_GUARANTEED``, and Newton's
6+
sensor and geometry kernels -- unlike its solvers -- take no per-module determinism option and
7+
fall back to that global. The scene BVH is built over an atomically compacted shape list, so its
8+
primitive order varied between processes and a tiled camera rendered a few pixels differently
9+
from identical simulation state, which was enough to make image-observation training diverge.

source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rl_games.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
apply_video_recording,
3131
create_isaaclab_env,
3232
pre_launch_video_config,
33+
request_determinism,
3334
resolve_checkpoint_selector,
3435
resolve_play_task_name,
3536
show_run_summary,
@@ -107,6 +108,8 @@ def main():
107108
train_task_name = task_name.replace("-Play", "")
108109

109110
env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs
111+
# Warp reads its determinism mode at module build time, so request it before the env exists.
112+
request_determinism(args_cli, env_cfg)
110113
env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device
111114

112115
if args_cli.seed == -1:

source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
apply_video_recording,
3030
create_isaaclab_env,
3131
pre_launch_video_config,
32+
request_determinism,
3233
resolve_checkpoint_selector,
3334
resolve_play_task_name,
3435
show_run_summary,
@@ -126,6 +127,8 @@ def main(env_cfg: ManagerBasedRLEnvCfg | DirectRLEnvCfg | DirectMARLEnvCfg, agen
126127

127128
agent_cfg = cli_args.update_rsl_rl_cfg(agent_cfg, args_cli)
128129
env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs
130+
# Warp reads its determinism mode at module build time, so request it before the env exists.
131+
request_determinism(args_cli, env_cfg)
129132

130133
agent_cfg = handle_deprecated_rsl_rl_cfg(agent_cfg, installed_version)
131134

source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_sb3.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
apply_video_recording,
2828
create_isaaclab_env,
2929
pre_launch_video_config,
30+
request_determinism,
3031
resolve_checkpoint_selector,
3132
resolve_play_task_name,
3233
show_run_summary,
@@ -112,6 +113,8 @@ def main():
112113
args_cli.seed = random.randint(0, 10000)
113114

114115
env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs
116+
# Warp reads its determinism mode at module build time, so request it before the env exists.
117+
request_determinism(args_cli, env_cfg)
115118
agent_cfg["seed"] = args_cli.seed if args_cli.seed is not None else agent_cfg["seed"]
116119
env_cfg.seed = agent_cfg["seed"]
117120
env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device

source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_skrl.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
create_isaaclab_env,
3333
pre_launch_video_config,
3434
preserve_attribute,
35+
request_determinism,
3536
resolve_checkpoint_selector,
3637
resolve_play_task_name,
3738
show_run_summary,
@@ -165,6 +166,8 @@ def _main():
165166
train_task_name = task_name.replace("-Play", "")
166167

167168
env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs
169+
# Warp reads its determinism mode at module build time, so request it before the env exists.
170+
request_determinism(args_cli, env_cfg)
168171
env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device
169172

170173
# configure the ML framework into the global skrl variable

source/isaaclab_rl/isaaclab_rl/entrypoints/common.py

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,61 @@ def apply_env_overrides(args_cli: argparse.Namespace, env_cfg: Any, *, apply_dev
493493
# --deterministic is an AppLauncher flag, so it only reaches carb settings on its own.
494494
# Record the request on the resolved physics config; each backend translates and validates
495495
# it when the simulation starts.
496-
if getattr(args_cli, "deterministic", False):
497-
physics_cfg = getattr(getattr(env_cfg, "sim", None), "physics", None)
498-
if physics_cfg is not None:
499-
physics_cfg.deterministic = True
496+
request_determinism(args_cli, env_cfg)
497+
498+
499+
def request_determinism(args_cli: argparse.Namespace, env_cfg: Any) -> None:
500+
"""Record a ``--deterministic`` request on the config tree and on Warp's global.
501+
502+
Call this before the environment is created: Warp reads its setting at module build time
503+
and the scene BVH is built while the environment is constructed.
504+
505+
Args:
506+
args_cli: Parsed command-line arguments.
507+
env_cfg: Isaac Lab environment config.
508+
"""
509+
if not getattr(args_cli, "deterministic", False):
510+
return
511+
physics_cfg = getattr(getattr(env_cfg, "sim", None), "physics", None)
512+
if physics_cfg is not None:
513+
physics_cfg.deterministic = True
514+
request_warp_determinism(physics_cfg)
515+
516+
517+
def request_warp_determinism(physics_cfg: Any) -> None:
518+
"""Ask Warp for deterministic atomics process-wide, matching the configured guarantee.
519+
520+
Newton's solvers take a ``deterministic`` argument and apply it as a per-module option, so a
521+
solver-level request already covers the physics kernels. Its sensor and geometry modules take
522+
no such argument and fall back to ``warp.config.deterministic``, which defaults to
523+
``NOT_GUARANTEED``. One of them, the BVH shape compaction in ``newton._src.geometry.bvh``,
524+
claims output slots with ``wp.atomic_add``, so the enabled-shape order -- and with it the
525+
order of the primitives the scene BVH is built over -- varies between processes. Ray queries
526+
then break ties differently and a tiled camera renders a handful of pixels differently from
527+
identical simulation state, which is enough to make an image-observation policy diverge.
528+
529+
A backend that names a stronger guarantee gets it here too: the strings accepted by
530+
:attr:`~isaaclab_newton.physics.NewtonCfg.deterministic_mode` are the
531+
``warp.DeterministicMode`` members lowercased, so ``"gpu_to_gpu"`` selects
532+
``GPU_TO_GPU`` rather than being weakened to ``RUN_TO_RUN``. Warp reads the setting at module
533+
build time, so it must land before the first kernel launch; :func:`apply_env_overrides` runs
534+
before the environment is created.
535+
536+
The modes are ordered by strength, and this only ever raises the setting. Reading the current
537+
value cannot tell a deliberate choice from the shipped default, so rather than guess at intent
538+
it never weakens a guarantee already in place -- whoever set it, they wanted at least that much.
539+
540+
Args:
541+
physics_cfg: Resolved physics config, or ``None`` when the config tree carries none.
542+
"""
543+
import warp as wp
544+
545+
requested = getattr(physics_cfg, "deterministic_mode", None)
546+
mode = getattr(wp.DeterministicMode, requested.upper(), None) if isinstance(requested, str) else None
547+
if mode is None or mode == wp.DeterministicMode.NOT_GUARANTEED:
548+
mode = wp.DeterministicMode.RUN_TO_RUN
549+
if wp.config.deterministic < mode:
550+
wp.config.deterministic = mode
500551

501552

502553
def validate_distributed_device(args_cli: argparse.Namespace) -> None:

source/isaaclab_rl/test/test_entrypoints_common.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,51 @@ def test_apply_env_overrides_leaves_physics_alone_without_the_flag(monkeypatch:
420420
assert env_cfg.sim.physics.deterministic is False
421421

422422

423+
@pytest.mark.parametrize(
424+
("already_set", "configured_mode", "expected"),
425+
[
426+
# The request lands when nothing has asked for a guarantee yet.
427+
("NOT_GUARANTEED", None, "RUN_TO_RUN"),
428+
# "not_guaranteed" is the shipped default, so it reads as unset rather than opt-out.
429+
("NOT_GUARANTEED", "not_guaranteed", "RUN_TO_RUN"),
430+
("NOT_GUARANTEED", "run_to_run", "RUN_TO_RUN"),
431+
# A backend naming a stronger guarantee gets it, not a weakened one.
432+
("NOT_GUARANTEED", "gpu_to_gpu", "GPU_TO_GPU"),
433+
("RUN_TO_RUN", "gpu_to_gpu", "GPU_TO_GPU"),
434+
# A guarantee already in place is never lowered.
435+
("GPU_TO_GPU", None, "GPU_TO_GPU"),
436+
("GPU_TO_GPU", "run_to_run", "GPU_TO_GPU"),
437+
],
438+
)
439+
def test_apply_env_overrides_raises_warp_determinism_to_the_configured_mode(
440+
already_set: str, configured_mode: str | None, expected: str, monkeypatch: pytest.MonkeyPatch
441+
) -> None:
442+
"""Warp's global is what covers Newton's sensor kernels, and it only ever moves upward."""
443+
import warp as wp
444+
445+
monkeypatch.setattr(wp.config, "deterministic", getattr(wp.DeterministicMode, already_set))
446+
attrs = {} if configured_mode is None else {"deterministic_mode": configured_mode}
447+
env_cfg = SimpleNamespace(sim=SimpleNamespace(physics=_fake_physics_cfg("NewtonCfg", **attrs)))
448+
449+
args_cli = argparse.Namespace(num_envs=None, device=None, deterministic=True)
450+
_rl_common.apply_env_overrides(args_cli, env_cfg, apply_device=False)
451+
452+
assert wp.config.deterministic == getattr(wp.DeterministicMode, expected)
453+
454+
455+
def test_apply_env_overrides_leaves_warp_alone_without_the_flag(monkeypatch: pytest.MonkeyPatch) -> None:
456+
"""Without ``--deterministic`` Warp keeps its default, so no run pays for determinism."""
457+
import warp as wp
458+
459+
monkeypatch.setattr(wp.config, "deterministic", wp.DeterministicMode.NOT_GUARANTEED)
460+
env_cfg = SimpleNamespace(sim=SimpleNamespace(physics=_fake_physics_cfg("NewtonCfg")))
461+
462+
args_cli = argparse.Namespace(num_envs=None, device=None, deterministic=False)
463+
_rl_common.apply_env_overrides(args_cli, env_cfg, apply_device=False)
464+
465+
assert wp.config.deterministic == wp.DeterministicMode.NOT_GUARANTEED
466+
467+
423468
@pytest.mark.parametrize("class_name", ["PhysxCfg", "OvPhysxCfg", "NewtonCfg", "SomeFutureBackendCfg"])
424469
def test_apply_env_overrides_records_the_request_for_every_backend(class_name: str) -> None:
425470
"""The request is backend-agnostic, so the entrypoint needs no per-backend knowledge."""

0 commit comments

Comments
 (0)