Skip to content

[Odin] Reject Newton Warp + simple shading presets on the cartpole camera tasks - #7530

Open
AntoineRichard wants to merge 2 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/fix/cartpole-camera-newton-shading
Open

[Odin] Reject Newton Warp + simple shading presets on the cartpole camera tasks#7530
AntoineRichard wants to merge 2 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/fix/cartpole-camera-newton-shading

Conversation

@AntoineRichard

@AntoineRichard AntoineRichard commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Description

Isaac-Cartpole-Camera and Isaac-Cartpole-Camera-Direct expose the renderer and the camera
data type as two independent presets= selectors, so newton_renderer can be combined with
simple_shading_constant_diffuse, simple_shading_diffuse_mdl, or simple_shading_full_mdl.
The Newton Warp renderer does not publish those data types, so the combination is accepted at
config resolution and only fails later, in Camera._create_buffers, once the simulator has
already started:

ValueError: Renderer NewtonWarpRenderer does not support the following requested
data types: ['simple_shading_constant_diffuse']

This showed up as 72 failed rows in a benchmark sweep (dispatch 20260901-153531, image
built from release/3.0.0 at f88dbc59c82): every failure was one of the two cartpole camera
tasks with renderer=newton_renderer and one of the three simple_shading_* presets, each
burning roughly 400 s of startup before raising. The same data-type presets completed normally
on isaacsim_rtx and ovrtx (107 completed rows), so the gap is specific to the Newton Warp
backend. simple_shading_* is an Isaac RTX shading-mode setting that OVRTX aliases to
LdrColor; Newton has no analogue.

This PR adds a validate_config hook to both cartpole camera env configs, mirroring the guards
ShadowHandCameraEnvCfg and ReorientEnvCfg (in lift_env_cfg.py) already have.
configclass's validate() invokes it, so the combination now fails immediately at config
resolution with an actionable message instead of after the simulator has come up. It also means
task-discovery tooling that calls env_cfg.validate() (e.g. our benchmark harness) drops these
combinations from the matrix automatically, rather than scheduling 72 jobs that are known to fail.

Whether the Newton Warp renderer should gain simple_shading_* support is a rendering-team
decision; this PR does not prejudge it.

One note for reviewers: the allow-list here was re-derived from
NewtonWarpRenderer.supported_output_types, which publishes 10 kinds, rather than copied from
Shadow Hand. Shadow Hand's and Lift's literal sets list only 7, omitting rgba, rgb_hdr,
and albedo, so they currently false-reject e.g. presets=newton_renderer,albedo. This PR uses
the renderer's actual 10 so cartpole's albedo preset keeps working on Newton, which matches the
sweep (no cartpole albedo rows failed). Fixing the two stale sets is left out of scope here.

Type of change

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

Behaviour change: presets=newton_renderer,simple_shading_* on the two cartpole camera tasks now
raises at config resolution instead of at env construction. It never produced a working run, so
nothing that used to train stops training. Migration guidance is in the changelog fragment: use
presets=newton_renderer,rgb, or keep the shading data types on an RTX backend with
presets=isaacsim_rtx,simple_shading_full_mdl.

The generated environment browser lists renderer presets and data-type presets as flat,
independent lists and does not encode combinations (Isaac-Reorient-Cube-Shadow-Camera-Direct,
which already has this guard, still lists both), so no regeneration is needed.

Screenshots

N/A

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation (changelog fragment)
  • 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 (changelog fragment; version files not edited per repo policy)
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Test evidence

New test: source/isaaclab_tasks/test/core/test_cartpole_camera_presets.py (12 cases). It
resolves the real configs via resolve_presets and calls cfg.validate(); no simulator is
launched. Both tasks, both directions: Newton + each simple_shading_* must raise;
isaacsim_rtx/ovrtx + simple_shading_full_mdl must still resolve; newton_renderer,rgb must
still resolve.

With the fix:

$ uv run --frozen --extra test python -m pytest \
    source/isaaclab_tasks/test/core/test_cartpole_camera_presets.py -q
12 passed in 3.23s

With the two config files reverted to their develop content (guard removed, test unchanged):

6 failed, 6 passed in 3.46s
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_constant_diffuse-direct]
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_constant_diffuse-manager]
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_diffuse_mdl-direct]
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_diffuse_mdl-manager]
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_full_mdl-direct]
FAILED ...::test_newton_renderer_rejects_simple_shading[simple_shading_full_mdl-manager]
E       Failed: DID NOT RAISE ValueError

Neighbouring suites (Shadow Hand presets, env-cfg import rules) plus this one: 102 passed.
uv run --frozen isaaclab -f passes.

The GPU/Isaac Sim rendering suites (test_rendering_cartpole*.py,
test_rendering_registered_tasks.py) were not run here. Checked statically instead: their
newton_renderer parametrizations draw from _NEWTON_WARP_DATA_TYPES in
rendering_test_utils.py, which is a strict subset of the allow-list, and their
simple_shading_* cases pin isaacsim_rtx/ovrtx, so none of them hits the new guard.

The cartpole camera tasks let ``presets=`` select a renderer and a camera data
type independently, so ``newton_renderer`` could be combined with one of the
``simple_shading_*`` presets. The Newton Warp renderer does not publish those
data types, so the combination only failed in ``Camera._create_buffers`` at env
construction, minutes into a run.

Add a ``validate_config`` hook to both cartpole camera env configs, mirroring
the guard the Shadow Hand camera task already has, so the combination is
rejected during config resolution. The allow-list is shared in
``isaaclab_tasks.utils.presets`` and mirrors the keys published by
``NewtonWarpRenderer.supported_output_types``.
@AntoineRichard
AntoineRichard requested a review from a team September 3, 2026 09:38
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Sep 3, 2026

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isaac Lab Review Bot

The change adds early config validation for incompatible Newton Warp and simple_shading_* preset combinations in both cartpole camera tasks, with migration guidance and focused regression coverage.

  • Design and architecture: The shared validator and Newton Warp output allow-list are colocated with MultiBackendRendererCfg, avoiding duplicate task-level contracts while keeping the change scoped to the two affected cartpole configurations. The allow-list is checked against the renderer’s published output types.
  • API: The behavior change is limited to rejecting configurations that the Newton Warp renderer cannot construct successfully. Supported Newton outputs and the tested Isaac RTX and OVRTX shading combinations remain accepted, and the changelog documents compatible alternatives.
  • Implementation: The direct and manager-based configurations validate their respective camera paths correctly. Tests cover both tasks, incompatible shading combinations, representative supported Newton outputs, unaffected RTX paths, and allow-list drift. The contract test’s unbound renderer-method stand-in is somewhat coupled to the current implementation, but it directly verifies the duplicated renderer contract and does not warrant pre-merge action.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds early validation for incompatible Newton Warp renderer and camera data-type presets in both cartpole camera configurations.

  • Introduces a shared Newton Warp output-type allow-list and validation helper.
  • Hooks validation into the direct and manager-based cartpole camera configurations.
  • Adds coverage for rejected simple-shading combinations, supported renderer combinations, and synchronization with the renderer contract.
  • Documents the behavior change and migration guidance.

Confidence Score: 5/5

The PR appears safe to merge, with the new validation aligned to the current renderer contract and covered across both cartpole camera configurations.

The guard recognizes the resolved Newton renderer correctly, matches its published output types, leaves RTX configurations unaffected, and does not omit inherited validation.

Important Files Changed

Filename Overview
source/isaaclab_tasks/isaaclab_tasks/utils/presets.py Adds a shared guard whose renderer identifier and supported output set match the current Newton Warp renderer contract.
source/isaaclab_tasks/isaaclab_tasks/core/cartpole/cartpole_direct_camera_env_cfg.py Adds config-resolution validation for the direct cartpole camera without suppressing inherited validation.
source/isaaclab_tasks/isaaclab_tasks/core/cartpole/cartpole_manager_camera_env_cfg.py Adds the equivalent config-resolution validation for the manager-based cartpole camera.
source/isaaclab_tasks/test/core/test_cartpole_camera_presets.py Covers incompatible and supported preset combinations and pins the shared allow-list to the renderer’s published outputs.
source/isaaclab_tasks/changelog.d/cartpole-camera-warp-shading-guard.rst Accurately documents the corrected failure timing and compatible alternatives.

Reviews (1): Last reviewed commit: "Reject Newton Warp + simple shading on t..." | Re-trigger Greptile

Drop the new public helper and allow-list from isaaclab_tasks.utils.presets and
inline the check in each cartpole camera config, matching the existing inline
guards in the Shadow Hand and Lift configs rather than introducing a third
pattern. Trim the regression test to the contract it proves and tighten the
changelog fragment.
@AntoineRichard AntoineRichard changed the title Reject Newton Warp + simple shading presets on the cartpole camera tasks [Odin] Reject Newton Warp + simple shading presets on the cartpole camera tasks Sep 3, 2026
@kellyguo11

Copy link
Copy Markdown
Contributor

The early failure is useful, but the renderer capability contract should not be copied into each task config. NewtonWarpRenderer.supported_output_types() is already the source of truth, and the existing Shadow Hand/Lift lists have demonstrably drifted. Could we expose a config-time capability query through the resolved renderer class and make camera/renderer compatibility validation generic? The runtime Camera._create_buffers() check should remain as a defensive guard.

This comment was AI-generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants