test torch_deterministic = True

[huidongc]

DO NOT MERGE THIS EXPERIMENTAL MR!

Experimening whether configure_seed(42, torch_deterministic=True) is really deterministic or not.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks 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

[greptile-apps]

Greptile Summary

This PR refactors seed handling in the rendering correctness tests by replacing env_cfg.seed + env.reset(seed=...) with an upfront configure_seed(42, torch_deterministic=True) call before environment construction, and centralises per-environment pixel-difference thresholds into a single _MAX_DIFFERENT_PIXELS_PERCENTAGE_BY_ENV_NAME dict. Golden images for all affected environments are updated to match the new (more deterministic) render outputs, and the per-env thresholds are tightened (e.g. shadow_hand 8 % → 3 %, dexsuite_kuka 10 % → 4 %).

Confidence Score: 5/5

Safe to merge; the only finding is a P2 style nit about a now-unused constant.

All logic changes are in test infrastructure only, the golden images are updated consistently, and the single finding is a minor unused-constant style issue that does not affect test correctness or CI reliability.

source/isaaclab_tasks/test/test_rendering_correctness.py — unused _ENV_SEED constant.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/test/test_rendering_correctness.py	Seed handling refactored to use configure_seed() before env creation; per-env pixel thresholds centralised into a dict; _ENV_SEED constant left unused.
source/isaaclab_tasks/test/golden_images/shadow_hand/physx-isaacsim_rtx_renderer-rgb.png	Golden image updated to reflect new deterministic render output from the seeding change.
source/isaaclab_tasks/test/golden_images/dexsuite_kuka/newton-newton_renderer-rgb.png	Golden image updated; byte-size delta is small and expected from the seeding fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test fixture invoked] --> B["configure_seed(42, torch_deterministic=True)"]
    B --> C[Create env with env_cfg]
    C --> D["env.reset()  (no seed arg)"]
    D --> E[Yield env to test function]
    E --> F["_validate_camera_outputs(env_name, ...)"]
    F --> G{"diff_pct ≤ _MAX_DIFFERENT_PIXELS_PERCENTAGE_BY_ENV_NAME[env_name]?"}
    G -- Yes --> H[Test PASSES]
    G -- No --> I[Test FAILS]

Loading

_{Reviews (1): Last reviewed commit: "test" | Re-trigger Greptile}

[greptile-apps]

_ENV_SEED constant is now dead code

_ENV_SEED = 42 (line 80) is never referenced after this PR. All three fixtures and test_registered_tasks now call configure_seed(42, ...) with the literal 42 instead of the named constant, making _ENV_SEED unreferenced. Either replace each hardcoded 42 with _ENV_SEED or remove the constant entirely.

Suggested change

	configure_seed(_ENV_SEED, torch_deterministic=True)

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR experiments with using configure_seed(42, torch_deterministic=True) to achieve deterministic rendering across test runs. The implementation replaces the previous approach of setting env_cfg.seed and passing seeds to reset(). The PR also updates 44 golden images and consolidates pixel difference thresholds into a central dictionary, reducing some tolerance values significantly.

⚠️ PR Status

DO NOT MERGE — The author explicitly states this is an experimental MR for testing torch determinism.

Architecture Impact

• Changes are self-contained to test infrastructure only
• No impact on production code paths
• Uses configure_seed() from isaaclab.utils.seed which sets torch.use_deterministic_algorithms(True) plus CUBLAS workspace config

Implementation Verdict

Minor fixes needed — The approach is reasonable for testing determinism, but there are several issues that should be addressed before this could be considered for merge.

Test Coverage

This PR is the test infrastructure. The changes modify how tests seed randomness and compare against golden images. No separate test coverage is needed.

CI Status

⚠️ 3 jobs failing:

• environments_training: Tests failed (unrelated to this PR's changes)
• isaaclab_tasks [1/3]: 1 failure in test_environments_with_stage_in_memory.py (Isaac-Cartpole-RGB-TheiaTiny-v0 — unrelated to rendering tests)
• isaaclab_tasks [2/3]: 2 failures in test_environments.py (Isaac-Cartpole-RGB-TheiaTiny-v0 — unrelated to rendering tests)

The failures appear to be related to a TheiaTiny environment, not to the rendering correctness test changes in this PR. Pre-commit checks pass ✅.

Findings

🟡 Warning: Significant tolerance reduction without explanation
The PR reduces max_different_pixels_percentage for shadow_hand from 8.0% to 3.0% and cartpole from 2.0% to 1.0%. If torch_deterministic=True truly eliminates rendering variance, these lower thresholds make sense — but this needs validation across multiple CI runs to confirm the determinism is stable. A single run passing doesn't prove determinism.

🟡 Warning: Mixing unrelated changes
The PR combines two separate changes:

1. Using configure_seed() instead of env_cfg.seed + reset(seed=)
2. Refactoring thresholds into a central dictionary

These should ideally be separate commits or PRs for clarity.

🔵 Improvement: Missing test validation methodology
To actually validate determinism, the test should be run multiple times (N≥3) on the same hardware and compare results. A single run with updated golden images doesn't prove the new approach is more deterministic than the old one — it just proves it produces different results. Consider adding a CI job that runs the rendering tests multiple times and verifies identical outputs.

🔵 Improvement: PR description lacks detail
The PR body just says "Experimenting whether configure_seed(42, torch_deterministic=True) is really deterministic or not" but doesn't:

• Document what hypothesis is being tested
• Explain why the golden images needed updating
• Note whether the tolerance reductions are intentional or a side effect
• Report results of any multi-run determinism validation

🔵 Improvement: File permission mode changes
Several PNG files changed from mode 100755 (executable) to 100644 (regular file). This is actually a good cleanup — PNG files shouldn't be executable — but it should be called out as an intentional fix.