Add settings to make training results deterministic#5449
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR adds determinism support for vision-based RL training by: (1) calling configure_seed() in the rl_games training script to enable PyTorch deterministic algorithms, and (2) providing a new .kit experience file with RTX renderer settings that force synchronous rendering. The changes are minimal and focused, addressing a known reproducibility issue (GitHub #3505) for camera-based tasks.
Architecture Impact
- train.py: The
configure_seed()call affects only this specific training script. Other RL library wrappers (RSL-RL, SB3, SKRL) are NOT updated — users of those frameworks will still have non-deterministic vision training. - .kit file: New experience file extends
isaaclab.python.headlesswith RTX determinism settings. No breaking changes to existing.kitfiles.
Implementation Verdict
Minor fixes needed — The core approach is sound, but there are gaps in test coverage, documentation, and consistency across training scripts.
Test Coverage
❌ No tests added. The PR description acknowledges this (checklist unchecked). For a bug fix claiming to resolve reproducibility issues, a regression test demonstrating determinism would be valuable — e.g., running 2 training iterations with the same seed and asserting identical loss/reward values. Without tests, the claim in the screenshots cannot be automatically verified.
CI Status
No CI checks available yet — cannot verify the changes don't break existing functionality.
Findings
🟡 Warning: scripts/reinforcement_learning/rl_games/train.py:207 — configure_seed hardcodes True for deterministic mode
configure_seed(env_cfg.seed, True)The second argument enables torch.use_deterministic_algorithms(True), which can cause runtime errors for operations without deterministic implementations (e.g., certain scatter/gather ops on CUDA). This should either:
- Be gated behind a CLI flag (e.g.,
--deterministic) so users can opt-in - At minimum, document which operations may fail
Looking at isaaclab.utils.seed.configure_seed, if the user hasn't explicitly requested determinism, forcing it could break existing workflows that previously worked.
🟡 Warning: Inconsistency across training scripts — only rl_games is updated
The RSL-RL (scripts/reinforcement_learning/rsl_rl/train.py), Stable-Baselines3, and SKRL training scripts are not updated. Users expecting determinism across all frameworks will be surprised. Either:
- Update all training scripts consistently
- Document that determinism is only supported for rl_games
🔵 Improvement: apps/isaaclab.python.headless.determinism.kit:168 — Missing newline at end of file
rtx.rtpt.lightcache.cached.enabled = false
File doesn't end with a newline. While minor, this violates POSIX standards and can cause issues with some tools.
🔵 Improvement: apps/isaaclab.python.headless.determinism.kit:12 — Package title doesn't reflect determinism purpose
title = "Isaac Lab Python Headless Camera"The title says "Headless Camera" but this file is specifically for deterministic rendering. Should be "Isaac Lab Python Headless Deterministic Rendering" or similar for clarity.
🔵 Improvement: apps/isaaclab.python.headless.determinism.kit — Missing documentation comments explaining the key determinism settings
The critical settings for determinism are:
- Line 79:
omni.replicator.asyncRendering = false - Line 82-84:
app.renderer.waitIdle=true,app.hydraEngine.waitIdle=true - Line 165-167:
rtx.rtpt.cached.enabled = false,rtx.rtpt.lightcache.cached.enabled = false
These should have comments explaining WHY they enable determinism, not just performance implications. Users looking at this file need to understand what makes it different from the non-deterministic version.
🔵 Improvement: PR description shows command example but no documentation update
The command example in the PR description:
./isaaclab.sh -p ... --experience isaaclab.python.headless.determinism.kit
This should be documented in the official docs (the checklist shows documentation is unchecked). Users won't discover this without documentation.
🟡 Warning: scripts/reinforcement_learning/rl_games/train.py:207 — configure_seed called after environment creation
The configure_seed call happens after gym.make() and wrapper creation (lines 161-189). If environment initialization involves any non-deterministic operations (random initialization, etc.), those won't be affected by this late seeding. The seed should be configured before environment creation, not after. Looking at the code flow:
- Line 111:
env_cfg.seed = agent_cfg["params"]["seed"]— seed is set in config - Line 161:
env = gym.make(...)— environment created - Line 207:
configure_seed(...)— deterministic mode enabled too late
This ordering means the environment creation itself may still be non-deterministic even with this fix.
Greptile SummaryThis PR adds two changes to make Isaac Lab RL training results deterministic for vision-based tasks: a new
Confidence Score: 3/5Not safe to merge as-is; hardcoded A P1 finding (unconditional
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant train.py
participant gym as gym.make (env)
participant configure_seed as configure_seed()
participant runner as rl_games Runner
User->>train.py: run with --seed 42 --experience determinism.kit
train.py->>train.py: set env_cfg.seed from agent_cfg["params"]["seed"]
train.py->>gym: gym.make(task, cfg=env_cfg) → env internally calls configure_seed(seed)
gym-->>train.py: env created (seed already applied)
train.py->>configure_seed: configure_seed(env_cfg.seed, True)
Note over configure_seed: Sets torch.use_deterministic_algorithms(True)<br/>Sets CUBLAS_WORKSPACE_CONFIG<br/>Re-seeds torch/numpy/random/warp
configure_seed-->>train.py: seed applied (deterministic mode ON)
train.py->>runner: runner.load(agent_cfg)
train.py->>runner: runner.run(train=True)
Note over runner: Any non-deterministic torch op<br/>raises RuntimeError here
Reviews (1): Last reviewed commit: "added name to the CONTRIBUTORS.md" | Re-trigger Greptile |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This follow-up addresses the previous review findings. The author has made several improvements: added --deterministic CLI flag (addressing the hardcoded True concern), renamed the kit file title to reflect determinism, added documentation to reproducibility.rst, and added explanatory comments for determinism settings in the kit file.
Previous Concerns Addressed
✅ configure_seed now uses args_cli.deterministic instead of hardcoded True
✅ Kit file title updated to "Isaac Lab Python Headless Deterministic Rendering"
✅ Documentation added explaining the --experience flag and --deterministic option
✅ Comments added explaining RTPT cache settings for determinism
Remaining/New Findings
🟡 Warning: scripts/reinforcement_learning/rl_games/train.py:215 — configure_seed still called after environment creation
The previous concern about ordering remains unaddressed. configure_seed() is called at line 215, but gym.make() happens at line 161. Any randomness during environment initialization won't be affected by the deterministic settings. The call should be moved before env = gym.make(...).
🟡 Warning: Test coverage still missing
The PR checklist still shows tests are not added. For a bug fix PR claiming to resolve reproducibility issues (#3505), a regression test demonstrating determinism would strengthen confidence in the fix.
🔵 Improvement: apps/isaaclab.python.headless.determinism.kit — File still missing trailing newline
Minor POSIX compliance issue remains.
Implementation Verdict
Minor fixes needed — The seed ordering issue is the main remaining concern that could affect correctness.
# Description the camera config was importing `isaaclab_physx.renderers` because the default render_cfg was set to that config. this PR sets that to RendererConfig to remove the import, but provides a get_default_render_config method to the backend_utils to lazily import the config if needed. this is called __post_init__ on the camera config to replace the generic config as soon as possible to avoid downstream issues referencing the renderer config. this action can be moved to the factory if downstream references are cleaned up. ## Type of change - Refactor to remove imports in cfg class ## 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 updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Co-authored-by: nvsekkin <72572910+nvsekkin@users.noreply.github.com>
…ields (isaac-sim#5275) # Description Splits IsaacLab's USD-physics cfg classes into solver-common base classes and backend-specific subclasses, and refactors the writers (`modify_*_properties`, `spawn_rigid_body_material`) so that schema application is data-driven rather than hard-coded per-class. Prepares the schema layer for multi-backend support (PhysX today, Newton/Mjc next) without polluting base classes with silently-ignored fields or stamping backend-specific schemas onto prims that didn't opt in. ## Architecture Two layered concepts: 1. **Per-declaring-class routing.** Each cfg field's USD namespace is determined by the class that declares it (walking the MRO). Base-class fields write under `physics:*`; subclass fields write under their own namespace (`physxRigidBody:*`, etc.). When a `PhysxRigidBodyPropertiesCfg` instance is written, base fields still go under `physics:*` because `_usd_namespace` is read from the declaring class via `__dict__`, not via `getattr` (which would hit the subclass override). 2. **Per-field exceptions.** Some "universal physics" fields have no USD path except through a backend-namespaced attribute today (e.g., `disable_gravity` only exists at `physxRigidBody:disableGravity`). These are declared as `_usd_field_exceptions = {applied_schema: (namespace, [fields...])}` on the base class; the writer applies the exception schema only when one of the listed fields is non-None. The single helper `_apply_namespaced_schemas(prim, cfg, cfg_dict)` in `schemas.py` does both passes for every writer (rigid body, collision, articulation root, joint drive, mesh collision, rigid-body material). ## Design constraints **One cfg class per spawner slot.** Spawners (`UsdFileCfg`, `MeshCuboidCfg`, etc.) carry a single field for each property group: `rigid_props: RigidBodyBaseCfg | None`, `collision_props: CollisionBaseCfg | None`, `joint_drive_props: JointDriveBaseCfg | None`, etc. The user cannot pass two cfgs into the same slot, so the cfg class hierarchy must be **single-rooted per spawner field** — one base class per group, with backend-specific subclasses below. This rules out a "PhysX cfg sits next to a Newton cfg as siblings" design and drives several placement decisions: | Constraint | Consequence | |---|---| | Universal-physics fields must be reachable from any backend's cfg | Goes on the **base** class, not a sibling backend cfg. Users on Newton-only deployments can use `RigidBodyBaseCfg(disable_gravity=True)` without importing `isaaclab_physx`. | | A PhysX-namespaced field whose semantics are universal (e.g., `disable_gravity`) | Lives on the base but routes to the PhysX namespace via `_usd_field_exceptions`. The base stays backend-clean; the writer dispatches the PhysX write only when the field is non-None. | | Writer logic must not branch on cfg subclass | Every writer is the same code path regardless of subclass. The cfg metadata (`_usd_namespace`, `_usd_applied_schema`, `_usd_field_exceptions`) drives behavior; the writer is a pure data interpreter. | | Adding a new backend (Newton, Mjc) | Requires a new subclass with its own `_usd_namespace` / `_usd_applied_schema`. No spawner-side changes, no writer-side changes, no base-cfg-side changes. | | A field has multiple USD paths today (one PhysX-namespaced, one Newton-namespaced) | Belongs on the **PhysX subclass**, not the base. A future `NewtonArticulationRootPropertiesCfg` will own the same conceptual field on the Newton side. ("Rule 2" — e.g., `enabled_self_collisions`.) | | A field has only one USD path today, namespaced under PhysX, but the conceptual quantity is universal | Belongs on the **base** with an `_usd_field_exceptions` entry. ("Rule 1" — e.g., `disable_gravity`, `articulation_enabled`, `contact_offset`, `rest_offset`, `max_joint_velocity`.) When Newton ships its own native attribute, the exception namespace switches transparently with no API change. | ## Field placement ### Base (solver-common) classes — `physics:*` namespace via `UsdPhysics.*API` | Cfg class | Field | USD attribute | |---|---|---| | `RigidBodyBaseCfg` | `rigid_body_enabled` | `physics:rigidBodyEnabled` | | `RigidBodyBaseCfg` | `kinematic_enabled` | `physics:kinematicEnabled` | | `CollisionBaseCfg` | `collision_enabled` | `physics:collisionEnabled` | | `MassPropertiesCfg` | `mass` | `physics:mass` | | `MassPropertiesCfg` | `density` | `physics:density` | | `RigidBodyMaterialBaseCfg` | `static_friction` | `physics:staticFriction` | | `RigidBodyMaterialBaseCfg` | `dynamic_friction` | `physics:dynamicFriction` | | `RigidBodyMaterialBaseCfg` | `restitution` | `physics:restitution` | | `JointDriveBaseCfg` | `drive_type` | `drive:<axis>:physics:type` | | `JointDriveBaseCfg` | `max_force` | `drive:<axis>:physics:maxForce` | | `JointDriveBaseCfg` | `stiffness` | `drive:<axis>:physics:stiffness` | | `JointDriveBaseCfg` | `damping` | `drive:<axis>:physics:damping` | | `MeshCollisionBaseCfg` | `mesh_approximation_name` | `physics:approximation` (token) | | `ArticulationRootBaseCfg` | `fix_root_link` | (synthesizes `UsdPhysics.FixedJoint`) | `JointDriveBaseCfg` and `MeshCollisionBaseCfg` use the typed `UsdPhysics.DriveAPI` / `UsdPhysics.MeshCollisionAPI` accessors at the writer level (multi-instance namespace and `TfToken` with `allowedTokens`, respectively); all other base fields flow through the helper's per-class routing. ### PhysX subclasses — `physx*:*` namespaces, `Physx*API` schemas | Cfg class | `_usd_namespace` | `_usd_applied_schema` | Adds fields | |---|---|---|---| | `PhysxRigidBodyPropertiesCfg` | `physxRigidBody` | `PhysxRigidBodyAPI` | `linear_damping`, `angular_damping`, `max_linear_velocity`, `max_angular_velocity`, `max_depenetration_velocity`, `max_contact_impulse`, `enable_gyroscopic_forces`, `retain_accelerations`, solver iter counts, sleep / stabilization thresholds | | `PhysxCollisionPropertiesCfg` | `physxCollision` | `PhysxCollisionAPI` | `torsional_patch_radius`, `min_torsional_patch_radius` | | `PhysxArticulationRootPropertiesCfg` | `physxArticulation` | `PhysxArticulationAPI` | `enabled_self_collisions`, solver iter counts, sleep / stabilization thresholds | | `PhysxJointDrivePropertiesCfg` | `physxJoint` | `PhysxJointAPI` | (currently empty; reserved for future PhysX-only knobs) | | `PhysxRigidBodyMaterialCfg` | `physxMaterial` | `PhysxMaterialAPI` | `compliant_contact_stiffness`, `compliant_contact_damping`, `friction_combine_mode`, `restitution_combine_mode` | | `PhysxConvexHullPropertiesCfg` | `physxConvexHullCollision` | `PhysxConvexHullCollisionAPI` | `hull_vertex_limit`, `min_thickness` | | `PhysxConvexDecompositionPropertiesCfg` | `physxConvexDecompositionCollision` | `PhysxConvexDecompositionCollisionAPI` | hull / voxel / shrink-wrap tunables | | `PhysxTriangleMeshPropertiesCfg` | `physxTriangleMeshCollision` | `PhysxTriangleMeshCollisionAPI` | `weld_tolerance` | | `PhysxTriangleMeshSimplificationPropertiesCfg` | `physxTriangleMeshSimplificationCollision` | `PhysxTriangleMeshSimplificationCollisionAPI` | `simplification_metric`, `weld_tolerance` | | `PhysxSDFMeshPropertiesCfg` | `physxSDFMeshCollision` | `PhysxSDFMeshCollisionAPI` | `sdf_margin`, `sdf_narrow_band_thickness`, `sdf_resolution`, etc. | ### `_usd_field_exceptions` table These fields are declared on a *base* class but the only USD path today goes through a non-base namespace. Each entry says: "if any listed field on this cfg is non-None, apply the exception schema and write that one attribute under the exception namespace." All other fields on the cfg follow the per-declaring-class routing rule. | Base cfg class | Exception schema | Namespace | Field(s) | Why on the base | |---|---|---|---|---| | `RigidBodyBaseCfg` | `PhysxRigidBodyAPI` | `physxRigidBody` | `disable_gravity` | Per-body gravity exclusion is universal physics; PhysX honors per-body, Newton consumes the same attribute via the bridge resolver (scene-level today; per-body fix is a Newton-side kernel change, not a cfg-API change) | | `CollisionBaseCfg` | `PhysxCollisionAPI` | `physxCollision` | `contact_offset`, `rest_offset` | Collision-pair generation distance and rest gap are universal physics; Newton importer consumes both via PhysX bridge to populate `Model.shape_collision_radius` / `_thickness` (`import_usd.py:2104, 2111`) | | `ArticulationRootBaseCfg` | `PhysxArticulationAPI` | `physxArticulation` | `articulation_enabled` | PhysX honors at sim time; IsaacLab Newton wrapper reads it as a spawn-time guard at `rigid_object.py:1035`. Universal user-facing intent | | `JointDriveBaseCfg` | `PhysxJointAPI` | `physxJoint` | `max_joint_velocity` | Sole USD path to `Model.joint_velocity_limit` in Newton (no `newton:*` equivalent today). The exception namespace switches transparently when Newton ships `newton:maxJointVelocity` as a registered applied API | When any exception field is non-None, the corresponding `Physx*API` schema is applied to the prim. When all exception fields are None, no PhysX schema is stamped — Newton-targeted prims authored from `*BaseCfg` stay free of PhysX schemas they didn't opt in to. ## Field renames (with deprecation aliases) To enforce the convention that python `snake_case` cfg field names map identity-style to USD `camelCase` attribute names, two legacy fields were renamed. Both keep the old name as a deprecation alias forwarded via `__post_init__` (emits `DeprecationWarning`, scheduled for removal in 5.0). | Old name | New name | USD attribute | |---|---|---| | `JointDriveBaseCfg.max_velocity` | `max_joint_velocity` | `physxJoint:maxJointVelocity` | | `JointDriveBaseCfg.max_effort` | `max_force` | `drive:<axis>:physics:maxForce` | ## Type of change - New feature (non-breaking change which adds functionality) - Breaking change (existing functionality will not work without user modification) The split is non-breaking at the spawner-cfg level — every base-class type accepts any subclass via polymorphism, and every legacy `RigidBodyPropertiesCfg` / `JointDrivePropertiesCfg` / `CollisionPropertiesCfg` / `ArticulationRootPropertiesCfg` / `MeshCollisionPropertiesCfg` / `RigidBodyMaterialCfg` / `FixedTendonPropertiesCfg` / `SpatialTendonPropertiesCfg` import path continues to work via deprecation-alias subclasses and `__getattr__` shims on `isaaclab.sim`, `isaaclab.sim.schemas`, and `isaaclab.sim.schemas.schemas_cfg`. Direct attribute access to the renamed fields still works through deprecation aliases. Removal scheduled for 5.0. The breaking aspect: cfg classes in `isaaclab_physx.sim.schemas` and `isaaclab_physx.sim.spawners.materials` are physically relocated. Anyone importing from internal paths (rather than `isaaclab.sim`) needs to update. ## Migration ```python # Before import isaaclab.sim as sim_utils rigid_props = sim_utils.RigidBodyPropertiesCfg(disable_gravity=True, linear_damping=0.1) joint_props = sim_utils.JointDrivePropertiesCfg(max_effort=80.0, max_velocity=5.0) collision_props = sim_utils.CollisionPropertiesCfg(contact_offset=0.02, torsional_patch_radius=1.0) material = sim_utils.RigidBodyMaterialCfg(static_friction=0.7, compliant_contact_stiffness=1000.0) # After (PhysX-targeted) import isaaclab.sim as sim_utils from isaaclab_physx.sim.schemas import ( PhysxRigidBodyPropertiesCfg, PhysxJointDrivePropertiesCfg, PhysxCollisionPropertiesCfg, ) from isaaclab_physx.sim.spawners.materials import PhysxRigidBodyMaterialCfg rigid_props = PhysxRigidBodyPropertiesCfg(disable_gravity=True, linear_damping=0.1) joint_props = PhysxJointDrivePropertiesCfg(max_force=80.0, max_joint_velocity=5.0) collision_props = PhysxCollisionPropertiesCfg(contact_offset=0.02, torsional_patch_radius=1.0) material = PhysxRigidBodyMaterialCfg(static_friction=0.7, compliant_contact_stiffness=1000.0) # After (Newton-targeted — base classes only, no PhysX schemas applied) from isaaclab.sim.schemas import RigidBodyBaseCfg, JointDriveBaseCfg, CollisionBaseCfg from isaaclab.sim.spawners.materials import RigidBodyMaterialBaseCfg rigid_props = RigidBodyBaseCfg(disable_gravity=True) # only base + exception fields available joint_props = JointDriveBaseCfg(max_force=80.0, max_joint_velocity=5.0) material = RigidBodyMaterialBaseCfg(static_friction=0.7) ``` Spawner type annotations remain unchanged — they accept any subclass via polymorphism. ## Internal helper ```python def _apply_namespaced_schemas(prim, cfg, cfg_dict): # 1. Per-field exceptions: pop listed fields, apply exception schema if any non-None, # write under exception namespace. # 2. Per-declaring-class routing: walk MRO to find each remaining field's owner class; # write under that class's _usd_namespace; apply that class's _usd_applied_schema. ``` Used by all five `modify_*_properties` writers and `spawn_rigid_body_material`. Replaced ~125 lines of duplicated gating logic with a single ~30-line helper. ## Side change: configclass `source/isaaclab/isaaclab/utils/configclass.py:_process_mutable_types` now detects string-form `ClassVar` annotations under PEP 563 (`from __future__ import annotations`) so it doesn't wrap `ClassVar[dict]` defaults in `field(default_factory=...)`. Matches Python stdlib `dataclasses` semantics. No pre-existing IsaacLab class used `ClassVar` inside a `@configclass` block, so the change has no effect on existing code; it enables the `ClassVar` metadata pattern this PR introduces. ## Test plan - [x] `test_schemas.py` (38 → 40 tests): all schema-cfg classes write correct attributes under the right namespace; PhysX schemas are NOT applied when only base/UsdPhysics fields are set; deprecation aliases (`max_velocity` → `max_joint_velocity`, `max_effort` → `max_force`) forward correctly and emit `DeprecationWarning`. **40 passed.** - [x] `test_schemas_shim.py`: legacy import paths (`isaaclab.sim.schemas.RigidBodyPropertiesCfg` etc.) resolve via `__getattr__` shims. **All passing.** - [x] `test_articulation.py`, `test_rigid_object_iface.py`, `test_valid_configs.py`, `test_spawn_*` — no regressions. - [x] Full suite (`./isaaclab.sh -t`): 8768/9205 pass, 437 unrelated baseline failures (rendering, `omni.physics.tensors.api` missing, OSC controller, `install_ci`, `pyglet`, Newton env-path, Anymal-C determinism). Zero new regressions; +123 passing tests vs. earlier state. - [x] `./isaaclab.sh -f` (pre-commit) clean. ## Supersedes Together with isaac-sim#5276, supersedes isaac-sim#4847 and isaac-sim#5203 with a cleaner schema-layer design. ## 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 (changelog fragments under `source/isaaclab/changelog.d/`) - [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 updated the changelog (fragment-based system) and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Co-authored-by: ooctipus <zhengyuz@nvidia.com>
…c-sim#5301) Updates docs for using nurec background in locomanipulation sdg ## Type of change - Documentation update ## Checklist - [ ] 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` - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com>
There was a problem hiding this comment.
Follow-up Review
Thanks for addressing the previous feedback! The changes look good:
- ✅ Added explanatory comment for
configure_seedordering afterRunner() - ✅ Added determinism-critical section header in
.kitfile - ✅ Added warning when
--deterministicis used without compatible flags - ✅ Increased
max_iterationsto 50 for more robust test assertions
Minor Polish (non-blocking)
Warning message formatting (app_launcher.py:1036-1038):
"--deterministic has no effect when not in headless mode or "
"when cameras are disabled or when livestreaming or XR is enabled."
"Use --enable_cameras --headless --deterministic for deterministic rendering."Two small issues:
- Double space:
"or when"(extra space) - Missing space before "Use" (strings concatenate directly, so
"enabled."+"Use"="enabled.Use")
Suggested fix:
"--deterministic has no effect when not in headless mode, "
"when cameras are disabled, or when livestreaming/XR is enabled. "
"Use --enable_cameras --headless --deterministic for deterministic rendering."Overall the PR is well-structured and addresses a real reproducibility pain point. The approach of separating rendering determinism (kit experience file) from PyTorch determinism (configure_seed) is clean, and the test coverage is thoughtful.
Status: Approved pending minor string fix (or can merge as-is if team prefers).
Update (d2631d8): Excellent extension! This commit expands deterministic seed support to all RL frameworks:
Training scripts now support --deterministic:
- rsl_rl/train.py
- skrl/train.py
- sb3/train.py
- (rl_games already had it, now refactored to conditional check)
Play scripts also support --deterministic:
- rl_games/play.py
- rsl_rl/play.py
- skrl/play.py
- sb3/play.py
Test coverage expanded:
- New
test_play_scripts_call_configure_seed_after_runner_or_agent_construction()for all play scripts - Extended train script test to cover all RL frameworks (was only rl_games)
The pattern is consistent: configure_seed is always called after runner/agent construction to avoid interfering with internal initialization. The conditional if args_cli.deterministic check is cleaner than passing the flag through.
Previous approval stands. This is now a comprehensive determinism solution across the entire RL stack. 🎉
Update (8cc5a12): Rebase onto develop branch. The deterministic feature code is unchanged — the only modification touching RL scripts is a version bump in rsl_rl/train.py (RSL_RL_VERSION = "3.0.1" → "5.0.1"), which is unrelated to determinism.
✅ All previous approvals still valid. Ready to merge.
Update (aedb643): Rebase onto latest develop. The determinism feature code remains unchanged — this commit incorporates upstream changes (LEAPP export refactor, camera math fixes, debugging improvements) that don't affect the PR's core functionality.
✅ All previous approvals remain valid. Ready to merge.
kellyguo11
left a comment
There was a problem hiding this comment.
My feeling is we don't need to tie the deterministic flag towards a specific RL library, especially since rl-games is less maintained now and we are shifting away from it. Could we do the same setup that enables determinism for RL Games in the train and play scripts for other RL libraries as well?
|
@StafaH for viz, this may impact your training script refactoring |
🔄 Follow-up on new commits (656255d)Reviewed incremental changes since last review (159eb2b):
No new issues found. These are CI/infrastructure improvements that complement the determinism features. Previous approval stands — ready to merge (optional minor string fix noted in prior review). |
# Description <!-- Thank you for your interest in sending a pull request. Please make sure to check the contribution guidelines. Link: https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html 💡 Please try to keep PRs small and focused. Large PRs are harder to review and merge. --> This PR adds a deterministic training path and documentation for Isaac Lab RL workflows. - Added apps/isaaclab.python.headless.determinism.kit as a deterministic headless rendering experience. - Updated scripts/reinforcement_learning/rl_games/train.py to add opt-in --deterministic and use configure_seed(env_cfg.seed, args_cli.deterministic). - Updated docs/source/features/reproducibility.rst to document --experience isaaclab.python.headless.determinism.kit and clarify that strict PyTorch determinism is currently exposed only for RL-Games. Test command example: ./isaaclab.sh -p scripts/reinforcement_learning/rl_games/train.py --task Isaac-Cartpole-RGB-v0 --enable_cameras --headless --seed 42 --max_iteration 20 **--deterministic --experience isaaclab.python.headless.determinism.kit** Fixes # (issue) <!-- As a practice, it is recommended to open an issue to have discussions on the proposed pull request. This makes it easier for the community to keep track of what is being developed or added, and if a given feature is demanded by more than one party. --> isaac-sim#3505 Non-reproducible training results in vision-based tasks with identical seeds ## Type of change <!-- As you go through the list, delete the ones that are not applicable. --> - Bug fix (non-breaking change which fixes an issue) ## Screenshots | Before | After | | ------ | ----- | | <img width="200" height="250" alt="Before" src="https://github.com/user-attachments/assets/57b52d82-ed32-4f79-8ac2-db19b32df54a" /> | <img width="200" height="250" alt="After" src="https://github.com/user-attachments/assets/5da0b220-7fef-445a-8efb-f0e1c6dab6a3" /> | <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## 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 - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: r-schmitt <139814266+r-schmitt@users.noreply.github.com> Co-authored-by: nvsekkin <72572910+nvsekkin@users.noreply.github.com> Co-authored-by: vidurv-nvidia <vidurv@nvidia.com> Co-authored-by: ooctipus <zhengyuz@nvidia.com> Co-authored-by: Yuchen Deng <yuchenkit@gmail.com> Co-authored-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: isaaclab-bot[bot] <282401363+isaaclab-bot[bot]@users.noreply.github.com> Co-authored-by: hujc <jichuanh@nvidia.com> Co-authored-by: Antoine RICHARD <antoiner@nvidia.com>
Description
This PR adds a deterministic training path and documentation for Isaac Lab RL workflows.
and clarify that strict PyTorch determinism is currently exposed only for RL-Games.
Test command example:
./isaaclab.sh -p scripts/reinforcement_learning/rl_games/train.py --task Isaac-Cartpole-RGB-v0 --enable_cameras --headless --seed 42 --max_iteration 20 --deterministic --experience isaaclab.python.headless.determinism.kit
Fixes # (issue)
#3505 Non-reproducible training results in vision-based tasks with identical seeds
Type of change
Screenshots
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there