Fix the default log file path of OVRTX renderer#5763
Conversation
# Description - The default log file path of OVRTX renderer uses `/tmp/ovrtx_renderer.log` which is Linux specific. We need to use a cross-platform temp directory instead. - Redirect OVRTX renderer log to stdout for rendering tests so that we can view what is going on with CI, e.g. time for shader compilation will be visible in the CI logs, see `rendering-correctness` test job (https://github.com/isaac-sim/IsaacLab/actions/runs/26287973633/job/77380393480?pr=5749): ```log 2026-05-22T12:41:09Z [376,470ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 30 seconds so far 2026-05-22T12:41:39Z [406,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 60 seconds so far 2026-05-22T12:42:09Z [436,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 90 seconds so far 2026-05-22T12:42:39Z [466,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 120 seconds so far 2026-05-22T12:43:09Z [496,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 150 seconds so far 2026-05-22T12:43:39Z [526,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 180 seconds so far 2026-05-22T12:44:09Z [556,471ms] [Info] [omni.rtx] Waiting for compilation of ray tracing shaders for RTX renderer main raytracing pipeline by GPU driver: 210 seconds so far 2026-05-22T12:44:16Z [563,300ms] [Info] [omni.rtx] Ray tracing shader compilation for RTX renderer main raytracing pipeline finished after 216 seconds ``` ## Type of change - Bug fix (non-breaking change which fixes an issue) ## 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
Greptile SummaryThis PR fixes two logging issues with the OVRTX renderer: replacing the hardcoded Linux-only
Confidence Score: 5/5Straightforward cross-platform fix with no behavioral regressions expected on existing Linux workflows. Both changes are narrow and well-contained: the config default is a simple string substitution using standard library calls, and the test helper is gated behind an explicit renderer check and only active in test code. No production paths are affected beyond the default log file location. No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "Fix the default log file path of OVRTX r..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review Summary
This PR fixes the cross-platform compatibility issue with the OVRTX renderer log file path and adds useful stdout redirection for CI visibility. Overall, this is a well-targeted bug fix.
✅ Positive Aspects
-
Correct cross-platform fix: Using
tempfile.gettempdir()is the idiomatic Python approach for cross-platform temporary directories. This will correctly resolve to/tmpon Linux,C:\Users\<user>\AppData\Local\Tempon Windows, and appropriate paths on macOS. -
Good CI observability: Redirecting OVRTX renderer logs to stdout during CI tests is valuable for debugging shader compilation times and renderer issues.
-
Proper documentation: The docstring update clearly explains the default behavior.
-
Changelog entries: Both the feature changelog and the skip file are properly added.
📝 Observations
Hardcoded camera names in test helper (non-blocking)
In _redirect_ovrtx_renderer_log_to_stdout, the camera names are hardcoded:
for camera_name in ("base_camera", "wrist_camera"):
camera_cfg = getattr(scene, camera_name, None)This pattern requires maintenance when new cameras are added to test environments. Consider whether a more dynamic approach (e.g., iterating over all attributes that match a naming pattern or type check) would be more maintainable. However, given that this is test infrastructure and the current approach is explicit and clear, this is not a blocking concern.
CI check status (informational)
The rendering-correctness-kitless check shows as failed. If this is a transient CI issue unrelated to this PR's changes, it may be worth re-running. If it's a genuine failure introduced by this PR, please investigate before merging.
Verdict
Looks good to merge once CI is green. The core changes are correct and well-implemented.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR fixes a cross-platform compatibility bug where the OVRTX renderer log file path was hardcoded to /tmp/ovrtx_renderer.log (Linux-specific). The fix uses tempfile.gettempdir() for cross-platform temp directory resolution. Additionally, the PR adds stdout redirection for OVRTX logs in CI tests to improve debugging visibility during shader compilation.
Design Assessment
Architecture: ✅ Appropriate
- Changes to
ovrtx_renderer_cfg.pyare correctly scoped to the renderer configuration - Test infrastructure changes in
rendering_test_utils.pyfollow established patterns - Changelog entries properly placed in respective extension directories
Implementation: ✅ Correct
tempfile.gettempdir()is the idiomatic Python approach for cross-platform temp paths- Import ordering follows stdlib-first convention
- Type hints and Google-style docstrings are complete
Findings
🔵 Suggestions
Hardcoded camera names in test helper
File: source/isaaclab_tasks/test/rendering_test_utils.py:305-307
for camera_name in ("base_camera", "wrist_camera"):
camera_cfg = getattr(scene, camera_name, None)The camera names are hardcoded, requiring maintenance when new cameras are added to test environments. A more dynamic approach (e.g., type-based discovery) could reduce maintenance burden. However, this explicit approach is clear and the current test coverage is sufficient.
Test Coverage
- Bug fix coverage: The cross-platform path fix is exercised by existing
rendering-correctnessandrendering-correctness-kitlessCI jobs that use OVRTX renderer configurations - New helper coverage:
_redirect_ovrtx_renderer_log_to_stdoutis applied consistently across all three test functions (rendering_test_shadow_hand,rendering_test_cartpole,rendering_test_dexsuite_kuka)
CI Status
| Check | Status |
|---|---|
| license-check | ✅ Pass |
| pre-commit | ✅ Pass |
| rendering-correctness | ✅ Pass |
| rendering-correctness-kitless | ❌ Fail |
| isaaclab_ov | ✅ Pass |
| Most others | ⏳ Pending |
Note on rendering-correctness-kitless failure: This is a CI infrastructure issue, not a code problem. The failure is caused by a network timeout during pip install ovrtx:
pip._vendor.urllib3.exceptions.ProtocolError: Connection broken: IncompleteRead(32768 bytes read, 76108 more expected)
ERROR: Failed to build 'ovrtx' when installing build dependencies for ovrtx
Re-running the CI job should resolve this transient network issue.
Verdict
No issues found
The implementation is correct and well-documented. The CI failure is an infrastructure issue unrelated to this PR's code changes.
c1b495b
into
isaac-sim:release/3.0.0-beta2
Description
Cherry-picked PR #5749 from
develop.Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there