From 6044466990af2cb4b54f07f3705e37f16e6a4d58 Mon Sep 17 00:00:00 2001 From: hujc Date: Fri, 4 Sep 2026 14:10:46 -0700 Subject: [PATCH] [Fix] Set Warp's global determinism mode under --deterministic (#7561) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # 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//changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit 838fb6d66288cc5f452c3aa1b1ad57f420e0f0e4) --- docs/source/features/reproducibility.rst | 15 ++++- .../changelog.d/warp-determinism-global.rst | 9 +++ .../entrypoints/backends/play_rl_games.py | 3 + .../entrypoints/backends/play_rsl_rl.py | 3 + .../entrypoints/backends/play_sb3.py | 3 + .../entrypoints/backends/play_skrl.py | 3 + .../isaaclab_rl/entrypoints/common.py | 59 +++++++++++++++++-- .../test/test_entrypoints_common.py | 45 ++++++++++++++ 8 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 source/isaaclab_rl/changelog.d/warp-determinism-global.rst diff --git a/docs/source/features/reproducibility.rst b/docs/source/features/reproducibility.rst index 75f812290c7e..872ab360a8ab 100644 --- a/docs/source/features/reproducibility.rst +++ b/docs/source/features/reproducibility.rst @@ -35,7 +35,20 @@ for **RL-Games**, **skrl**, **RSL-RL**, and **Stable-Baselines3**: each calls so library initialization is not disturbed, then training proceeds with the requested global RNG and optional PyTorch deterministic algorithms. Whether the **rendering** half of the flag matters depends on the workload: **physics-only** simulation does not render at all; **RTX** rendering (non-minimal -mode) needs it for reproducible imagery; **Newton** rendering is already deterministic. +mode) needs it for reproducible imagery; **Newton** rendering needs Warp's global determinism mode, +which the flag sets (see :ref:`below `). + +.. _reproducibility-warp-determinism: + +**Warp determinism.** Newton's solvers accept a ``deterministic`` argument that Isaac Lab supplies +from :attr:`~isaaclab.physics.PhysicsCfg.deterministic`, and they apply it as a per-module option. +Its sensor and geometry kernels take no such argument and fall back to ``warp.config.deterministic``, +so the RL training and play entrypoints set that global to ``RUN_TO_RUN`` when +``--deterministic`` is passed. A script that builds its own environment without those entrypoints +must set ``warp.config.deterministic`` itself, before the environment is created. Without it the scene BVH is built +over an atomically compacted shape list whose order varies between processes, and a tiled camera can +render a few pixels differently from identical simulation state. An explicitly chosen mode, such as +``GPU_TO_GPU``, is left untouched. .. note:: diff --git a/source/isaaclab_rl/changelog.d/warp-determinism-global.rst b/source/isaaclab_rl/changelog.d/warp-determinism-global.rst new file mode 100644 index 000000000000..e018f755fa38 --- /dev/null +++ b/source/isaaclab_rl/changelog.d/warp-determinism-global.rst @@ -0,0 +1,9 @@ +Fixed +^^^^^ + +* Fixed ``--deterministic`` not making camera observations reproducible. The flag configured the + physics solver but left :attr:`warp.config.deterministic` at ``NOT_GUARANTEED``, and Newton's + sensor and geometry kernels -- unlike its solvers -- take no per-module determinism option and + fall back to that global. The scene BVH is built over an atomically compacted shape list, so its + primitive order varied between processes and a tiled camera rendered a few pixels differently + from identical simulation state, which was enough to make image-observation training diverge. diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rl_games.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rl_games.py index 36e91e81173c..a9cca1cff94e 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rl_games.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rl_games.py @@ -30,6 +30,7 @@ apply_video_recording, create_isaaclab_env, pre_launch_video_config, + request_determinism, resolve_checkpoint_selector, resolve_play_task_name, show_run_summary, @@ -107,6 +108,8 @@ def main(): train_task_name = task_name.replace("-Play", "") env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs + # Warp reads its determinism mode at module build time, so request it before the env exists. + request_determinism(args_cli, env_cfg) env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device if args_cli.seed == -1: diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py index 890f17da185a..43bc29399b78 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_rsl_rl.py @@ -29,6 +29,7 @@ apply_video_recording, create_isaaclab_env, pre_launch_video_config, + request_determinism, resolve_checkpoint_selector, resolve_play_task_name, show_run_summary, @@ -126,6 +127,8 @@ def main(env_cfg: ManagerBasedRLEnvCfg | DirectRLEnvCfg | DirectMARLEnvCfg, agen agent_cfg = cli_args.update_rsl_rl_cfg(agent_cfg, args_cli) env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs + # Warp reads its determinism mode at module build time, so request it before the env exists. + request_determinism(args_cli, env_cfg) agent_cfg = handle_deprecated_rsl_rl_cfg(agent_cfg, installed_version) diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_sb3.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_sb3.py index f2b69743063b..7474e94b5822 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_sb3.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_sb3.py @@ -27,6 +27,7 @@ apply_video_recording, create_isaaclab_env, pre_launch_video_config, + request_determinism, resolve_checkpoint_selector, resolve_play_task_name, show_run_summary, @@ -112,6 +113,8 @@ def main(): args_cli.seed = random.randint(0, 10000) env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs + # Warp reads its determinism mode at module build time, so request it before the env exists. + request_determinism(args_cli, env_cfg) agent_cfg["seed"] = args_cli.seed if args_cli.seed is not None else agent_cfg["seed"] env_cfg.seed = agent_cfg["seed"] env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_skrl.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_skrl.py index b493fbb2cabe..14be1888e0a7 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_skrl.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/backends/play_skrl.py @@ -32,6 +32,7 @@ create_isaaclab_env, pre_launch_video_config, preserve_attribute, + request_determinism, resolve_checkpoint_selector, resolve_play_task_name, show_run_summary, @@ -165,6 +166,8 @@ def _main(): train_task_name = task_name.replace("-Play", "") env_cfg.scene.num_envs = args_cli.num_envs if args_cli.num_envs is not None else env_cfg.scene.num_envs + # Warp reads its determinism mode at module build time, so request it before the env exists. + request_determinism(args_cli, env_cfg) env_cfg.sim.device = args_cli.device if args_cli.device is not None else env_cfg.sim.device # configure the ML framework into the global skrl variable diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py index a464be18975d..9a1fad8d60d3 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py @@ -493,10 +493,61 @@ def apply_env_overrides(args_cli: argparse.Namespace, env_cfg: Any, *, apply_dev # --deterministic is an AppLauncher flag, so it only reaches carb settings on its own. # Record the request on the resolved physics config; each backend translates and validates # it when the simulation starts. - if getattr(args_cli, "deterministic", False): - physics_cfg = getattr(getattr(env_cfg, "sim", None), "physics", None) - if physics_cfg is not None: - physics_cfg.deterministic = True + request_determinism(args_cli, env_cfg) + + +def request_determinism(args_cli: argparse.Namespace, env_cfg: Any) -> None: + """Record a ``--deterministic`` request on the config tree and on Warp's global. + + Call this before the environment is created: Warp reads its setting at module build time + and the scene BVH is built while the environment is constructed. + + Args: + args_cli: Parsed command-line arguments. + env_cfg: Isaac Lab environment config. + """ + if not getattr(args_cli, "deterministic", False): + return + physics_cfg = getattr(getattr(env_cfg, "sim", None), "physics", None) + if physics_cfg is not None: + physics_cfg.deterministic = True + request_warp_determinism(physics_cfg) + + +def request_warp_determinism(physics_cfg: Any) -> None: + """Ask Warp for deterministic atomics process-wide, matching the configured guarantee. + + Newton's solvers take a ``deterministic`` argument and apply it as a per-module option, so a + solver-level request already covers the physics kernels. Its sensor and geometry modules take + no such argument and fall back to ``warp.config.deterministic``, which defaults to + ``NOT_GUARANTEED``. One of them, the BVH shape compaction in ``newton._src.geometry.bvh``, + claims output slots with ``wp.atomic_add``, so the enabled-shape order -- and with it the + order of the primitives the scene BVH is built over -- varies between processes. Ray queries + then break ties differently and a tiled camera renders a handful of pixels differently from + identical simulation state, which is enough to make an image-observation policy diverge. + + A backend that names a stronger guarantee gets it here too: the strings accepted by + :attr:`~isaaclab_newton.physics.NewtonCfg.deterministic_mode` are the + ``warp.DeterministicMode`` members lowercased, so ``"gpu_to_gpu"`` selects + ``GPU_TO_GPU`` rather than being weakened to ``RUN_TO_RUN``. Warp reads the setting at module + build time, so it must land before the first kernel launch; :func:`apply_env_overrides` runs + before the environment is created. + + The modes are ordered by strength, and this only ever raises the setting. Reading the current + value cannot tell a deliberate choice from the shipped default, so rather than guess at intent + it never weakens a guarantee already in place -- whoever set it, they wanted at least that much. + + Args: + physics_cfg: Resolved physics config, or ``None`` when the config tree carries none. + """ + import warp as wp + + requested = getattr(physics_cfg, "deterministic_mode", None) + mode = getattr(wp.DeterministicMode, requested.upper(), None) if isinstance(requested, str) else None + if mode is None or mode == wp.DeterministicMode.NOT_GUARANTEED: + mode = wp.DeterministicMode.RUN_TO_RUN + if wp.config.deterministic < mode: + wp.config.deterministic = mode def validate_distributed_device(args_cli: argparse.Namespace) -> None: diff --git a/source/isaaclab_rl/test/test_entrypoints_common.py b/source/isaaclab_rl/test/test_entrypoints_common.py index be6b977a116c..8a1e9499c09b 100644 --- a/source/isaaclab_rl/test/test_entrypoints_common.py +++ b/source/isaaclab_rl/test/test_entrypoints_common.py @@ -420,6 +420,51 @@ def test_apply_env_overrides_leaves_physics_alone_without_the_flag(monkeypatch: assert env_cfg.sim.physics.deterministic is False +@pytest.mark.parametrize( + ("already_set", "configured_mode", "expected"), + [ + # The request lands when nothing has asked for a guarantee yet. + ("NOT_GUARANTEED", None, "RUN_TO_RUN"), + # "not_guaranteed" is the shipped default, so it reads as unset rather than opt-out. + ("NOT_GUARANTEED", "not_guaranteed", "RUN_TO_RUN"), + ("NOT_GUARANTEED", "run_to_run", "RUN_TO_RUN"), + # A backend naming a stronger guarantee gets it, not a weakened one. + ("NOT_GUARANTEED", "gpu_to_gpu", "GPU_TO_GPU"), + ("RUN_TO_RUN", "gpu_to_gpu", "GPU_TO_GPU"), + # A guarantee already in place is never lowered. + ("GPU_TO_GPU", None, "GPU_TO_GPU"), + ("GPU_TO_GPU", "run_to_run", "GPU_TO_GPU"), + ], +) +def test_apply_env_overrides_raises_warp_determinism_to_the_configured_mode( + already_set: str, configured_mode: str | None, expected: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """Warp's global is what covers Newton's sensor kernels, and it only ever moves upward.""" + import warp as wp + + monkeypatch.setattr(wp.config, "deterministic", getattr(wp.DeterministicMode, already_set)) + attrs = {} if configured_mode is None else {"deterministic_mode": configured_mode} + env_cfg = SimpleNamespace(sim=SimpleNamespace(physics=_fake_physics_cfg("NewtonCfg", **attrs))) + + args_cli = argparse.Namespace(num_envs=None, device=None, deterministic=True) + _rl_common.apply_env_overrides(args_cli, env_cfg, apply_device=False) + + assert wp.config.deterministic == getattr(wp.DeterministicMode, expected) + + +def test_apply_env_overrides_leaves_warp_alone_without_the_flag(monkeypatch: pytest.MonkeyPatch) -> None: + """Without ``--deterministic`` Warp keeps its default, so no run pays for determinism.""" + import warp as wp + + monkeypatch.setattr(wp.config, "deterministic", wp.DeterministicMode.NOT_GUARANTEED) + env_cfg = SimpleNamespace(sim=SimpleNamespace(physics=_fake_physics_cfg("NewtonCfg"))) + + args_cli = argparse.Namespace(num_envs=None, device=None, deterministic=False) + _rl_common.apply_env_overrides(args_cli, env_cfg, apply_device=False) + + assert wp.config.deterministic == wp.DeterministicMode.NOT_GUARANTEED + + @pytest.mark.parametrize("class_name", ["PhysxCfg", "OvPhysxCfg", "NewtonCfg", "SomeFutureBackendCfg"]) def test_apply_env_overrides_records_the_request_for_every_backend(class_name: str) -> None: """The request is backend-agnostic, so the entrypoint needs no per-backend knowledge."""