[Fix] Make --deterministic reproduce training runs by configuring the physics backend - #7334
Conversation
The flag configured PyTorch and the Isaac RTX renderer but never reached the physics solver. Two defaults moved out from under it between 3.0beta2 and GA: classic core tasks switched to Newton (isaac-sim#7066) and the default renderer to Newton+Warp, so on Isaac-Cartpole-Camera the flag became a no-op for physics while NewtonCfg.deterministic_mode stayed "not_guaranteed". Training reward curves therefore diverged run to run even with --deterministic passed. Translate the flag onto the resolved physics config in apply_env_overrides, the existing CLI-to-cfg seam, which runs after scan() has resolved the backend to a concrete config and before the solver is constructed. Dispatch on the config type name so this module still imports without the optional backend packages installed. Backends that cannot honour the guarantee now fail at config time instead of deep inside launch.
github-actions
Bot
added
documentation
Greptile SummaryThe PR extends the training
Confidence Score: 3/5The PR should not merge until supported Newton solver subclasses stop being rejected and the documented explicit determinism opt-out is either honored or corrected. The new configuration seam breaks valid subclass-based Newton configurations at startup and silently overwrites an explicit not_guaranteed mode despite documenting that setting as an opt-out. Files Needing Attention: source/isaaclab_rl/isaaclab_rl/entrypoints/common.py, source/isaaclab_rl/test/test_entrypoints_common.py, source/isaaclab_rl/changelog.d/deterministic-flag-wires-physics.rst Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
CLI[--deterministic] --> Scan[Resolve physics backend]
Scan --> Override[apply_env_overrides]
Override --> PhysX[Enable enhanced determinism]
Override --> Newton{Newton solver supported?}
Newton -->|Yes| Mode[Select run_to_run mode]
Newton -->|MJWarp| Sensors[Disable internal MJWarp sensors]
Newton -->|No| Error[Raise configuration error]
PhysX --> Env[Construct environment]
Mode --> Env
Sensors --> Env
Reviews (1): Last reviewed commit: "Wire --deterministic into the resolved p..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| solver_name = type(solver_cfg).__name__ | ||
| if solver_name not in _DETERMINISTIC_NEWTON_SOLVERS: | ||
| raise ValueError( | ||
| f"--deterministic is not supported by {solver_name}. Use MJWarp on the GPU, XPBD, or" | ||
| " Featherstone, or drop --deterministic." | ||
| ) |
There was a problem hiding this comment.
When a Newton configuration uses a subclass of MJWarpSolverCfg, XPBDSolverCfg, or FeatherstoneSolverCfg, this exact-name check rejects it, causing training with --deterministic to abort even though Newton's runtime accepts supported solver subclasses through isinstance.
There was a problem hiding this comment.
Agreed. More fundamentally, this validation duplicates NewtonManager._validate_deterministic_solver_cfg(), which already uses isinstance and owns the backend-specific error messages. Please remove the solver whitelist/rejection logic from the RL layer: set the requested mode on the resolved config and let NewtonManager validate it during solver initialization. That preserves supported subclasses and prevents the two validation policies from drifting.
| if physics_cfg.deterministic_mode == "not_guaranteed": | ||
| physics_cfg.deterministic_mode = "run_to_run" |
There was a problem hiding this comment.
When a user explicitly sets deterministic_mode="not_guaranteed" with --deterministic, this value-only check cannot distinguish the override from the default and replaces it with run_to_run, silently ignoring the documented opt-out and imposing deterministic-physics overhead.
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The physics override is correctly placed in the shared RL entrypoint path and is covered by focused tests, but the user-facing documentation overstates the scope of --deterministic, and the changelog gives opt-out guidance that the implementation does not support.
- Design and architecture: Applying the override after physics configuration resolution and before solver construction is appropriate for the RL training entrypoints and avoids optional backend imports. However, this behavior belongs to
isaaclab_rl.entrypoints.common.apply_env_overrides, notAppLauncheritself, so the reproducibility documentation must scope the physics behavior to those RL entrypoints rather than presenting it as universal app-level flag behavior. - API: The CLI surface remains unchanged, but its documented contract needs correction. Standalone
AppLauncherconsumers do not receive the physics override, and explicitly settingdeterministic_mode="not_guaranteed"does not opt out because the override replaces that value with"run_to_run"; dropping--deterministicis the implemented opt-out. - Implementation: The Newton and PhysX mutations are applied at the intended configuration seam, with stronger Newton determinism modes preserved and the MJWarp sensor prerequisite enforced. The implementation is internally consistent, but the documentation and release guidance must accurately describe its entrypoint scope and opt-out behavior.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| mode) needs it for reproducible imagery; **Newton** rendering is already deterministic. | ||
|
|
||
| Pass ``--deterministic`` to enable reproducible rendering from the app launcher. (Isaac RTX only) | ||
| **Physics determinism** comes from the same flag. It sets |
There was a problem hiding this comment.
🔵 Suggestion · Design Architecture — Docs overstate scope of physics determinism
The section introduces --deterministic as an AppLauncher flag, then claims physics determinism "comes from the same flag" and that unsupported backends raise. That holds only for code paths going through isaaclab_rl.entrypoints.common.apply_env_overrides; standalone scripts that pass the flag to AppLauncher still get no physics configuration and no error. Scope this paragraph to the RL training entrypoints.
| * Changed ``--deterministic`` to also configure the resolved physics backend: Newton backends now use | ||
| ``deterministic_mode="run_to_run"`` (MJWarp additionally with ``disable_sensors=True``, which that | ||
| mode requires), and PhysX backends ``enable_enhanced_determinism=True``. Deterministic physics costs | ||
| runtime and memory, so drop the flag or set ``deterministic_mode`` explicitly to opt out. Passing |
There was a problem hiding this comment.
🔵 Suggestion · Api — Opt-out migration guidance is inaccurate
The note tells users to "set deterministic_mode explicitly to opt out", but an explicit "not_guaranteed" is indistinguishable from the default at common.py:516 and is replaced with "run_to_run", and MJWarp disable_sensors is forced regardless of mode. Only dropping the flag opts out; reword the guidance to match the implemented behavior.
| ) | ||
| if getattr(solver_cfg, "use_mujoco_cpu", False): | ||
| raise ValueError( | ||
| "--deterministic is not supported by the MuJoCo CPU backend. Set" |
There was a problem hiding this comment.
is CPU actually not deterministic? I would have thought CPU could be deterministic by default
There was a problem hiding this comment.
Confirmed: the pure MuJoCo CPU pipeline is documented as entirely deterministic and reproducible: https://mujoco.readthedocs.io/en/stable/computation/index.html#reproducibility. use_mujoco_cpu=True therefore should not be rejected merely because --deterministic was passed. I suggest leaving NewtonCfg.deterministic_mode unchanged for that path while still applying the Torch/rendering parts of the flag. The current Newton validation only establishes that Warp’s DeterministicMode guarantee is not applicable to the CPU solver; it is not evidence that CPU MuJoCo is nondeterministic.
| if solver_name == "MJWarpSolverCfg": | ||
| # MJWarp's internal sensor kernels mix atomic reduction families, which Warp's | ||
| # deterministic code generation rejects. Isaac Lab sensors read Newton state directly. | ||
| solver_cfg.disable_sensors = True |
There was a problem hiding this comment.
would this cause any issues in tasks?
There was a problem hiding this comment.
For maintained Isaac Lab tasks this should be safe: disable_sensors disables MuJoCo Warp’s internal sensor computation, while Isaac Lab sensors read Newton state directly. It can still change behavior for custom/direct integrations that consume native MJWarp sensor outputs. Please scope this automatic mutation to the MJWarp GPU path for which the deterministic mode requires it, and document that native sensor-output limitation.
| """ | ||
| class_name = type(physics_cfg).__name__ | ||
| if class_name in ("PhysxCfg", "OvPhysxCfg"): | ||
| physics_cfg.enable_enhanced_determinism = True |
There was a problem hiding this comment.
This treats OvPhysxCfg.enable_enhanced_determinism=True as if it satisfies the same reproducibility guarantee. Our OvPhysX 0.5.10 Ant tests still produced three distinct 100-iteration reward hashes with --deterministic, enhanced determinism, PXR_WORK_THREAD_LIMIT=1, and single-threaded USDRT population. Please either exclude OvPhysX from the guaranteed path, document this as best-effort, or add an end-to-end OvPhysX determinism test before claiming the guarantee.
There was a problem hiding this comment.
ovphysx determinism issues are addressed in ovphysx 0.5.11
|
I checked this against current I support a narrower version of this PR: keep the post-preset-resolution wiring, but let each physics backend own validation, special-case pure MuJoCo CPU as already deterministic, scope the MJWarp sensor mutation/limitation precisely, and document the physics behavior as RL-entrypoint behavior rather than universal |
Review feedback on isaac-sim#7334: the RL entrypoint duplicated the solver policy that NewtonManager._validate_deterministic_solver_cfg already owns, using name equality where the backend uses isinstance. The copy rejected supported solver subclasses and gave the two policies room to drift. Drop the solver whitelist and both raises. The entrypoint now only requests the guarantee; NewtonManager rejects an unsupported solver at initialization and keeps its own error messages. Leave MuJoCo on the CPU alone: it is documented as reproducible, and Warp's deterministic mode does not reach that path, so requesting the mode there only tripped validation. Scope the MJWarp sensor mutation to the GPU path and document that it also disables the rne_postconstraint stage. Scope the docs to the RL training entrypoints and mark OvPhysX reproducibility best-effort, since it is not verified end to end on the pinned ovphysx==0.5.10.
…terministic-physics
The RL entrypoint still chose backend-specific determinism settings by class name: deterministic_mode and disable_sensors for Newton, enable_enhanced_ determinism for PhysX. That kept four backend field names in isaaclab_rl and required matching class names across the MRO because the backend types cannot be imported there. Add PhysicsCfg.deterministic, set it from --deterministic, and let each physics manager translate it. NewtonManager derives the mode, applies the MJWarp prerequisite on the GPU path, and leaves MuJoCo on the CPU alone; PhysX and OvPhysX enable enhanced determinism. Adding a backend no longer means editing isaaclab_rl. Reject a determinism request that would disable MuJoCo Warp's sensor stage while a sensor needs it. Doing so also skips rne_postconstraint, which fills body_qdd and body_parent_f; the IMU, PVA and joint-wrench sensors read that state, so Isaac-Ant, Isaac-Humanoid and Isaac-Repose-Cube-Shadow would have trained on values that are never refreshed, with no error. NewtonManager now raises at solver initialization, which is the only point where both the solver configuration and the registered sensors are visible. Validated on 4x L40: three --deterministic runs of Isaac-Cartpole-Camera are bitwise identical across all 55 checkpoint tensors, two unflagged runs differ, and Isaac-Ant --deterministic fails with the sensor message.
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
| from isaaclab_newton.physics.newton_collision_cfg import NewtonCollisionPipelineCfg | ||
|
|
||
|
|
||
| _SENSOR_STAGE_STATE_ATTRIBUTES = frozenset({"body_qdd", "body_parent_f"}) |
There was a problem hiding this comment.
is there a cleaner way to handle this? or perhaps this is more of a newton issue we should raise? this and the changes below could probably have a cleaner design?
There was a problem hiding this comment.
Yes, after diving deeper into this, I think it's a newton side bug. Filed issue and following up to see if updates can also happen in mjwarp
…rs must be off The error opened with three clauses of MuJoCo mechanism before saying the task was unsupported. Lead with that instead, name only the sensors actually implicated, and derive their names from a mapping rather than a fixed string: a joint-wrench-only task no longer reads advice about the IMU. Also record why disable_sensors is a prerequisite rather than a choice. MuJoCo Warp's tactile kernel applies atomic_max and atomic_add to one output array, and Warp's deterministic code generation cannot lower two reduction families on a single target, so the sensor module fails to compile under a guarantee -- verified as WarpCodegenError in _sensor_tactile. The docs previously described only the consequence, leaving a reader to think the prerequisite was optional.
…terministic-physics
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
|
Backported to |
… physics backend (#7334) # Description `--deterministic` configured PyTorch and the Isaac RTX renderer but never reached the physics solver, so training on Newton backends was not reproducible even with the flag passed. Two defaults moved out from under the flag between 3.0beta2 and GA: | Default | v3.0.0-beta2 | GA | |---|---|---| | `CartpolePhysicsCfg.default` | `PhysxCfg()` | `NewtonCfg` MJWarp, `deterministic_mode="not_guaranteed"` | | `MultiBackendRendererCfg.default` | `IsaacRtxRendererCfg()` | `NewtonWarpRendererCfg()` | The physics switch landed in `0caae64dc7c` (#7066), absent from all three `v3.0.0-beta*` tags. At beta2 the flag worked because PhysX is run-to-run deterministic for rigid bodies **and** Isaac RTX was the default renderer; both premises were removed without re-wiring the flag. `--deterministic` now sets `PhysicsCfg.deterministic` on the resolved physics config, in `apply_env_overrides()` — the existing CLI-to-cfg seam, after `scan()` resolves the backend and before the solver is built. That field is the backend-agnostic request; each physics manager translates it when the simulation starts: - **Newton** derives `deterministic_mode="run_to_run"`, applies the MJWarp `disable_sensors` prerequisite on the GPU path, and leaves MuJoCo-CPU alone. An explicitly set `deterministic_mode` wins. - **PhysX / OvPhysX** enable `enable_enhanced_determinism`. OvPhysX is best-effort and not verified end to end. Validation stays with the backend: `NewtonManager._validate_deterministic_solver_cfg()` rejects an unsupported solver at solver initialization, so there is one policy and one set of error messages rather than a copy in the RL layer. Adding a backend no longer means editing `isaaclab_rl`. **A determinism request that would starve a sensor is now refused.** Disabling MuJoCo Warp's sensors also skips its `rne_postconstraint` stage, which fills Newton's `body_qdd` / `body_parent_f`. The IMU, PVA and joint-wrench sensors read that state, so `Isaac-Ant`, `Isaac-Humanoid` and `Isaac-Repose-Cube-Shadow` — all defaulting to `newton_mjwarp` and feeding `joint_wrench` into their policy observations — would have trained on values that are never refreshed, with no error. `NewtonManager` raises at solver initialization, the only point where both the solver config and the registered sensors are visible. The guard restates `{"body_qdd", "body_parent_f"}`, which Newton also states internally (`solver_mujoco.py:4360` as a set, `:5179` as an equivalent `or`-chain). That duplication is tracked upstream in newton-physics/newton#4109, which asks for two things: Newton refusing the combination at the source, and exporting the field set as a public constant. Either lets this guard shrink — the constant and its sensor-tracking delete entirely once Isaac Lab pins a Newton that raises. Until then the guard is what prevents the silent case, so it stays. Fixes NVBug 6658578 (P0, Isaac Lab 3.0 GA). ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Validation Three `--deterministic` runs of `Isaac-Cartpole-Camera` (50 epochs, task defaults) on one L40, each in its own container: ``` det1 vs det2 55/55 checkpoint tensors bitwise identical det1 vs det3 55/55 checkpoint tensors bitwise identical ctrl1 vs ctrl2 41/55 differ <- same task, no flag ``` The unflagged controls diverge, so the agreement above is the flag's doing rather than a task that is trivially reproducible. `Isaac-Ant --deterministic` fails at startup with the sensor message. MuJoCo-CPU could not be exercised: `SolverMuJoCo.get_max_contact_count()` raises `NotImplementedError` on that path, so it is unreachable in Isaac Lab today. 8 unit tests added; 200 pass across `test_entrypoints_common.py` and `test_newton_manager_abstraction.py`. ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## 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 (cherry picked from commit 3748a72)
# 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
2 participants
Description
--deterministicconfigured PyTorch and the Isaac RTX renderer but never reached the physics solver, so training on Newton backends was not reproducible even with the flag passed. Two defaults moved out from under the flag between 3.0beta2 and GA:CartpolePhysicsCfg.defaultPhysxCfg()NewtonCfgMJWarp,deterministic_mode="not_guaranteed"MultiBackendRendererCfg.defaultIsaacRtxRendererCfg()NewtonWarpRendererCfg()The physics switch landed in
0caae64dc7c(#7066), absent from all threev3.0.0-beta*tags. At beta2 the flag worked because PhysX is run-to-run deterministic for rigid bodies and Isaac RTX was the default renderer; both premises were removed without re-wiring the flag.--deterministicnow setsPhysicsCfg.deterministicon the resolved physics config, inapply_env_overrides()— the existing CLI-to-cfg seam, afterscan()resolves the backend and before the solver is built. That field is the backend-agnostic request; each physics manager translates it when the simulation starts:deterministic_mode="run_to_run", applies the MJWarpdisable_sensorsprerequisite on the GPU path, and leaves MuJoCo-CPU alone. An explicitly setdeterministic_modewins.enable_enhanced_determinism. OvPhysX is best-effort and not verified end to end.Validation stays with the backend:
NewtonManager._validate_deterministic_solver_cfg()rejects an unsupported solver at solver initialization, so there is one policy and one set of error messages rather than a copy in the RL layer. Adding a backend no longer means editingisaaclab_rl.A determinism request that would starve a sensor is now refused. Disabling MuJoCo Warp's sensors also skips its
rne_postconstraintstage, which fills Newton'sbody_qdd/body_parent_f. The IMU, PVA and joint-wrench sensors read that state, soIsaac-Ant,Isaac-HumanoidandIsaac-Repose-Cube-Shadow— all defaulting tonewton_mjwarpand feedingjoint_wrenchinto their policy observations — would have trained on values that are never refreshed, with no error.NewtonManagerraises at solver initialization, the only point where both the solver config and the registered sensors are visible.The guard restates
{"body_qdd", "body_parent_f"}, which Newton also states internally (solver_mujoco.py:4360as a set,:5179as an equivalentor-chain). That duplication is tracked upstream in newton-physics/newton#4109, which asks for two things: Newton refusing the combination at the source, and exporting the field set as a public constant. Either lets this guard shrink — the constant and its sensor-tracking delete entirely once Isaac Lab pins a Newton that raises. Until then the guard is what prevents the silent case, so it stays.Fixes NVBug 6658578 (P0, Isaac Lab 3.0 GA).
Type of change
Validation
Three
--deterministicruns ofIsaac-Cartpole-Camera(50 epochs, task defaults) on one L40, each in its own container:The unflagged controls diverge, so the agreement above is the flag's doing rather than a task that is trivially reproducible.
Isaac-Ant --deterministicfails at startup with the sensor message. MuJoCo-CPU could not be exercised:SolverMuJoCo.get_max_contact_count()raisesNotImplementedErroron that path, so it is unreachable in Isaac Lab today.8 unit tests added; 200 pass across
test_entrypoints_common.pyandtest_newton_manager_abstraction.py.Release backport
developChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there