[Workflow] Fix zero agent and random agent to launch newton visualizer by default - #7518
Conversation
Greptile SummaryThe PR changes only the zero agent’s default visualizer from Kit to Newton GL, avoiding an unnecessary Isaac Sim runtime for Newton workflows while preserving explicit overrides and the random-agent default.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or compatibility issues identified. The policy-dependent default is correctly parsed, explicit visualizer selections remain authoritative, and the visualizer implementation includes a non-Newton backend path. Important Files Changed
Reviews (1): Last reviewed commit: "Default zero agent to Newton visualizer" | Re-trigger Greptile |
|
run-ci |
There was a problem hiding this comment.
Isaac Lab Review Bot
The shared parser changes the default visualizer from kit to newton_gl for both zero and random agents, despite the PR’s stated intent to preserve random-agent behavior. The test and changelog currently encode this unintended broader API change.
- Design and architecture: Using
_parse_argsto set the CLI default is appropriate, but the unconditional default applies to every policy handled by the shared parser. The existingpolicyargument should distinguish the zero-agent default unless the broader behavior change is explicitly intended and documented. - API: The zero-agent default changes as intended and explicit
--vizoverrides remain available. However,random_agentalso changes its user-facing default, contradicting the stated compatibility scope and the requirement to preserve unaffected behavior. - Implementation: The implementation is small and directly tested, but the parametrized test asserts the unintended random-agent change. The default should be conditional on
policy, with regression coverage for zero usingnewton_gland random retainingkit; the comment and changelog should match that behavior.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| # simple agents should open Kit visualizer by default | ||
| parser.set_defaults(visualizer=["kit"]) | ||
| # Keep checkpoint-free agents on the kitless default path. | ||
| parser.set_defaults(visualizer=["newton_gl"]) |
There was a problem hiding this comment.
🟡 Warning · Api — Shared default also changes random agent
_parse_args is shared by the zero and random policies, so this unconditional set_defaults also switches random_agent's visualizer from kit to newton_gl, contradicting the stated intent of preserving the random-agent default. Either branch on the already-available policy argument so only zero defaults to newton_gl, or make the broader change explicit in the PR description; also fix the comment, since the previous default was kit, not kitless.
|
run-ci |
|
Backported to |
…r by default (#7518) # Description The zero and random agents forced the Kit visualizer by default, which launched the Isaac Sim runtime even when a task resolved to Newton physics. This change defaults both checkpoint-free agents to the canonical Newton GL visualizer while preserving explicit --viz overrides. A regression test covers both agent defaults. An isaaclab_rl changelog fragment documents how to retain the Kit visualizer explicitly. Fixes # N/A ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [ ] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into develop ## Screenshots N/A — workflow default change. ## Validation - uv run --frozen python -m pytest source/isaaclab_rl/test/test_entrypoints.py -q (20 passed, 2 skipped) - uv run --frozen python tools/changelog/cli.py check develop - uv run --frozen isaaclab -f - uv run --frozen isaaclab zero_agent --task Isaac-Cartpole-Direct --num_envs 1 --max_steps 32 physics=newton_mjwarp - uv run --frozen isaaclab random_agent --task Isaac-Cartpole-Direct --num_envs 1 --max_steps 32 physics=newton_mjwarp Both smoke tests initialized NewtonVisualizer and completed without launching Isaac Sim/Kit. In the headless validation shell, the Newton viewer used EGL because DISPLAY was unset. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks - [x] I have made corresponding changes to the documentation through the changelog fragment - [x] My changes generate no new warnings - [x] I have added tests that prove the fix is effective - [x] I have added a changelog fragment for every touched package - [x] My name already exists in CONTRIBUTORS.md (cherry picked from commit 94c24c0)
Description
The zero and random agents forced the Kit visualizer by default, which launched the Isaac Sim runtime even when a task resolved to Newton physics. This change defaults both checkpoint-free agents to the canonical Newton GL visualizer while preserving explicit --viz overrides.
A regression test covers both agent defaults. An isaaclab_rl changelog fragment documents how to retain the Kit visualizer explicitly.
Fixes # N/A
Type of change
Release backport
Screenshots
N/A — workflow default change.
Validation
Both smoke tests initialized NewtonVisualizer and completed without launching Isaac Sim/Kit. In the headless validation shell, the Newton viewer used EGL because DISPLAY was unset.
Checklist