Skip to content

[Fix] Bump test_rendering_shadow_hand.py timeout to 1500s#6431

Merged
mataylor-nvidia merged 1 commit into
isaac-sim:developfrom
mataylor-nvidia:fix/rendering-timeout-per-aov
Jul 8, 2026
Merged

[Fix] Bump test_rendering_shadow_hand.py timeout to 1500s#6431
mataylor-nvidia merged 1 commit into
isaac-sim:developfrom
mataylor-nvidia:fix/rendering-timeout-per-aov

Conversation

@mataylor-nvidia

Copy link
Copy Markdown

Description

test_rendering_shadow_hand.py builds and tears down a full RTX env once per parametrized AOV (data type). It had no entry in PER_TEST_TIMEOUTS, so it fell back to DEFAULT_TIMEOUT (1000s). That budget never scaled as data types were added to _DEFAULT_SENSOR_DATA_TYPES (the list grew from 7 → 13), so the file eventually exceeded 1000s and timed out on CI (retry-masked).

This gives the file an explicit 1500s budget (~45s per AOV) and adds a reminder comment to bump it when renderer cases are added.

Fixes nvbug 6299039.

Type of change

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

Screenshots

N/A

Checklist

  • 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

🤖 Generated with Claude Code

The rendering correctness test builds and tears down a full RTX env once
per parametrized AOV (data type). It fell back to DEFAULT_TIMEOUT (1000s),
which never scaled as data types were added to _DEFAULT_SENSOR_DATA_TYPES,
so the file eventually exceeded the budget and timed out (nvbug 6299039).

Give it an explicit 1500s budget (~45s per AOV) with a reminder to bump it
when renderer cases are added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added bug Something isn't working infrastructure labels Jul 8, 2026
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds an explicit 1500s timeout entry for test_rendering_shadow_hand.py in PER_TEST_TIMEOUTS, replacing the previous implicit fallback to DEFAULT_TIMEOUT (1000s) that was insufficient as _DEFAULT_SENSOR_DATA_TYPES grew from 7 to 13 entries.

  • Adds "test_rendering_shadow_hand.py": 1500 to tools/test_settings.py, providing headroom for all 29 parametrized combinations (13 data types × 2 RTX backends + 3 Warp renderer cases at ~45s each ≈ 1305s peak).
  • Includes a maintenance comment pointing future contributors to rendering_test_utils.py when adding new AOV cases.

Confidence Score: 5/5

Single-line config addition to a well-understood timeout dictionary; no logic or runtime behavior changes.

The change adds one dictionary entry and a two-line comment. The chosen value (1500s) comfortably covers all 29 current parametrized combinations (~1305s worst case), leaving a ~15% buffer. The maintenance comment correctly points to the growth site (_DEFAULT_SENSOR_DATA_TYPES in rendering_test_utils.py). There are no side effects on other tests.

No files require special attention.

Important Files Changed

Filename Overview
tools/test_settings.py Adds explicit 1500s timeout for test_rendering_shadow_hand.py; straightforward config-only change with helpful maintenance comment.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CI runner picks up test_rendering_shadow_hand.py"] --> B["Looks up PER_TEST_TIMEOUTS"]
    B -->|"before PR"| C["Falls back to DEFAULT_TIMEOUT\n= 1000s"]
    B -->|"after PR"| D["Explicit entry\n= 1500s"]
    C --> E["13 data types × 2 RTX backends\n+ 3 Warp combos = 29 cases\n~45s each → ~1305s\n❌ Exceeds 1000s → Timeout"]
    D --> F["Same 29 cases × ~45s = ~1305s\n✅ Within 1500s → Pass"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["CI runner picks up test_rendering_shadow_hand.py"] --> B["Looks up PER_TEST_TIMEOUTS"]
    B -->|"before PR"| C["Falls back to DEFAULT_TIMEOUT\n= 1000s"]
    B -->|"after PR"| D["Explicit entry\n= 1500s"]
    C --> E["13 data types × 2 RTX backends\n+ 3 Warp combos = 29 cases\n~45s each → ~1305s\n❌ Exceeds 1000s → Timeout"]
    D --> F["Same 29 cases × ~45s = ~1305s\n✅ Within 1500s → Pass"]
Loading

Reviews (1): Last reviewed commit: "[Fix] Bump test_rendering_shadow_hand.py..." | Re-trigger Greptile

@mataylor-nvidia
mataylor-nvidia merged commit 7d75759 into isaac-sim:develop Jul 8, 2026
38 checks passed
hujc7 added a commit to hujc7/IsaacLab that referenced this pull request Jul 8, 2026
Re-sync onto latest develop (adds isaac-sim#6431 rendering-test timeout bump, isaac-sim#5730,
isaac-sim#6363, OVPhysX test enablement). Keep device_scope test_devices() migration
in test_rigid_object.py over develop's cpu/cuda:0 list.
hujc7 added a commit that referenced this pull request Jul 13, 2026
…overage (#5823)

## 1. Summary

- **CI:** Adds a dedicated multi-GPU pytest workflow that runs one shard
per non-default `cuda:N`, with a shared atomic work queue, per-file
reports, and end-of-run reconciliation.
- **isaaclab:** Adds composable `DeviceScope` flags and `test_devices()`
for `scope ∩ runtime` parametrization while retaining exact custom
string masks.
- **isaaclab:** Uses `ISAACLAB_TEST_DEVICES` as the single source for
both test parametrization and explicit Kit launch-device selection;
`AppLauncher` has no implicit test-environment override.
- **isaaclab:** Migrates 19 device-parametrized test modules across the
core, Newton, OV PhysX, and PhysX suites while preserving the existing
cpu + cuda:0 single-GPU behavior.

## 2. Device selection

```python
from isaaclab.test.utils import DeviceScope, test_devices

test_devices()                                                  # cpu + every GPU
test_devices(DeviceScope.CUDA)                                  # every GPU
test_devices(DeviceScope.CPU | DeviceScope.NON_DEFAULT_CUDA)    # composed scope
test_devices("101X")                                            # exact custom mask
```

- `ISAACLAB_TEST_DEVICES` limits which devices a run may use; unset
preserves the historical cpu + cuda:0 runtime.
- The multi-GPU lane discovers non-default-capable scopes and narrows
each shard to one concrete device.

## 3. Dependencies

All prerequisite changes have merged into `develop`, so this PR no
longer carries them — it is now scoped to the device-selection
infrastructure and the multi-GPU pytest workflow only:

- #5695 — Cross-platform Part 1 (base pytest markers / shared
scaffolding).
- #5881 — Sim honors the device kwarg over `sim_cfg.device` in
`build_simulation_context`.
- #5933 — Kit renderer defaults to one GPU + adds
`ISAACLAB_FABRIC_USE_GPU_INTEROP`; the multi-GPU lane sets
`ISAACLAB_FABRIC_USE_GPU_INTEROP=0`.
- #6322 — Newton multi-GPU initialization on non-default CUDA devices
(`cuda:1` and higher).

## 4. Test plan

- [x] Device-selection unit suite passes.
- [x] Changed Python modules compile; multi-GPU shell scripts pass `bash
-n`.
- [x] Full documentation build (`./isaaclab.sh -d`).
- [x] Full pre-commit suite (`./isaaclab.sh -f`).
- [x] GitHub Actions green on latest `develop` (one
`rendering-correctness-kitless` flake cleared on rerun; fixed upstream
on `develop` by #6431).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants