Add Sensor PPISP controller support for camera post-processing

[moennen]

Description

Adds PPISP PPISPAuto controller support to IsaacLab camera post-processing.

This change parses embedded PPISP controller weights from USD/SPG-authored controller CUDA sources, stores them in PpispCfg, and evaluates the controller in Warp before applying PPISP.
The controller predicts per-camera exposure and color-latent parameters, while existing static PPISP config continues to provide responsivity, vignetting, and CRF parameters.

The implementation keeps the original pure-Warp controller path as a correctness reference and adds a native wp.func_native CUDA-snippet backend for high-env-count tiled camera workloads. The default controller backend uses an automatic heuristic: Warp for single/large images, native CUDA snippets for many small camera tiles.

Also adds tests for:

• USD PPISPAuto controller parsing
• controller weight layout and NumPy equivalence
• native-vs-Warp controller equivalence
• PPISP camera sensor coverage across Isaac RTX, OVRTX, and Newton paths
• opt-in PPISP pipeline performance sweep

Dependencies:

• Existing Warp dependency
• CUDA required for the native controller backend and performance test

Fixes #

Type of change

• New feature (non-breaking change which adds functionality)

Screenshots

Not applicable. This is camera post-processing and renderer/backend integration behavior.

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]

📋 Review: Add Sensor PPISP Controller Support for Camera Post-Processing

Summary

This PR adds PPISPAuto controller graph support to the Isaac Lab PPISP pipeline. It introduces a controller network (CNN feature pooling + MLP) that predicts per-camera exposure and color-latent parameters from HDR input images, enabling adaptive camera ISP at runtime. The implementation provides both a pure-Warp reference path and a native CUDA snippet backend for high-environment-count tiled camera workloads.

Overall, this is a well-structured, high-quality feature addition with strong test coverage. The architecture cleanly separates concerns (USD parsing → weight extraction → Warp/native execution → PPISP application).

---

✅ Strengths

• Excellent test coverage: NumPy reference equivalence, native-vs-Warp equivalence, cross-renderer integration (Isaac RTX, OVRTX, Newton), and an opt-in performance sweep
• Dual backend with auto-selection heuristic: Provides correctness reference (Warp) and performance path (native CUDA) with a sensible automatic choice based on env count and tile size
• Clean separation of parsing from execution: controller_source.py handles only USD/asset parsing; kernels.py handles only GPU execution
• Backwards compatible: Existing static PPISP path is untouched; the controller path is additive
• Good defensive coding: Weight count validation at import time, comprehensive input validation in kernel entry points

---

🔍 Findings (from initial review)

1. CI Failure: Changelog Fragment Check (Medium) ✅ Resolved

The changelog CI job is now passing with the .skip and .minor.rst fragments in place.

2. controller_source.py — Regex-based weight parsing is fragile (Medium)

File: source/isaaclab_ppisp/isaaclab_ppisp/controller_source.py, _extract_controller_weights()

The weight extraction relies on a regex matching kControllerWeights[TOTAL_WEIGHTS] = { ... };. The code comment acknowledges this fragility ("A renamed symbol or a numeric [241961] extent would break this match"). Consider:

• Adding a fallback pattern that also matches the literal numeric extent kControllerWeights[241961]
• Documenting the expected source format in a docstring so NRE export changes can be coordinated

3. kernels.py — Shared memory in native snippets may collide on large block dims (Low)

File: source/isaaclab_ppisp/isaaclab_ppisp/kernels.py

The native pool snippet declares __shared__ float gs_reduce[256] (matching PPISP_CONTROLLER_POOL_THREAD_GROUP_SIZE). If the block dim is ever changed without updating the snippet template, shared memory will silently under-allocate. Since the snippet uses Template.substitute() with a hardcoded dict, this is safe today, but a static assertion in the CUDA snippet (e.g., static_assert(blockDim.x == 256)) would catch any future mismatch at kernel compilation.

4. pipeline.py — Unbounded buffer cache growth (Low)

File: source/isaaclab_ppisp/isaaclab_ppisp/pipeline.py

_controller_buffers_by_shape caches scratch buffers keyed by (device, num_cameras, height, width). In scenarios where camera resolution or env count changes dynamically (e.g., curriculum learning with varying camera counts), old entries are never evicted. For typical fixed-resolution use cases this is fine, but consider adding an LRU eviction or a maximum cache size for robustness.

5. cfg.py — assert used for non-optional logic (Low)

File: source/isaaclab_ppisp/isaaclab_ppisp/cfg.py, line in _merge_shader_inputs_with_cfg

assert ppisp_cfg.shader_prim_path is not None

Using assert for runtime validation is fragile (assertions are stripped with -O). Consider using an explicit if ... raise ValueError(...) instead, especially since this is in the resolution path that renderer backends call.

6. generate_synthetic_gaussian_asset.py — Large embedded string literals (Informational)

The test fixture generator embeds ~400 lines of CUDA/Lua/USDA as module-level string constants. This works well and keeps the test self-contained, but if these grow further, extracting them to fixture files alongside the test might improve maintainability.

---

🧪 Test Assessment

The test additions are comprehensive and well-designed:

• Unit tests (test_ppisp_kernels.py): Controller network matches NumPy reference, native matches Warp, color latent ordering is correct
• Integration tests (3 renderer test files): Controller output matches static PPISP across Isaac RTX, OVRTX, and Newton
• USD parsing tests (test_ppisp.py): Embedded weight loading, shader preference order
• Performance benchmark (test_ppisp_pipeline_perf.py): Opt-in sweep with CSV output, memory guards, proper GPU timing via Warp events

One minor suggestion: the performance test uses lambda closures in timing loops (lines with _time_gpu_operation(lambda: ...)). Due to Python's late binding, if any captured variable is reassigned in the loop body, the lambda may reference the wrong value. This appears safe in the current code since the captured arrays are not reassigned within the timing section, but worth noting.

---

📊 Verdict

Approve with minor suggestions. This is a solid, well-tested feature addition that follows Isaac Lab's existing patterns. The findings above are all low-to-medium severity and none block merging.

---

🔄 Updates

Update 2 (a55496c — 2026-06-04T11:49:39Z):

The PR was rebased on top of the develop branch (previous head was c4d7e19, now a55496cc33b7c06c5896d989b7deca586887c357). This incorporates base-branch changes (version bumps, changelog rollups, other packages' changes) but introduces no substantive changes to the PPISP feature code.

Previous findings status:

• Finding #1 (CI changelog): Resolved ✅
• Finding #2 (regex fragility): Unchanged — still uses TOTAL_WEIGHTS symbol
• Finding #3 (shared memory size): Unchanged — hardcoded gs_reduce[256]
• Finding #4 (buffer cache unbounded): Unchanged
• Finding #5 (assert vs raise): Still uses assert ppisp_cfg.shader_prim_path is not None
• Finding #6 (embedded strings): Unchanged — informational only

No new issues identified in this rebase. The PR remains ready to merge pending CI green.

---

Update (3e495ee): All previous concerns addressed:

• ✅ Fixed: Magic literal 1601 replaced with named constant PPISP_CONTROLLER_MLP_INPUT_DIM in the offset table and both MLP kernel branches.
• ✅ Fixed: file:// URI handling now uses proper urlparse/unquote via _file_uri_to_path(), correctly handling UNC authority-form URIs (with dedicated test coverage).

No new issues found in the incremental changes.

[greptile-apps]

Greptile Summary

This PR adds PPISPAuto controller support to the IsaacLab PPISP camera post-processing pipeline. It parses embedded CNN+MLP controller weights from USD/SPG-authored CUDA sources, stores them in PpispCfg, and evaluates the controller in Warp (or a native CUDA snippet backend) before applying the PPISP ISP transform per camera per frame.

• Controller parsing (controller_source.py): reads kControllerWeights from a .cu source asset (local file, USDZ member, or omni.client URL), validates the exact 241,961-element count, and stores the tuple in PpispCfg.controller_weights.
• Dual-backend controller inference (kernels.py): a pure-Warp reference path (CNN pool + MLP via tile primitives) and a native CUDA snippet path for high-env/small-tile workloads; pipeline.py caches per-device weight arrays and per-shape scratch buffers and dispatches to the right backend automatically.

Confidence Score: 4/5

The new controller inference path is well-structured and the dual-backend design is sound; the only comments are style-level maintainability concerns around hardcoded constants.

The hardcoded literal 1601 for the trunk0 weight-row stride appears in the module-level offset constant and twice inside the Warp MLP kernel; numerically it matches PPISP_CONTROLLER_FEATURE_LEN + 1 today, but a future architectural change to the feature dimension would silently produce wrong weight indexing without any compile-time or runtime error. The block_dim == 1 branch in the Warp MLP kernel is unreachable under the current launch configuration and would fail at runtime on CUDA if somehow triggered, but it doesn't affect the hot path. No blocking correctness issues were found.

source/isaaclab_ppisp/isaaclab_ppisp/kernels.py — the hardcoded 1601 literal in the offset table and MLP kernel loops

Important Files Changed

Filename	Overview
source/isaaclab_ppisp/isaaclab_ppisp/kernels.py	Adds CNN-pool and MLP Warp kernels plus native CUDA snippet equivalents; uses hardcoded literal 1601 for the trunk0 weight-row stride in both the module-level offset constant and the Warp MLP kernel loops instead of PPISP_CONTROLLER_FEATURE_LEN + 1
source/isaaclab_ppisp/isaaclab_ppisp/controller_source.py	New module for USD PPISPAuto shader parsing; multi-strategy source-asset resolution (local, USDZ, omni.client, Ar resolver) is thorough; regex weight extraction and count validation are clear
source/isaaclab_ppisp/isaaclab_ppisp/pipeline.py	Adds controller dispatch logic and per-device/shape buffer caching to PpispPipeline; logic is sound and the None-cfg guard is correct
source/isaaclab_ppisp/isaaclab_ppisp/cfg.py	Adds PPISPAuto constants, controller fields to PpispCfg, preference-ordered shader name discovery, and assert guard in _merge_shader_inputs_with_cfg; resolve_and_normalize correctly drops redundant stage argument after pre-resolution
source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py	Drops redundant stage= argument from PpispPipeline constructor; correct because resolve_and_normalize already fully populates the cfg before this call

Sequence Diagram

sequenceDiagram
    participant USD as USD Stage
    participant CS as controller_source.py
    participant CFG as PpispCfg
    participant PL as PpispPipeline
    participant KNL as kernels.py

    USD->>CS: ppisp_cfg_from_usd_shader(PPISPAuto shader)
    CS->>CS: _find_controller_shader_for_auto_shader()
    CS->>CS: _read_shader_source_asset() [local/USDZ/omni.client/Ar]
    CS->>CS: _extract_controller_weights() [regex → 241 961 floats]
    CS-->>CFG: PpispCfg(controller_weights, controller_prior_exposure)

    Note over PL: PpispPipeline.__init__(cfg)
    PL->>PL: normalize_ppisp_cfg(cfg)

    loop per frame
        PL->>PL: _compute_controller_params(hdr)
        PL->>PL: _controller_weights_by_device [cache wp.array]
        PL->>PL: _controller_buffers [cache scratch (features, params)]
        PL->>KNL: compute_ppisp_controller_params(hdr, weights, features, params)
        KNL->>KNL: _resolve_ppisp_controller_backend() → warp/native
        alt Warp backend
            KNL->>KNL: launch_tiled _ppisp_controller_pool_features_kernel
            KNL->>KNL: launch_tiled _ppisp_controller_mlp_kernel
        else Native CUDA backend
            KNL->>KNL: launch_tiled _ppisp_controller_pool_features_native_kernel
            KNL->>KNL: launch_tiled _ppisp_controller_mlp_native_kernel
        end
        KNL-->>PL: controller_params (N×9)
        PL->>KNL: apply_ppisp_to_rgba_with_controller_params(hdr, rgba, cfg, params)
        KNL-->>PL: LDR RGBA output
    end

Loading

Comments Outside Diff (1)

1. source/isaaclab_ppisp/isaaclab_ppisp/kernels.py, line 172-173 (link)

   Single-thread path is unreachable with current launch configuration

   _ppisp_controller_mlp_kernel is always launched via compute_ppisp_controller_params_warp with block_dim=PPISP_CONTROLLER_MLP_THREAD_GROUP_SIZE (128), so wp.block_dim() == 1 is never true on CUDA. The branch appears intended as a CPU-fallback or serial correctness reference, but Warp tile primitives (wp.tile_zeros, wp.tile_scatter_masked, wp.tile_extract) are CUDA-only and would error on a CPU device. A brief comment explaining when this path is expected to run would prevent future confusion.

_{Reviews (1): Last reviewed commit: "Add PPISP controller Warp support" | Re-trigger Greptile}

[isaaclab-review-bot]

Re-review (commit 3e495ee):

All previously raised concerns have been addressed in this update:

✅ Magic literal 1601 replaced - Now uses PPISP_CONTROLLER_MLP_INPUT_DIM consistently across all offset calculations and kernel code

✅ file:// URI handling fixed - _file_uri_to_path() now uses urlparse to correctly handle UNC-style URIs (e.g., file://server/share/path) with proper authority preservation

The new commits also include:

• Substantial test coverage for the controller network (NumPy reference implementation, native vs Warp comparison)
• Performance benchmark suite for the PPISP pipeline
• Proper urlparse/unquote for file URI decoding with tests verifying authority handling

No new blocking issues identified in this update. The PPISP controller implementation looks solid.

---

Update (b3173b0): Reviewed incremental changes. This commit completes the architectural shift from shader-based to camera-attribute-based PPISP discovery (ppisp_cfg_from_usd_camera replaces ppisp_cfg_from_usd_shader), adds native CUDA controller pool/MLP kernels via wp.func_native with proper Template substitution of architecture constants, and introduces PpispPipeline controller weight caching with per-device/shape buffer management. Tests updated comprehensively. No new issues found.

---

Update (4ddbcfb): Reviewed incremental changes. This commit finalizes the camera-attribute PPISP model:

• Demo scripts refactored: shared binding helpers extracted into _demo_utils.py, removing ~100 lines of duplication between ppisp_camera.py and ppisp_camera_ovrtx.py
• resolve_and_normalize now gracefully handles camera_prim_path=None by falling back to stage-wide discovery
• Renderer prepare_cameras hooks (Isaac RTX, OVRTX, Newton) updated to tolerate empty spec.camera_prim_paths
• Comprehensive new integration tests for camera-authored static attrs and controller-vs-static equivalence across all three renderer backends
• Unrelated quality fixes included: build_simulation_context device-override semantics, disabled_fabric_change_notifies kitless no-op, OVPhysX redundant USD replication removal, Newton visualizer particle options

No new issues found.

---

Update (da239e0): Reviewed incremental changes. Significant additions in this push:

• Newton Fabric body binding overhaul: _rename_builder_labels now returns fabric body bindings and _initialize_fabric_body_prims handles per-body Fabric attribute writes (creates missing prims as Xforms with world-matrix attrs). Well-tested with new test_newton_fabric_body_sync.py.
• split_clone_template utility: Extracted from ad-hoc partition("{}") calls across Newton, OVPhysX, and PhysX cloner modules. Adds a ValueError when the template lacks {}. New tests in test_cloner.py.
• Multi-backend task migration: Stack (UR10, Galbot) and Place (Agibot) tasks expose PhysicsCfg presets with config-time validation rejecting unsupported backend/feature combos (surface gripper on Newton, reversed-joint robots).
• Teleop device fix: SE(3) delta devices (keyboard/gamepad/spacemouse) moved from ReachEnvCfg base to ik_rel_env_cfg.py only, fixing action-shape mismatch on absolute-IK and joint-pos variants.
• isaacteleop import guard: Galbot stack configs now tolerate missing isaacteleop dependency.
• Newton visualizer set_camera_view fix: Stores updated eye/lookat in cfg and applies immediately even before initialization.
• Surface gripper wildcard regex: re.sub(r"(?<!\.)\\*", ".*", ...) converts bare * to .* for GripperView compatibility.
• OVPhysX CI markers: Removed @pytest.mark.isaacsim_ci from all OVPhysX tests (device-split marker handles CI scheduling).
• Rendering test refactor: _make_sensor_data_type_params helper de-duplicates the AOV combination matrix.

No new issues found.

Update (1bc33e5): Reviewed incremental changes. This push is primarily a large-scale documentation cleanup:

• Removed deprecated --headless flag from ~50+ code examples across docs (RL training, imitation learning, benchmarks, quickstart, policy deployment, troubleshooting, etc.). Commands now run headless by default when no visualizer is selected.
• PPISP test fixtures expanded: generate_synthetic_gaussian_asset.py gains render_synthetic_gaussian_scene_with_static_ppisp_attrs and render_synthetic_gaussian_scene_with_controller_ppisp_attrs helpers plus assert_images_meaningfully_different / assert_ppisp_controller_matches_static assertion utilities.
• New integration tests across Isaac RTX, Newton, and OVRTX backends for camera-authored static PPISP attributes and controller-vs-static equivalence.
• isaaclab_teleop README updated to reference --visualizer none / --viz none instead of deprecated --headless.
• Changelog skip files added for isaaclab and isaaclab_physx packages (internal-only changes).

No new issues found.

---

Update (1bc33e5, re-trigger): Base branch (develop) was updated with PR #6102 (deprecated --headless flag docs cleanup), causing a webhook re-trigger. The PR HEAD remains at commit 1bc33e59 which was already reviewed above. No new changes to review.

---

Update (02a0a3b): Reviewed incremental changes. This push includes:

• RMPFlow quaternion convention fix (rmp_flow.py): Removes the spurious convert_quat(..., to="xyzw") call — IsaacLab quaternions are already (x, y, z, w), so the double-conversion was scrambling the orientation target. New regression test test_rmpflow_reset_stability.py validates that zero-command arms hold their reset pose across 4 RMPFlow task configs.
• PPISP docs updated for controller workflow (explains ppisp:controllerWeights camera attribute, controller-then-image-pass pipeline, per-view predictions).
• Teleop docs: Added note about humanoid arm joint limits and RMPFlow preference.
• Task config cleanup: Removed redundant zero-offset OffsetCfg entries and added viewer eye/lookat defaults for Galbot/Agibot tasks.
• Minor: Augmented imitation docs tip about verifying models directory, changelog entries.

No new issues found.

[moennen]

@pbarejko : CI failures seem unrelated.