diff --git a/.agents/skills/isaaclab-auditing-an-issue b/.agents/skills/isaaclab-auditing-an-issue new file mode 120000 index 000000000000..0c26b7a577cc --- /dev/null +++ b/.agents/skills/isaaclab-auditing-an-issue @@ -0,0 +1 @@ +../../skills/developer/issue-audit \ No newline at end of file diff --git a/.agents/skills/isaaclab-triaging-issue-backlog b/.agents/skills/isaaclab-triaging-issue-backlog new file mode 120000 index 000000000000..97114ccf6e82 --- /dev/null +++ b/.agents/skills/isaaclab-triaging-issue-backlog @@ -0,0 +1 @@ +../../skills/developer/issue-backlog-triage \ No newline at end of file diff --git a/.github/workflows/nightly-isaacsim-image.yml b/.github/workflows/nightly-isaacsim-image.yml index 42f59a3cc5de..5a3d97546cdc 100644 --- a/.github/workflows/nightly-isaacsim-image.yml +++ b/.github/workflows/nightly-isaacsim-image.yml @@ -179,6 +179,7 @@ jobs: CURRENT_PIN: ${{ steps.pin.outputs.current }} CANDIDATE_PIN: ${{ steps.pin.outputs.candidate }} DIGEST: ${{ steps.pin.outputs.digest }} + BRANCH_CHANGED: ${{ steps.pin.outputs.branch_changed }} run: | set -euo pipefail @@ -187,6 +188,8 @@ jobs: title="[CI] Bump Isaac Sim image to $short_digest" body_file="$RUNNER_TEMP/isaacsim-image-update.md" { + echo "# Description" + echo echo "This automated draft updates CI to the current Isaac Sim nightly image." echo echo "| Field | Value |" @@ -199,6 +202,30 @@ jobs: echo "Source: https://registry.ngc.nvidia.com/orgs/0947644777160149/teams/internal/containers/isaac-sim/tags" echo echo "New PRs are opened as drafts so maintainers can merge after the CI results are acceptable." + echo + echo "## Type of change" + echo + echo "- Infrastructure update" + echo + echo "## Release backport" + echo + echo '- [ ] Backport this pull request to the active release branch after it merges into `develop`' + echo + echo "## Screenshots" + echo + echo "Not applicable." + echo + echo "## Checklist" + echo + echo "Docker and GPU tests run on demand. This workflow comments \`run-ci\` after opening the pull request or updating its commit." + echo + echo '- [ ] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)' + echo '- [ ] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format`' + echo '- [ ] I have made corresponding changes to the documentation' + echo '- [ ] My changes generate no new warnings' + echo '- [ ] I have added tests that prove my fix is effective or that my feature works' + echo '- [ ] I have added a changelog fragment under `source//changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that)' + echo '- [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there' } > "$body_file" repository_owner=${REPOSITORY%%/*} @@ -208,6 +235,7 @@ jobs: -f head="$repository_owner:$UPDATE_BRANCH" \ --jq '.[0].number // empty') + pr_created=false if [ -n "$pr_number" ]; then pr_url=$(gh api --method PATCH "repos/$REPOSITORY/pulls/$pr_number" \ -f title="$title" \ @@ -223,10 +251,18 @@ jobs: -F body=@"$body_file" \ -F draft=true \ --jq '.html_url') + pr_number=${pr_url##*/} + pr_created=true echo "Opened draft PR: $pr_url" echo "Draft PR: $pr_url" >> "$GITHUB_STEP_SUMMARY" fi + if [ "$BRANCH_CHANGED" = "true" ] || [ "$pr_created" = "true" ]; then + gh api --method POST "repos/$REPOSITORY/issues/$pr_number/comments" \ + -f body='run-ci' >/dev/null + echo "Triggered CI on PR #$pr_number." + fi + - name: Report no-op or dry run if: ${{ steps.pin.outputs.changed != 'true' || inputs.dry_run }} env: diff --git a/skills/developer/issue-audit/SKILL.md b/skills/developer/issue-audit/SKILL.md new file mode 100644 index 000000000000..c3d4b5247bfd --- /dev/null +++ b/skills/developer/issue-audit/SKILL.md @@ -0,0 +1,142 @@ +--- +name: isaaclab-auditing-an-issue +description: Audits one open GitHub issue against current code to decide close or keep-open, and drafts the closing comment. Use when deciding whether a specific issue is still valid, whether a reported bug was fixed, or what to reply before closing. +audience: developer +status: experimental +owners: + - isaaclab-maintainers +--- + +# Auditing an Issue + +## When To Use + +Use this skill for a single issue: deciding whether it is still valid against current code, and drafting the reply if it is being closed. + +For auditing many issues at once, use `isaaclab-triaging-issue-backlog`, which applies this skill per issue and adds the batch controls. + +Do not close an issue purely because it is old. Age is not evidence. + +## Core Principle + +A comment that reads well is not evidence that its claims are true. Verify every factual claim against current code before posting, and ask what is lost by closing, not only whether closing is defensible. + +## Workflow + +1. Read the whole thread, not just the first post: `gh issue view NUMBER --repo isaac-sim/IsaacLab --comments`. +2. Identify each checkable claim you intend to make in the reply. +3. Verify each one using the table below. Prefer running code over reading it. +4. Check the Do Not Close list before deciding. +5. Draft the comment in the style below, then confirm every link resolves. + +## Mandatory Checks + +| Claim shape | Required check | +|---|---| +| "X is fixed / landed in #N" | `gh pr view N` returns MERGED, and its diff touches the reported behavior. A generic "Merges changes from main" PR is never an acceptable citation. | +| "Isaac Lab doesn't have X" | Grep `source/isaaclab_tasks/isaaclab_tasks/contrib/`, `source/isaaclab_contrib/` and the `*_experimental` packages. Features move to contrib and absence claims go stale. | +| "`SYMBOL` no longer exists" | `grep -rn "SYMBOL" source/` — any live hit refutes it. | +| "`BACKEND` supports X" | Check that backend's own package. Backends differ; several classes raise `NotImplementedError` on one and not another. | +| Behavioral claim about an error or a limit | Write a temporary script and run it with `uv run python`. Reading the raise site is not enough — the guard may be narrower than it looks. | +| A usage answer | Confirm it is still the best current advice, not merely that some answer was given. Check the relevant capability on the Isaac Sim side too. | +| Any URL | `curl -s -o /dev/null -w "%{http_code}" -L "URL"` returns 200. | + +## URL Traps + +- `github.com/isaac-sim/IsaacLab/blob/main/...` **404s** for anything under `source/isaaclab_newton/`, `source/isaaclab_ov/` or `source/isaaclab_contrib/` — those packages are not on `main`. Use `blob/develop/...`, or cite the path in backticks with no link. Package layout shifts over time (e.g. `isaaclab_ovphysx` was merged into `isaaclab_ov`), so verify the path exists on the ref you link. +- `isaac-sim.github.io/IsaacLab/main/...` is the stale 2.x docs build. Use `release/3.0.0/...`, and confirm the anchor still exists — several 2.x anchors were removed in the 3.x reorganisation. + +## Do Not Close + +Hold the issue when any of these apply, even if the stated close reason is technically defensible: + +- A maintainer asked the reporter a question and the thread is still recent. +- It is pre-merge feedback on an open PR. +- It is a duplicate of an issue that was itself closed without a fix. +- The titled ask is unmet, even if a related bug in the thread was fixed. +- It is blocked on upstream work that is planned but not landed. Confirm the upstream issue exists; "file it upstream" is not a resolution if nobody has. + +When holding, record the concrete next step and the evidence, so the next audit does not redo the work. + +## Comment Style + +- Thank the reporter by handle in the first sentence, specifically and briefly. +- State what actually resolves it, with file paths in backticks and a verified PR link. +- Invite a reopen if it still reproduces on a current release. +- Do not assign fault. Avoid "we never heard back" (blames the reporter) and "nothing on our side" (blames a sibling team). Describe where the behavior lives, and route rather than deflect. +- Do not name a colleague as the source of a negative finding. Crediting a helpful answer is fine. +- If the reply was AI-assisted, end with the disclaimer footer below, and do not claim a level of human review that did not happen. + +### Disclaimer Footer + +End an AI-assisted reply with a horizontal rule and a one-line footer: + +```markdown +--- +*🤖 This issue was reviewed as part of an AI-assisted triage of the Isaac Lab backlog, with automated verification against the current codebase. If we got this wrong, please reopen — we'd rather hear about it!* +``` + +Match the claim to what actually happened. "with automated verification against the current codebase" is accurate for a machine-checked reply. Only say "verified by a maintainer" if a maintainer read that specific comment. + +### Worked Example + +A close for a bug that was genuinely fixed: + +```markdown +Thanks @Mitchell-Torok for catching that `reset_joints_by_offset` ignored the `joint_names` you passed in `SceneEntityCfg`. + +It's fixed: both `reset_joints_by_offset` and `reset_joints_by_scale` in `isaaclab/envs/mdp/events.py` now index the default joint state and the soft-limit clamp with `asset_cfg.joint_ids` (broadcasting `env_ids[:, None]` for subsets), so only the named joints are touched. Shipped in https://github.com/isaac-sim/IsaacLab/pull/2899. Sorry for the delay — please reopen if extra joints still move on a current release. + +--- +*🤖 This issue was reviewed as part of an AI-assisted triage of the Isaac Lab backlog, with automated verification against the current codebase. If we got this wrong, please reopen — we'd rather hear about it!* +``` + +Each part is doing work: the handle and the specific symptom in sentence one, the mechanism plus file path so the comment stands alone in search results, a PR link confirmed MERGED, a three-word apology rather than a paragraph, and a reopen invitation tied to a current release. + +### Fault Assignment + +The rewrites below are the difference between a reply that reads as closure and one that reads as a brush-off. + +| Instead of | Write | +|---|---| +| "We asked twice and never heard back." | "We weren't able to get to the bottom of this at the time, and a lot has changed since." | +| "There's nothing on our side to fix." | "That code path lives in ``, so `` is the better home for it." | +| "This is an Isaac Sim bug that their team owns." | "The `sim.reset()` requirement lives in that layer and is tracked there." | +| "As @SomeDev confirmed, it's broken." | State the finding directly, without attributing it to a colleague. | + +## Common Mistakes + +- Trusting a polished draft. Fluency correlates with neither accuracy nor completeness. +- Answering "was a reply given?" instead of "is that answer still correct and complete?" +- Citing a PR whose title matches the topic without reading its diff. +- Closing an issue whose stated reason is sound but which is the only remaining record of a live problem. + +## Validation + +Verify citations and links before posting: + +```bash +gh pr view PR_NUMBER --repo isaac-sim/IsaacLab --json state,mergedAt,title +curl -s -o /dev/null -w "%{http_code}" -L "URL" +``` + +For a behavioral claim, write a temporary script and run it: + +```bash +uv run python PATH_TO_SCRIPT +``` + +If skills changed, run: + +```bash +uv run --no-project python tools/skills/cli.py check +``` + +## Maintenance + +Keep the URL traps section synchronized with the current package layout. When a package moves between `main` and `develop`, or the docs build version changes, update it before it produces a dead link in a public comment. + +## References + +- [Backlog triage skill](../issue-backlog-triage/SKILL.md) +- [Contributing guide](../../../docs/source/refs/contributing.rst) diff --git a/skills/developer/issue-backlog-triage/SKILL.md b/skills/developer/issue-backlog-triage/SKILL.md new file mode 100644 index 000000000000..a89965a9eec9 --- /dev/null +++ b/skills/developer/issue-backlog-triage/SKILL.md @@ -0,0 +1,88 @@ +--- +name: isaaclab-triaging-issue-backlog +description: Triages many open GitHub issues at once into close/keep-open with drafted closing comments. Use when auditing the whole issue backlog, running a bulk close pass, or reporting how many issues can be retired. +audience: developer +status: experimental +owners: + - isaaclab-maintainers +--- + +# Triaging the Issue Backlog + +## When To Use + +Use this skill when auditing many issues at once: a backlog sweep, a bulk close pass, or a report on how much of the backlog can be retired. + +For a single issue, use `isaaclab-auditing-an-issue` directly. + +**REQUIRED SUB-SKILL:** Use `isaaclab-auditing-an-issue` for every individual issue. This skill only adds batch orchestration; it does not restate the per-issue checks. + +## Core Principle + +Scale multiplies confident errors. A batch of drafted comments will read uniformly well whether or not the claims are true, so verification must be adversarial and the human review gate is not optional. + +## Workflow + +1. Dump the backlog with metadata and split it into batches: + + ```bash + gh issue list --repo isaac-sim/IsaacLab --state open --limit 500 \ + --json number,title,author,createdAt,updatedAt,labels,comments,assignees,url + ``` + +2. Audit each issue per `isaaclab-auditing-an-issue`, recording a verdict, the evidence, and a confidence level. +3. Run an adversarial pass over every close recommendation. Instruct the reviewer to **refute**, and to return REFUTED or UNCERTAIN whenever it cannot independently confirm the evidence. Verify low-confidence recommendations as strictly as the rest — they refute at a far higher rate. +4. Sweep the drafted comments for claims that a per-issue audit misses across a batch: capability assertions, citations whose PR does not match, and dead links. +5. Review every comment with a maintainer, in batches, before anything is posted. +6. Post only what a maintainer explicitly approved. + +## Batch Controls + +- **Never infer approval.** Approve on an explicit statement, not on a reply that merely discusses a batch. Record approvals so a resumed session cannot post something unreviewed. +- **Keep one plan file** listing every issue to be closed with its final comment, plus appendices for held issues and for issues pulled out during review with their next steps. +- **Re-show anything edited after approval.** Later passes rewrite earlier comments; approval does not survive a rewrite. +- **Report what was excluded.** State how many issues were refuted, held, or pulled out, not only how many are closeable. + +## Expect These Failure Modes + +Measured over one full pass of the backlog; treat them as the default, not the exception. + +| Failure mode | What it looks like | +|---|---| +| Stale absence claims | "Isaac Lab doesn't support X" when X moved to `contrib/`. | +| Mismatched citations | A merged PR whose title fits the topic but whose diff never touches the behavior. | +| Capability drift | Asserting a backend or release supports something it does not. | +| Answered-but-wrong | A usage answer that was correct when written and is now outdated. | +| Blame framing | "We never heard back" or "nothing on our side", which read badly in public. | +| Defensible but lossy | A close whose reason is sound but which discards the only record of a live problem. | + +## Common Mistakes + +- Treating a high-confidence label as a substitute for verification. +- Batching approval across issues that were edited at different times. +- Reporting a reduction figure without the refuted and held counts. +- Posting before a maintainer has read the specific comment being posted. + +## Validation + +Before posting, confirm the batch is internally consistent: every comment ends with the intended marker, every cited PR is merged, and every URL resolves. + +```bash +gh pr view PR_NUMBER --repo isaac-sim/IsaacLab --json state,mergedAt,title +curl -s -o /dev/null -w "%{http_code}" -L "URL" +``` + +If skills changed, run: + +```bash +uv run --no-project python tools/skills/cli.py check +``` + +## Maintenance + +Keep the per-issue checks in `isaaclab-auditing-an-issue` and do not duplicate them here. Update the failure-mode table when a new class of error survives verification into a posted comment. + +## References + +- [Issue audit skill](../issue-audit/SKILL.md) +- [Contributing guide](../../../docs/source/refs/contributing.rst) diff --git a/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip b/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip new file mode 100644 index 000000000000..f08e4d9ff050 --- /dev/null +++ b/source/isaaclab/changelog.d/fix-standalone-soak-deadline.skip @@ -0,0 +1 @@ +Allowed standalone smoke tests that reached readiness before the startup deadline to complete their full soak interval. diff --git a/source/isaaclab/test/app/standalone_script_cases.py b/source/isaaclab/test/app/standalone_script_cases.py index d9a5e89351b7..f220f033ab16 100644 --- a/source/isaaclab/test/app/standalone_script_cases.py +++ b/source/isaaclab/test/app/standalone_script_cases.py @@ -489,7 +489,7 @@ def read_available_output(timeout: float) -> bool: _terminate_process_group(process) returncode = process.poll() break - if now - start_time >= startup_timeout: + if ready_at is None and now - start_time >= startup_timeout: _terminate_process_group(process) returncode = process.poll() break diff --git a/source/isaaclab/test/app/test_standalone_scripts.py b/source/isaaclab/test/app/test_standalone_scripts.py index 3d0b8c6480cf..a43cb2349b27 100644 --- a/source/isaaclab/test/app/test_standalone_scripts.py +++ b/source/isaaclab/test/app/test_standalone_scripts.py @@ -22,6 +22,7 @@ import sys from dataclasses import replace from pathlib import Path +from unittest import mock import pytest import standalone_script_cases as script_cases @@ -371,6 +372,37 @@ def test_subprocess_supervisor_soaks_then_stops_process_group(): assert result.elapsed < 2.0 +def test_subprocess_supervisor_completes_soak_after_startup_deadline(monkeypatch): + """Readiness just before the startup deadline must still receive the full soak.""" + process = mock.Mock(returncode=None) + process.poll.side_effect = lambda: process.returncode + process.communicate.return_value = (b"", None) + selector = mock.Mock() + now = 0.0 + poll_times = iter((299.0, 300.0, 304.0)) + + def select(timeout): + nonlocal now + if timeout == 0.0: + return [] + now = next(poll_times) + if now == 299.0: + return [(mock.Mock(fileobj=process.stdout), script_cases.selectors.EVENT_READ)] + return [] + + selector.select.side_effect = select + monkeypatch.setattr(script_cases.subprocess, "Popen", lambda *args, **kwargs: process) + monkeypatch.setattr(script_cases.selectors, "DefaultSelector", lambda: selector) + monkeypatch.setattr(script_cases.os, "read", lambda *args: b"READY\n") + monkeypatch.setattr(script_cases.time, "monotonic", lambda: now) + monkeypatch.setattr(script_cases, "_terminate_process_group", lambda process: setattr(process, "returncode", -15)) + + result = run_until_ready(["demo.py"], r"READY", startup_timeout=300.0, soak_time=5.0) + assert result.ready + assert result.stopped_after_soak + assert result.elapsed == 304.0 + + def test_subprocess_supervisor_ignores_fatal_output_after_intentional_teardown(monkeypatch): """Fatal-looking output caused by intentional teardown must not fail a healthy launch.""" diff --git a/source/isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip b/source/isaaclab_ov/changelog.d/fix-ovrtx-clone-plan-test-import.skip new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst b/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst new file mode 100644 index 000000000000..536a6351a26a --- /dev/null +++ b/source/isaaclab_rl/changelog.d/antoiner-pretrained-checkpoint-errors.minor.rst @@ -0,0 +1,17 @@ +Fixed +^^^^^ + +* Fixed :func:`~isaaclab_rl.utils.pretrained_checkpoint.get_published_pretrained_checkpoint` reporting + every download failure as ``A pre-trained checkpoint is currently unavailable for this task.``. A + checkpoint the asset server does not provide is still reported that way, but the message now names the + location that was tried, the task and backends it was derived from, and what to do instead. + +Changed +^^^^^^^ + +* Changed :func:`~isaaclab_rl.utils.pretrained_checkpoint.get_published_pretrained_checkpoint` to raise + ``RuntimeError`` when a published checkpoint cannot be downloaded, for instance when the + ``.pretrained_checkpoints`` cache directory is not writable, instead of returning ``None``. The + originating error is chained as the cause. ``None`` is now returned only when the asset server does not + report the checkpoint, which covers both an unpublished checkpoint and a server that could not be + reached; callers that relied on ``None`` to mask local download failures must catch ``RuntimeError``. diff --git a/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py b/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py index 4d2a30ac252e..d64c31ff9f80 100644 --- a/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py +++ b/source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py @@ -318,7 +318,16 @@ def get_published_pretrained_checkpoint( from :data:`sys.argv`. Legacy checkpoints do not use preset-qualified names. Returns: - The path. + The path, or None when the asset server does not report a checkpoint for this task + and backend combination. That covers both a checkpoint that was never published and + a server that could not be reached, which ``omni.client`` does not distinguish, so a + transient outage is not evidence that a checkpoint does not exist. The reason is + printed before returning. + + Raises: + RuntimeError: If the checkpoint is published but could not be downloaded, for + instance because the local cache directory is not writable. The originating + error is chained as the cause. """ if preset_names is None and physics_backend is not None and render_backend is not None: preset_names = get_pretrained_checkpoint_preset_names(task_name) @@ -339,9 +348,35 @@ def get_published_pretrained_checkpoint( print(f"Fetching pre-trained checkpoint : {ov_path}") try: resume_path = retrieve_file_path(ov_path, download_dir) - except Exception: - print("A pre-trained checkpoint is currently unavailable for this task.") + except FileNotFoundError: + # the asset server reports a checkpoint that was never published and a server it + # cannot reach the same way, so both are covered by the same message + backends = ( + "" + if physics_backend is None + else f" with the '{physics_backend}' physics and '{render_backend}' render backends" + ) + print( + "A pre-trained checkpoint is currently unavailable for this task.\n" + f" The asset server does not provide '{ov_path}'.\n" + f" Either no checkpoint is published for task '{task_name}'{backends}, or the asset" + " server could not be reached.\n" + " Train the task, or pass --checkpoint to use a checkpoint of your own." + ) return None + except Exception as exc: + # the checkpoint exists on the server, so this is a local failure the user has to fix; + # reporting it as an unavailable checkpoint would send them looking in the wrong place + hint = "" + if isinstance(exc, OSError): + hint = ( + " Check that the cache directory is writable and that the disk is not full;" + " a directory left behind by a container run is owned by root." + ) + raise RuntimeError( + f"Failed to download the pre-trained checkpoint '{ov_path}' into" + f" '{os.path.abspath(download_dir)}': {type(exc).__name__}: {exc}.{hint}" + ) from exc else: print("Using pre-fetched pre-trained checkpoint") return resume_path diff --git a/source/isaaclab_rl/test/test_pretrained_checkpoint.py b/source/isaaclab_rl/test/test_pretrained_checkpoint.py index 91aa9d494cd7..4983795dd8da 100644 --- a/source/isaaclab_rl/test/test_pretrained_checkpoint.py +++ b/source/isaaclab_rl/test/test_pretrained_checkpoint.py @@ -200,3 +200,55 @@ def _retrieve_file_path(remote_path: str, download_dir: str) -> str: "remote_path": f"omniverse://IsaacLab/PretrainedCheckpoints/rl_games/{filename}", "download_dir": expected_download_dir, } + + +def test_get_published_pretrained_checkpoint_reports_unpublished_checkpoint( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +): + """Test that a checkpoint missing from the asset server names the location that was tried.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(pretrained_checkpoint, "ISAACLAB_NUCLEUS_DIR", "omniverse://IsaacLab") + + def _retrieve_file_path(remote_path: str, download_dir: str) -> str: + raise FileNotFoundError(f"Unable to find the file: {remote_path}") + + monkeypatch.setattr(pretrained_checkpoint, "retrieve_file_path", _retrieve_file_path) + + path = pretrained_checkpoint.get_published_pretrained_checkpoint( + "rsl_rl", + "Isaac-Cartpole", + "physx", + "none", + ) + + assert path is None + output = capsys.readouterr().out + assert "A pre-trained checkpoint is currently unavailable for this task." in output + assert "omniverse://IsaacLab/PretrainedCheckpoints/rsl_rl/Isaac-Cartpole_physx_none_rsl_rl.pt" in output + assert "'physx' physics and 'none' render backends" in output + + +def test_get_published_pretrained_checkpoint_raises_on_unwritable_cache( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +): + """Test that a local download failure is reported instead of being reported as unavailable.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(pretrained_checkpoint, "ISAACLAB_NUCLEUS_DIR", "omniverse://IsaacLab") + cause = PermissionError(13, "Permission denied", str(tmp_path / ".pretrained_checkpoints")) + + def _retrieve_file_path(remote_path: str, download_dir: str) -> str: + raise cause + + monkeypatch.setattr(pretrained_checkpoint, "retrieve_file_path", _retrieve_file_path) + + with pytest.raises(RuntimeError) as error: + pretrained_checkpoint.get_published_pretrained_checkpoint("rsl_rl", "Isaac-Cartpole", "physx", "none") + + message = str(error.value) + assert "Isaac-Cartpole_physx_none_rsl_rl.pt" in message + assert str(tmp_path / ".pretrained_checkpoints" / "rsl_rl") in message + assert "Permission denied" in message + assert error.value.__cause__ is cause diff --git a/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip b/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip new file mode 100644 index 000000000000..a882c8161446 --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/test-scene-partitioning-perspective-golden.skip @@ -0,0 +1 @@ +Test-only visual regression coverage; no user-facing changelog entry. diff --git a/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png b/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png new file mode 100644 index 000000000000..aa2e5f9b1ea4 Binary files /dev/null and b/source/isaaclab_visualizers/test/golden_images/cartpole_all_envs/physx-kit-viewport.png differ diff --git a/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py b/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py index e23eeddb0cec..7dfe5fb254a3 100644 --- a/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py +++ b/source/isaaclab_visualizers/test/test_visualizer_golden_physx.py @@ -9,6 +9,8 @@ viewport and tiled-camera capture modes for the cartpole, shadow hand, and AnymalD environments. Golden images are stored under ``golden_images/{scene}/{physics_backend}-{visualizer_type}-{mode}.png``. +The Kit perspective-camera coverage also includes one four-environment cartpole +view to guard the all-partitions spectator path. On the first run for a new combination the current frame is saved as the golden; the test fails so the file can be reviewed before committing. @@ -89,6 +91,17 @@ def _run_anymal_d_physx( ] +def test_kit_perspective_all_envs_golden() -> None: + """The Kit perspective camera renders all four scene-partitioned environments.""" + _golden.run_visualizer_golden_cartpole( + _PHYSICS_BACKEND, + "kit", + "viewport", + _COMPARISON_SCORES, + all_envs_perspective=True, + ) + + @pytest.mark.parametrize("scene,visualizer_type,mode", SCENE_VISUALIZER_MODE_COMBINATIONS) def test_visualizer_golden(scene: str, visualizer_type: str, mode: str) -> None: """Golden image correctness for PhysX across scenes, visualizer types, and modes.""" diff --git a/source/isaaclab_visualizers/test/visualizer_golden_utils.py b/source/isaaclab_visualizers/test/visualizer_golden_utils.py index e7cc7e70a420..871f0198cbc7 100644 --- a/source/isaaclab_visualizers/test/visualizer_golden_utils.py +++ b/source/isaaclab_visualizers/test/visualizer_golden_utils.py @@ -359,6 +359,7 @@ def run_visualizer_golden_cartpole( comparison_scores: list[dict], *, buffer_steps: int = 0, + all_envs_perspective: bool = False, ) -> None: """Run a golden-image test for one ``(physics_backend, visualizer_type, mode)`` combination. @@ -372,7 +373,11 @@ def run_visualizer_golden_cartpole( comparison_scores: Module-level accumulator forwarded to :func:`validate_visualizer_frame`. buffer_steps: Physics steps to run before capture (default 0 — capture the reset pose so the pole remains at its initial 45° angle and is clearly attached to the cart). + all_envs_perspective: Whether to frame four environments in one Kit perspective-camera image. """ + if all_envs_perspective and (visualizer_type != "kit" or mode != "viewport"): + raise ValueError("The all-environment perspective golden requires Kit viewport capture.") + import torch import visualizer_integration_utils as _viz_utils @@ -396,7 +401,12 @@ def _capture_frame(env, viz_type: str, capture_mode: str, backend: str, actions: _viz_utils._prepare_visualizer_test_process() sim_utils.create_new_stage() tiled = mode == "tiled" - env = _viz_utils._make_cartpole_camera_env(visualizer_type, physics_backend, tiled_camera=tiled) + env = _viz_utils._make_cartpole_camera_env( + visualizer_type, + physics_backend, + tiled_camera=tiled, + all_envs_perspective=all_envs_perspective, + ) _viz_utils._configure_sim_for_visualizer_test(env) actions = torch.zeros((env.num_envs, env.action_space.shape[-1]), device=env.device) # Pin the initial pole angle to a fixed value so the golden image shows a clearly @@ -417,8 +427,11 @@ def _capture_frame(env, viz_type: str, capture_mode: str, backend: str, actions: env.step(action=actions) frame = _capture_frame(env, visualizer_type, mode, physics_backend, actions) + if all_envs_perspective: + _viz_utils._assert_non_flat_frame_array(frame) - validate_visualizer_frame("cartpole", physics_backend, visualizer_type, mode, frame, comparison_scores) + test_name = "cartpole_all_envs" if all_envs_perspective else "cartpole" + validate_visualizer_frame(test_name, physics_backend, visualizer_type, mode, frame, comparison_scores) finally: _viz_utils._cleanup_visualizer_test_process(env) diff --git a/source/isaaclab_visualizers/test/visualizer_integration_utils.py b/source/isaaclab_visualizers/test/visualizer_integration_utils.py index c3dc7cd086a4..929a95e47a92 100644 --- a/source/isaaclab_visualizers/test/visualizer_integration_utils.py +++ b/source/isaaclab_visualizers/test/visualizer_integration_utils.py @@ -69,12 +69,24 @@ _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS = 4 """Vectorized env count for generated visualizer tiled-camera integration tests.""" +_CARTPOLE_ALL_ENVS_INTEGRATION_NUM_ENVS = 4 +"""Vectorized env count for the all-environment perspective-camera golden test.""" + +_CARTPOLE_ALL_ENVS_INTEGRATION_SPACING = 8.0 +"""Environment spacing [m] for a clear four-cartpole perspective view.""" + _CARTPOLE_INTEGRATION_VISUALIZER_EYE: tuple[float, float, float] = (2.25, 0.0, 3.5) """Passed to :class:`~isaaclab.visualizers.visualizer_cfg.VisualizerCfg` subclasses (``eye``).""" _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT: tuple[float, float, float] = (0.0, 0.0, 2.25) """Passed to visualizer cfgs (``lookat``); also applied to :class:`~isaaclab.envs.common.ViewerCfg` for the env.""" +_CARTPOLE_ALL_ENVS_VISUALIZER_EYE: tuple[float, float, float] = (9.0, 9.0, 10.0) +"""Perspective-camera eye position that frames all four cartpole environments.""" + +_CARTPOLE_ALL_ENVS_VISUALIZER_LOOKAT: tuple[float, float, float] = (0.0, 0.0, 2.25) +"""Perspective-camera target at the center of the four-environment grid.""" + _CARTPOLE_INTEGRATION_TILED_CAMERA_EYE_OFFSET: tuple[float, float, float] = tuple( eye - lookat for eye, lookat in zip(_CARTPOLE_INTEGRATION_VISUALIZER_EYE, _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT) ) @@ -307,17 +319,26 @@ def _allocate_rerun_test_ports(host: str = "127.0.0.1") -> tuple[int, int]: return web_port, grpc_port -def _cartpole_integration_visualizer_camera_kwargs() -> dict[str, tuple[float, float, float]]: +def _cartpole_integration_visualizer_camera_kwargs( + *, all_envs_perspective: bool = False +) -> dict[str, tuple[float, float, float]]: """Eye/lookat for all :class:`~isaaclab.visualizers.visualizer_cfg.VisualizerCfg` subclasses in these tests.""" + if all_envs_perspective: + return { + "eye": _CARTPOLE_ALL_ENVS_VISUALIZER_EYE, + "lookat": _CARTPOLE_ALL_ENVS_VISUALIZER_LOOKAT, + } return { "eye": _CARTPOLE_INTEGRATION_VISUALIZER_EYE, "lookat": _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT, } -def _get_visualizer_cfg(visualizer_kind: str, *, tiled_camera: bool = False): +def _get_visualizer_cfg(visualizer_kind: str, *, tiled_camera: bool = False, all_envs_perspective: bool = False): """Return (visualizer_cfg, expected_visualizer_cls) for the given visualizer kind.""" - cam = _cartpole_integration_visualizer_camera_kwargs() + if tiled_camera and all_envs_perspective: + raise ValueError("Tiled-camera and all-environment perspective modes are mutually exclusive.") + cam = _cartpole_integration_visualizer_camera_kwargs(all_envs_perspective=all_envs_perspective) tiled_cam = ( { "streaming_view": True, @@ -1815,16 +1836,29 @@ def _make_franka_cloth_env(visualizer_kind: str | tuple[str, ...], *, tiled_came def _make_cartpole_camera_env( - visualizer_kind: str | tuple[str, ...], backend_kind: str, *, tiled_camera: bool = False + visualizer_kind: str | tuple[str, ...], + backend_kind: str, + *, + tiled_camera: bool = False, + all_envs_perspective: bool = False, ) -> CartpoleCameraEnv: """Create cartpole camera env configured with selected visualizer and physics backend.""" + if tiled_camera and all_envs_perspective: + raise ValueError("Tiled-camera and all-environment perspective modes are mutually exclusive.") physics = "newton_mjwarp" if backend_kind == "newton" else "physx" env_cfg = _compose_task_cfg("Isaac-Cartpole-Camera-Direct", physics, "renderer=isaacsim_rtx") env_cfg.scene.num_envs = ( - _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS if tiled_camera else _CARTPOLE_INTEGRATION_NUM_ENVS + _CARTPOLE_TILED_CAMERA_INTEGRATION_NUM_ENVS + if tiled_camera + else _CARTPOLE_ALL_ENVS_INTEGRATION_NUM_ENVS + if all_envs_perspective + else _CARTPOLE_INTEGRATION_NUM_ENVS ) - env_cfg.viewer.eye = _CARTPOLE_INTEGRATION_VISUALIZER_EYE - env_cfg.viewer.lookat = _CARTPOLE_INTEGRATION_VISUALIZER_LOOKAT + if all_envs_perspective: + env_cfg.scene.env_spacing = _CARTPOLE_ALL_ENVS_INTEGRATION_SPACING + camera_kwargs = _cartpole_integration_visualizer_camera_kwargs(all_envs_perspective=all_envs_perspective) + env_cfg.viewer.eye = camera_kwargs["eye"] + env_cfg.viewer.lookat = camera_kwargs["lookat"] tw, th = _CARTPOLE_TILED_CAMERA_INTEGRATION_WH env_cfg.tiled_camera.width = tw env_cfg.tiled_camera.height = th @@ -1832,7 +1866,14 @@ def _make_cartpole_camera_env( env_cfg.observation_space = [th, tw, env_cfg.observation_space[2]] env_cfg.seed = None visualizer_kinds = (visualizer_kind,) if isinstance(visualizer_kind, str) else tuple(visualizer_kind) - visualizer_cfgs = [_get_visualizer_cfg(kind, tiled_camera=tiled_camera)[0] for kind in visualizer_kinds] + visualizer_cfgs = [ + _get_visualizer_cfg( + kind, + tiled_camera=tiled_camera, + all_envs_perspective=all_envs_perspective, + )[0] + for kind in visualizer_kinds + ] env_cfg.sim.visualizer_cfgs = visualizer_cfgs[0] if len(visualizer_cfgs) == 1 else visualizer_cfgs return CartpoleCameraEnv(env_cfg)