Avoid disk I/O when preparing USD stage for OVRTX renderer

[huidongc]

Description

Avoid disk I/O when preparing USD stage for OVRTX renderer

Type of change

• Bug fix (non-breaking change which fixes an issue)

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

[isaaclab-review-bot]

📋 Code Review: Avoid file-backed exporting/importing of USD stage in OVRTX renderer

Summary

This PR refactors the OVRTX renderer to avoid file-backed export/import of USD stages, instead using in-memory string serialization with stage.ExportToString() and open_usd_from_string(). This is a welcome performance improvement that eliminates unnecessary disk I/O in the rendering pipeline.

Overall Assessment: ✅ Approve with minor suggestions — The implementation is well-structured with good backward compatibility handling for OVRTX < 0.3.0.

---

🔍 Key Findings

✅ Strengths

1. Clean API Modernization (ovrtx_usd.py:106-148)

   • Renamed inject_cameras_into_usd → build_render_product_as_string with a cleaner interface
   • Function now returns just the USD string snippet rather than writing to disk
   • Removed dependency on OVRTXRendererCfg parameter, improving modularity

2. Backward Compatibility (ovrtx_renderer.py:234-249)

   • Correctly handles OVRTX < 0.3.0 which lacks open_usd_from_string()
   • Falls back to temp file materialization when needed
   • Uses tempfile.gettempdir() for fallback path

3. Config Simplification (ovrtx_renderer_cfg.py:26-29)

   • temp_usd_dir now defaults to None instead of always creating temp files
   • Removed unused temp_usd_suffix field
   • Cleaner config with optional debug output

4. Improved Logging (ovrtx_usd.py:196-201)

   • Changed verbose per-environment logging to DEBUG level
   • Added summary log at INFO level for better signal-to-noise ratio

---

📁 Files Changed

File	Changes	Assessment
ovrtx_renderer.py	+45/-22	✅ Clean refactor with proper version gating
ovrtx_renderer_cfg.py	+2/-7	✅ Simplified config, removed unused fields
ovrtx_usd.py	+23/-44	✅ Cleaner API, better separation of concerns

---

✅ Checklist Verification

• Changelog fragment — Added
• Code follows project conventions
• No apparent regressions in functionality
• Backward compatibility maintained

---

Update (commit 573ac20): ✅ Reviewed new commits pushed since last review.

Summary of Changes Since c4a315d:

The PR has been rebased on top of the latest develop branch. The OVRTX-specific changes include:

1. Memory release implemented — self._exported_usd_string is now set to None after concatenation (line 225), addressing the memory concern from my previous comment. ✅

2. Code clean-up finalized:

   • Renamed inject_cameras_into_usd → build_render_product_as_string (clearer interface)
   • Renamed export_stage_for_ovrtx → export_stage_to_string (returns string, not file path)
   • Parameter renamed use_cloning → use_ovrtx_cloning for clarity
   • Removed unused usd_file_path parameter from _update_scene_partitions_after_clone
   • Improved logging: per-environment deactivation logs changed to DEBUG level with an INFO-level summary

3. Changelog fragment added — Properly documents the Changed/Removed/Fixed sections

Status of Previous Suggestions:

Item	Status
P1: Temp filename collision for OVRTX < 0.3.0	ℹ️ Low risk - fixed filename is fine for debug use case
P2: Missing use_ovrtx_cloning docstring	ℹ️ Minor - Args section could document the new param name, but not blocking
Memory concern	✅ Fixed in this commit

Assessment:

No new issues introduced. The implementation is clean and complete. PR approved by maintainer. ✅

Recommendation: ✅ Approve

---

Update (commit 9a7fbc7): ✅ Reviewed incremental changes since 573ac20.

Summary of Changes:

The PR has been rebased onto the latest develop branch with additional improvements:

1. Multi-GPU device handling fixed — The renderer now correctly uses spec.device instead of hardcoded cuda:0. Added _device_id property for OVRTX binding.map() calls. ✅

2. Heterogeneous env detection — Added is_homogeneous() check that falls back to full-stage export when the simulation uses a heterogeneous env setup, preventing incorrect cloning behavior. ✅

3. ProxyArray compatibility — Updated set_outputs() and update_camera() to accept ProxyArray types, matching the updated BaseRenderer interface. ✅

4. Config default changed — use_ovrtx_cloning now defaults to True (was False), enabling the performance benefits by default for compatible setups.

5. Kernel module cleanup — Removed hardcoded DEVICE = "cuda:0" constant from ovrtx_renderer_kernels.py; device is now passed dynamically.

6. Changelog updated — Version bumped to 0.2.0 with comprehensive changelog entries.

Assessment:

All changes are improvements that enhance robustness (multi-GPU, heterogeneous env handling) and align with the broader codebase refactor (ProxyArray types). No new concerns introduced.

Recommendation: ✅ Approve — PR is ready to merge.

---

Update (commit 2b68928): ✅ Reviewed incremental changes since 9a7fbc7.

Summary of Changes:

Minor logging additions to ovrtx_renderer.py:

1. Success logging for in-memory loading — Added logger.info("OVRTX loaded USD from string successfully") after open_usd_from_string() call (OVRTX ≥ 0.3.0 path).

2. Success logging for file-based loading — Added logger.info("OVRTX loaded USD from file successfully (path: %s, handle: %s)") after add_usd() call (OVRTX < 0.3.0 fallback path).

Assessment:

These are purely diagnostic improvements that help with debugging USD loading issues. No functional changes, no concerns.

Recommendation: ✅ Approve — PR remains ready to merge.

[greptile-apps]

Greptile Summary

This PR eliminates disk I/O during OVRTX stage preparation by switching from stage.Export(path) → file-read → file-write to stage.ExportToString() + string concatenation + open_usd_from_string() for OVRTX ≥ 0.3.0. For older OVRTX versions a disk fallback is retained (now under tempfile.gettempdir()/ovrtx/).

• In-memory path (≥ 0.3.0): export_stage_to_string replaces export_stage_for_ovrtx; build_render_product_as_string replaces inject_cameras_into_usd; the combined stage is loaded via open_usd_from_string without touching disk unless temp_usd_dir is explicitly set for debugging.
• Legacy path (< 0.3.0): A fixed filename ovrtx_renderer_stage.usda under the system temp directory is written and passed to add_usd; concurrent runs on the same machine share this path.
• Config change: temp_usd_dir defaults to None (opt-in disk write) and temp_usd_suffix is removed.

Confidence Score: 5/5

The in-memory path for OVRTX >= 0.3.0 is clean and removes unnecessary disk I/O; the legacy fallback writes to a shared fixed path which is safe in single-process setups.

The string-based stage export and in-memory loading follow the same structural pattern as the previous file-based implementation, minimising regression risk. No new correctness issues were found in the changed code paths.

The legacy OVRTX < 0.3.0 fallback in ovrtx_renderer.py (fixed shared temp path) warrants a second look in multi-process training scenarios.

Important Files Changed

Filename	Overview
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py	Core renderer refactored to hold exported USD as a string instead of a file path; new in-memory loading path via open_usd_from_string for OVRTX >= 0.3.0, with a disk fallback for older versions
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_usd.py	Removed file-based inject_cameras_into_usd; replaced with build_render_product_as_string (returns snippet only) and export_stage_to_string (uses ExportToString() instead of Export(path))
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer_cfg.py	Changed temp_usd_dir default from a computed temp path to None; removed temp_usd_suffix field
source/isaaclab_ov/changelog.d/huidongc-avoid-ovrtx-disk-rw.rst	Changelog entry documenting the removal of temp_usd_suffix, the new None default for temp_usd_dir, and the memory-based stage loading fix

_{Reviews (2): Last reviewed commit: "Avoid disk I/O when preparing USD stage ..." | Re-trigger Greptile}