Skip to content

Commit ca23aca

Browse files
authored
Re-enable the Kit visualizer pause checks (#7049)
Temporary workaround for NVBUG 6570125 while we wait for the upstream renderer fix. ## Problem RTX responsiveDenoising continues refining the image after physics stops, so fresh renders captured during a pause are no longer stable. The affected Kit visualizer checks are currently green for the wrong reasons: Tiled camera: #6658 changed the paused captures to reuse the cached sensor frame, so the assertion effectively compares the same frame against itself and can't detect renderer instability. Viewport: still performs a real comparison, but the known denoiser noise can push it over the existing threshold and the failure is absorbed by retries. ## What this changes This PR restores fresh renders for the tiled pause check and adjusts the per-channel difference threshold only for the affected Kit RTX pause paths. The existing 100-pixel gate stays unchanged. We're changing how large a per-channel difference must be before a pixel is counted, rather than simply allowing more differing pixels. | site | @50 | @80 | @100 | @120 | @150 | @160 | outcome | |---|---|---|---|---|---|---|---| | Kit viewport (Newton) | 67 | **12** | 8 | 7 | 3 | — | 80 | | Kit viewport (PhysX) | 14 | 4 | 3 | 1 | 0 | — | 80 | | Kit tiled (Newton) | 999 | 604 | 226 | 120 | 59 | **49** | 160 | | Kit tiled (PhysX) | 103 | 4 | 4 | 2 | 0 | — | 80 | | Newton ViewerGL | 0 | 0 | 0 | 0 | 0 | — | 50 | Values are the number of differing pixels at each per-channel threshold. For the margin-critical case (Kit tiled camera, Newton backend), threshold 160 leaves at most 49 px of paused denoiser noise (worst of n=36) against at least 160 px with real motion (weakest of n=72), both measured with the denoiser enabled. With the existing 100-pixel gate the known renderer noise passes at 2.0× margin while real motion is still detected at 1.6×. ## Changes `_assert_frames_remain_stable` now accepts a per-call `channel_diff_threshold`; the default remains 50. Kit RTX pause checks use 80 for the viewport, 160 for the tiled camera on Newton, and 80 for the tiled camera on PhysX — three separate constants, since the tiled PhysX cell measures 103 px at the default threshold and must not silently track a later change to the viewport value. Newton ViewerGL remains at the strict default since it doesn't exercise the affected DLSS path. `max_differing_pixels` remains 100 everywhere. The tiled pause check is restored to fresh renders instead of cached frames, so the assertion is actually exercising renderer stability again. ## Validation With retries disabled, the workaround passed 40/40 runs across all four affected configurations. As a control, restoring the original threshold of 50 reproduced the failure 6/6 times at 766–949 differing pixels. Those six failures are all the **tiled** assertion: the control ran under `pytest -x` and the viewport test is defined first in the same file, so the run aborted at the tiled test and the viewport site passed at 50 in those reps. The control therefore demonstrates that the tiled workaround filters real denoiser noise rather than simply making the test pass; it is not evidence for the viewport site. The viewport raise from 50 to 80 rests instead on production CI, where the assertion has really been failing (`107 pixels differed, expected at most 100`, with a historical band reaching 161 px), plus n=5 bench samples showing 67 px at threshold 50 falling to 12 px at 80. The same change was validated on three independent L40S CI runners through #7055, with all four visualizer integration tests passing first-pass, no retries. Restoring fresh renders also had no meaningful runtime impact (~27s median). This is a temporary workaround for NVBUG 6570125 and should be removed or re-measured once the upstream fix is available. That revert is load-bearing rather than cosmetic: with the denoiser disabled — the world after the renderer fix — the weakest real motion in the governing cell measures 80 px at threshold 160, which is *below* the 100-pixel gate. Leaving 160 in place after the fix ships would make the tiled check blind to the weakest motion it exists to catch, so the removal comment on the constants is the thing guarding that. --------- Signed-off-by: fanes <74020209+fatimaanes@users.noreply.github.com>
1 parent a4c0d82 commit ca23aca

2 files changed

Lines changed: 35 additions & 11 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Test-only change: raises the per-channel difference threshold in the visualizer integration
2+
test helper for the Kit RTX pause checks, and restores fresh renders in the paused tiled
3+
capture (NVBUG 6570125 work-around). No user-facing behavior change.

source/isaaclab_visualizers/test/visualizer_integration_utils.py

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,20 @@
175175
_TILED_CAMERA_MOTION_MIN_DIFFERING_PIXELS = 25
176176
"""Minimum differing pixels for tiled camera motion checks."""
177177

178+
# NVBUG 6570125 — Remove these overrides once it ships the fix and paused frames are stable again.
179+
_KIT_PAUSED_VIEWPORT_CHANNEL_DIFF_THRESHOLD = 80
180+
"""Per-channel threshold for paused Kit viewport comparisons (0–255 space)."""
181+
182+
_KIT_PAUSED_TILED_CAMERA_NEWTON_CHANNEL_DIFF_THRESHOLD = 160
183+
"""Per-channel threshold for paused Kit tiled camera comparisons on Newton (0–255 space)."""
184+
185+
_KIT_PAUSED_TILED_CAMERA_PHYSX_CHANNEL_DIFF_THRESHOLD = 80
186+
"""Per-channel threshold for paused Kit tiled camera comparisons on PhysX (0–255 space).
187+
188+
Matches the viewport value but is kept separate: this cell measures 103 differing pixels at the
189+
default threshold, so it needs its own floor rather than tracking whatever the viewport uses.
190+
"""
191+
178192
_FRAME_MIN_CHANNEL_RANGE = 10
179193
"""Minimum per-frame channel range to reject all-one-color images."""
180194

@@ -569,12 +583,14 @@ def _assert_frames_remain_stable(
569583
phase: str,
570584
debug_phase: str,
571585
max_differing_pixels: int = 100,
586+
channel_diff_threshold: float = _FRAME_MOTION_CHANNEL_DIFF_THRESHOLD,
572587
) -> None:
573588
"""Assert two viewport frames are effectively unchanged while simulation is paused."""
574-
n_diff = _count_significantly_differing_pixels(frame_a, frame_b)
589+
n_diff = _count_significantly_differing_pixels(frame_a, frame_b, channel_diff_threshold=channel_diff_threshold)
575590
assert n_diff <= max_differing_pixels, (
576591
f"{case_label} failed to pause during {phase}: {n_diff} pixels differed, expected at most "
577-
f"{max_differing_pixels}. Frame shape={_frame_shape_for_message(frame_a)}. "
592+
f"{max_differing_pixels} with per-channel threshold {channel_diff_threshold} in 0-255 space. "
593+
f"Frame shape={_frame_shape_for_message(frame_a)}. "
578594
f"Debug frames: {_current_visualizer_debug_dir()}/*{debug_phase}*.png."
579595
)
580596

@@ -1259,6 +1275,7 @@ def _attempt_kit_pause():
12591275
case_label=case_label,
12601276
phase="pausing",
12611277
debug_phase="pausing",
1278+
channel_diff_threshold=_KIT_PAUSED_VIEWPORT_CHANNEL_DIFF_THRESHOLD,
12621279
)
12631280

12641281
try:
@@ -1400,16 +1417,13 @@ def _run_visualizer_tiled_camera_motion_test(env, visualizer, *, physics_kind: s
14001417

14011418
def _attempt_pause():
14021419
_set_kit_simulation_paused(env, True)
1403-
# Read the sensor's last completed frame while paused. Forcing a new RTX
1404-
# render here introduces TAA edge jitter even though physics is frozen.
1405-
paused_start_frame = _capture_visualizer_tiled_camera_rgb(
1406-
visualizer, label="2a_pausing_frame_20", force_recompute=False
1407-
)
1420+
# Re-render both paused captures: comparing the sensor's cached frame with itself cannot
1421+
# detect a renderer that keeps changing the image after physics stops. The denoiser residue
1422+
# that motivated caching is handled by the per-channel threshold below (NVBUG 6570125).
1423+
paused_start_frame = _capture_visualizer_tiled_camera_rgb(visualizer, label="2a_pausing_frame_20")
14081424
for _ in range(PAUSE_VIZ_N_STEP):
1409-
env.sim.render(skip_app_pumping=isinstance(visualizer, KitVisualizer))
1410-
paused_end_frame = _capture_visualizer_tiled_camera_rgb(
1411-
visualizer, label="2b_pausing_frame_25", force_recompute=False
1412-
)
1425+
env.sim.render()
1426+
paused_end_frame = _capture_visualizer_tiled_camera_rgb(visualizer, label="2b_pausing_frame_25")
14131427
_save_visualizer_debug_phase_images(
14141428
paused_start_frame,
14151429
paused_end_frame,
@@ -1425,6 +1439,13 @@ def _attempt_pause():
14251439
case_label=case_label,
14261440
phase="pausing",
14271441
debug_phase="pausing_tiled",
1442+
channel_diff_threshold=(
1443+
_KIT_PAUSED_TILED_CAMERA_NEWTON_CHANNEL_DIFF_THRESHOLD
1444+
if isinstance(visualizer, KitVisualizer) and physics_kind == "newton"
1445+
else _KIT_PAUSED_TILED_CAMERA_PHYSX_CHANNEL_DIFF_THRESHOLD
1446+
if isinstance(visualizer, KitVisualizer)
1447+
else _FRAME_MOTION_CHANNEL_DIFF_THRESHOLD
1448+
),
14281449
)
14291450

14301451
try:

0 commit comments

Comments
 (0)