[Backport] PR #6905 to release/3.0.0 - #7557
Conversation
Each CI run starts with an empty `nv_shadercache` directory, so the
NVIDIA driver recompiles every pipeline state object (PSO) before the
first render — roughly 600 s, which is what `COLD_CACHE_BUFFER = 700` in
`tools/conftest.py` exists to absorb. The cost is paid on every
rendering job, twice per run (kit-based and kitless).
This persists the driver's PSO blobs across runs with GitHub Actions
cache, so warm runs skip the compile entirely.
Two independent cache trees live under
`${RUNNER_TEMP}/isaaclab-ovrtx-shader-cache/`, because the two render
paths are compiled by different runtimes and a bump to one must not
invalidate the other:
| Tree | Produced by | Reaches the container as |
|---|---|---|
| `kit/` | RTX renderer inside the Isaac Sim image | nested bind mount
over `/isaac-sim/kit/cache/nv_shadercache`, which the enclosing
`kit/cache` tmpdir mount would otherwise present as empty |
| `kitless/` | pip-installed `ovrtx` wheel | `OVRTX_SHADER_CACHE_PATH`,
applied by `OVRTXRenderer` as a carb setting |
One action owns the whole lifecycle — key computation, restore,
reporting and writeback — so callers only decide *when* each phase runs.
A composite action cannot span the caller's test step, so the phases are
separate invocations selected by `mode`:
- `restore` — reads the newest compatible snapshot of each tree, never
writes
- `report` — summarises how far the run compiled beyond what was
restored
- `save` — reports, then writes each **populated** tree back as a new
snapshot; only the warmer uses it
Every mode recomputes keys from the same `key.sh`, so the collection a
job reads and the one it writes cannot drift.
Cache key composition:
| Component | Source | Why |
|---|---|---|
| `RUNNER_OS`, `RUNNER_ARCH` | GHA env | platform |
| `sm<cc>` | `nvidia-smi --query-gpu=compute_cap` | PSO blobs are
GPU-architecture specific |
| `drv<version>` | `nvidia-smi --query-gpu=driver_version` | PSO blobs
are driver specific |
| `isaacsim<tag>` (kit only) | `isaacsim-version` input | identifies the
Kit RTX build that compiled `kit/` |
| `ovrtx<version>` (kitless only) | `uv.lock` pin | keeps the key a
function of the commit, so an upstream wheel release cannot re-key open
PRs onto a cold collection |
`verify.sh` fails the warmer when either tree came out empty, which
would otherwise publish a silent half-warm snapshot that every consumer
prefers over the last good one.
- `run-package-tests`: new `ovrtx-shader-cache` input (`''` / `restore`
/ `save`); restores before the tests, reports or publishes after.
Publish is gated on `!cancelled()` rather than `success()` — compiled
PSO blobs stay valid when a golden-image assertion fails.
- `run-tests` / `run_tests.sh`: new `ovrtx-shader-cache-host-dir` input;
owns the canonical mount layout and runs the pre-test mount check.
- `build.yaml`: `ovrtx-shader-cache: restore` on
`rendering-correctness`, `rendering-correctness-kitless-legacy` and
`rendering-correctness-kitless-ovstage`; new post-merge
`warm-ovrtx-cache` job (`continue-on-error`, publishes only on `main` /
`develop` / `release/*`) that renders one Kit and one kitless cartpole
case to fill both trees.
- `source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_shader_cache.py`
(new): reads `OVRTX_SHADER_CACHE_PATH` and points both
`/rtx/shaderDb/driverShaderCachePath` and
`/rtx/shaderDb/driverAppShaderCachePath` at it via the ovrtx settings
extension. No-ops when the variable is unset; warns when the runtime has
no settings extension; raises when a setting is rejected, since a
partial redirect still compiles cold. Kept out of `ovrtx_renderer` so
the policy imports without the ovrtx runtime.
- `ovrtx_renderer.py`: calls `redirect_shader_cache(OVRTX_CONFIG)`
before constructing `Renderer()` — the settings are read at
renderer-creation time and cannot be changed on a live instance.
- `tools/verify_ovrtx_shader_cache.py` (new): in-container check that
the mounted trees are present and writable, run before tests start. A
shadowed `kit/` mount is otherwise invisible — the restore step still
reports a hit and the only symptom is a slower job.
- `tools/conftest.py`: `COLD_CACHE_BUFFER` docstring condensed to its
functional contract; the value is unchanged.
`source/isaaclab_ov/test/test_ovrtx_shader_cache_redirect.py` covers the
redirect boundary with a fake settings applier — no GPU, no ovrtx
runtime, no renderer: both settings redirected, rejection raises and is
not logged as success, unset env var queries nothing, the renderer
config reaches the applier (it is what initializes the library), and a
runtime without the extension degrades to a warning.
- New feature (non-breaking change which adds functionality)
- [x] <!-- backport-active-release --> Backport this pull request to the
active release branch after it merges into `develop`
- [ ] `warm-ovrtx-cache` runs post-merge and publishes both entries —
step summary shows a non-zero file count for `kit/` and `kitless/`
- [ ] Subsequent `rendering-correctness` and
`rendering-correctness-kitless-*` runs restore the cache and report
`fully covered`
- [ ] The `COLD_CACHE_BUFFER` allowance is no longer consumed on warm
runs — the first camera test completes within its normal budget
- [ ] Cold run (ovrtx bump or driver bump) still succeeds: cache miss →
compile → warmer publishes into the new collection
- [x] I have read and understood the contribution guidelines
- [x] I have run the `pre-commit` checks with `./isaaclab.sh --format`
- [x] My changes generate no new warnings
- [x] I have added a changelog fragment under
`source/isaaclab_ov/changelog.d/`
- [x] My name already exists in `CONTRIBUTORS.md`
(cherry picked from commit 9847b71)
![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 shader-cache plumbing is coherent, but the conflict resolution rewrites most of ovrtx_renderer.py and introduces concrete renderer regressions: simple-shading outputs are no longer populated, the ovstage backend is largely stubbed out, ovstage camera updates fail, and the configured combined-stage diagnostic export is removed.
- Design and architecture: The cache action and dedicated shader-cache redirection module are cleanly separated. However, the renderer rewrite is far broader than the cache redirect requires, violates the smallest-change and behavior-preservation criteria, and replaces the functional ovstage implementation with no-op methods while dropping clone transforms.
- API: The new shader-cache environment contract is documented and tested, but existing renderer contracts regress. Three advertised simple-shading output types are left unwritten,
temp_usd_dirno longer emits the combined renderer stage, and public method annotations and docstrings were broadly removed. - Implementation: The CI restore, mount, report, verification, and warmer save paths are connected end to end. In the renderer, the ovstage binding and update methods are now
pass; clone root positions are no longer written; and_update_camera_ovstagecalls the legacy implementation, which accesses an attribute not initialized in ovstage mode and therefore raisesAttributeError. The deleted renderer behavior should be restored while limiting the backport to the shader-cache integration.
Needs rework. Posted 5 actionable findings inline.
Automated review; human maintainers own approval decisions.
| def _process_render_frame(self, data, frame, outputs): | ||
| data.renderer_info.clear() | ||
| for key, output in ( | ||
| (_LDR_COLOR_VAR, "rgba"), |
There was a problem hiding this comment.
🔴 Critical · Api — Simple-shading outputs never populated
_process_render_frame now routes LdrColor only to rgba; the previous selection that fell back to an _RTX_MINIMAL_MODES key was deleted. supported_output_types still advertises SIMPLE_SHADING_CONSTANT_DIFFUSE, SIMPLE_SHADING_DIFFUSE_MDL and SIMPLE_SHADING_FULL_MDL, so a camera requesting only those data types receives an untouched zero buffer. Restore the minimal-mode buffer-key selection.
| if self._object_xform_query is None or self._object_newton_indices is None or self._object_scales is None: | ||
| return | ||
| from isaaclab_newton.physics import NewtonManager | ||
| def _setup_xform_bindings_ovstage(self): |
There was a problem hiding this comment.
🔴 Critical · Design Architecture — ovstage backend reduced to no-op stubs
_setup_xform_bindings_ovstage, _setup_deformable_bindings_ovstage, _setup_particle_bindings_ovstage, _setup_cable_bindings_ovstage, _update_transforms_ovstage, _update_geometries_ovstage and _update_scene_partitions_after_clone_ovstage now only pass, and _clone_sources_ovstage no longer writes env-root positions. With ISAAC_LAB_OVRTX_USE_OVSTAGE=1 nothing is bound or updated, so clones stack at the origin and geometry stays frozen. Restore the deleted implementations.
| pass | ||
|
|
||
| def _update_camera_ovstage(self, data, positions, orientations, intrinsics): | ||
| self._update_camera_legacy(data, positions, orientations, intrinsics) |
There was a problem hiding this comment.
🔴 Critical · Implementation — ovstage camera update raises AttributeError
_update_camera_ovstage delegates to _update_camera_legacy, which reads self._camera_xform_binding (line 549). _init_fields_ovstage defines _camera_xform_query and never that attribute, so the first camera update in ovstage mode raises AttributeError and the ovstage camera query is never written. Restore the ovstage write_attribute path for the camera transform.
|
|
||
| - **ovrtx_usd.py**: USD helpers for OVRTX: render var config, camera injection, etc. | ||
| """ | ||
| """OVRTX Renderer implementation.""" |
There was a problem hiding this comment.
🟡 Warning · Design Architecture — Backport rewrites renderer beyond shader-cache scope
The shader-cache change needs only the redirect_shader_cache import and call, yet this file loses ~1130 lines: module/class/method docstrings, public parameter and return annotations, and validation and cleanup code. That conflicts with the repository rules on smallest change, preserved behavior, Google-style docstrings for public APIs, and concrete public types. Reduce the diff to the redirect import and call.
| ) | ||
| self._render_product_paths.append(render_product_path) | ||
| combined_usd_string = self._exported_usd_string + "\n\n" + render_product_string | ||
| combined = self._exported_usd_string + "\n\n" + product |
There was a problem hiding this comment.
🟡 Warning · Api — temp_usd_dir no longer writes combined stage
Both _initialize_from_spec_legacy and _initialize_from_spec_ovstage build combined but dropped the _write_file(..., "ovrtx_renderer_stage.usda", ...) call guarded by cfg.temp_usd_dir. Only the pre-renderer stage is still written, so the configured diagnostic artifact silently disappears while the config field remains. Restore the write before opening the combined stage.
Greptile SummaryThis backport adds an OVRTX shader-cache redirect and CI cache lifecycle, but its conflict resolution also rewrites the renderer and removes the ovstage backend’s synchronization logic.
Confidence Score: 4/5This PR is not safe to merge until the ovstage renderer’s removed setup and synchronization implementations are restored. Enabled ovstage rendering now invokes no-op scene, binding, transform, and geometry methods, while camera updates target a legacy binding that the ovstage path never initializes, so affected rendering jobs can produce stale or incorrect frames. Files Needing Attention: source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
W[Rendering workflow] --> R[Restore compatible shader cache]
R --> M[Bind-mount Kit and kitless trees]
M --> T[Run rendering tests]
T --> G[Measure cache growth]
G --> S[Publish changed snapshots]
T --> O[OVRTX ovstage renderer]
O --> N[No-op scene and transform updates]
N --> F[Stale or incorrect frames]
Reviews (1): Last reviewed commit: "Add OVRTX shader cache for CI rendering ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def _update_scene_partitions_after_clone_ovstage(self, count): | ||
| pass | ||
|
|
||
| def _setup_particle_bindings_ovstage(self) -> None: | ||
| try: | ||
| from isaaclab_newton.physics import NewtonManager | ||
| except ImportError: | ||
| return | ||
| records = NewtonManager._particle_visual_prims | ||
| if not records: | ||
| return | ||
| paths = list(records) | ||
| self._particle_visual_offsets = [record.offset for record in records.values()] | ||
| self._particle_visual_counts = [record.count for record in records.values()] | ||
| self._particle_paths_list = self._stage_paths.create_path_list_from_strings(paths) | ||
| self._particle_points_query = self._stage.query_from_path_list(self._particle_paths_list) | ||
| self._write_identity_xforms(self._particle_points_query, len(paths)) | ||
|
|
||
| def _setup_cable_bindings_ovstage(self) -> None: | ||
| discovered = self._discover_cable_segment_bindings() | ||
| if discovered is None: | ||
| return | ||
| paths, ids, offsets, counts = discovered | ||
| self._cable_paths_list = self._stage_paths.create_path_list_from_strings(paths) | ||
| self._cable_points_query = self._stage.query_from_path_list(self._cable_paths_list) | ||
| self._write_identity_xforms(self._cable_points_query, len(paths)) | ||
| self._allocate_cable_device_buffers(ids, offsets, counts) | ||
| self._cable_point_slices = [ | ||
| self._cable_points[offset + curve : offset + curve + count + 1] | ||
| for curve, (offset, count) in enumerate(zip(offsets, counts, strict=True)) | ||
| ] | ||
| self._cable_point_tensors = [points_tensor_from_warp(points) for points in self._cable_point_slices] | ||
|
|
||
| def _write_identity_xforms(self, query, count: int) -> None: | ||
| self._stage.write_attribute( | ||
| query, | ||
| "omni:resetXformStack", | ||
| ordinal=self._current_ordinal, | ||
| tensors=np.full(count, True, dtype=np.bool_), | ||
| is_array=False, | ||
| ).wait() | ||
| self._stage.write_attribute( | ||
| query, | ||
| "omni:xform", | ||
| ordinal=self._current_ordinal, | ||
| tensors=xform_tensor_from_numpy(np.tile(np.eye(4, dtype=np.float64), (count, 1, 1))), | ||
| is_array=False, | ||
| semantic=ovstage.AttributeSemantic.MATRIX, | ||
| ).wait() | ||
|
|
||
| def _update_transforms_ovstage(self) -> None: | ||
| if self._object_xform_query is None or self._object_newton_indices is None or self._object_scales is None: | ||
| return | ||
| from isaaclab_newton.physics import NewtonManager | ||
| def _setup_xform_bindings_ovstage(self): | ||
| pass |
There was a problem hiding this comment.
When ISAAC_LAB_OVRTX_USE_OVSTAGE=1, initialization and frame updates invoke methods that now do nothing, while camera updates use a legacy binding that the ovstage path never creates. Scene partitions, cameras, object transforms, and dynamic geometry therefore remain stale, causing incorrect frames in the ovstage rendering jobs.
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
2 participants
Backports #6905 to
release/3.0.0.The original automatic cherry-pick conflicted in
ovrtx_renderer.py. Its inferred resolution incorrectly replaced the release branch's newer renderer with the older develop-side file, producing a+444/-1130renderer diff and removing ovstage behavior.Follow-up commit
64ae8edb1corrects that resolution:ovrtx_renderer.pyretains the complete release implementation.redirect_shader_cacheimport and the call immediately beforeRenderer(config).9847b71e3324cb46d4f5882d226ac666bbc22f4d2af02510cc7cac6171fa25bf912f1c4dc83c327988ef9ed54831eb33ea9a93fdc0c860089488f71e64ae8edb1Validation
uv run --no-project --with pytest --with lazy-loader python -m pytest source/isaaclab_ov/test/test_ovrtx_shader_cache_redirect.py -q- 6 passed.develop; the existing Add OVRTX shader cache for CI rendering tests #6905isaaclab_ovchangelog fragment remains included.uv run isaaclab -fcommand cannot resolve the release branch's Linux/Windows-only lockfile on macOS; equivalent file-scoped hooks were run directly.