Make automatic PhysX defaults concrete and fix install hints - #6849
Make automatic PhysX defaults concrete and fix install hints#6849kellyguo11 wants to merge 11 commits into
Conversation
Make suffixless task configurations select concrete Isaac Sim PhysX while preserving the explicit automatic physx selector. Update optional backend and visualizer errors and documentation to lead with runnable uv commands.
Limit the concrete Isaac Sim PhysX default change to tasks that previously used automatic PhysX selection. Keep established Newton defaults and document the distinction.
Keep RSL-RL commands on the default uv environment while selecting extras for optional RL frameworks. Document the separate RLinf installation workflow and correct RSL-RL troubleshooting.
Greptile SummaryThe PR makes task defaults that previously used automatic PhysX selection resolve to concrete Isaac Sim PhysX while preserving explicit Newton and other backend defaults. It also corrects optional dependency guidance across runtime diagnostics and documentation.
Confidence Score: 5/5The PR appears safe to merge, with no actionable defects found in the changed default-selection or installation-guidance paths. The new task defaults consistently select their existing concrete Isaac Sim PhysX configurations, explicit automatic selection remains available, non-PhysX defaults are preserved, and the revised dependency hints correspond to declared project extras or published runtime wheels. Important Files Changed
Reviews (1): Last reviewed commit: "Correct RL framework extra guidance" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
The task preset changes consistently make isaacsim_physx concrete defaults while retaining physics=physx for automatic selection, with corresponding tests and breaking-change documentation. Two documentation issues remain: the migration recipe unexpectedly changes a PhysX example to a Newton default, and two runtime guard messages still describe physics=physx as the default.
- Design and architecture: The concrete-default/automatic-selector split matches the renderer convention and is enforced by the updated preset registry tests. However, the multi-backend migration recipe starts from a concrete PhysX configuration and then assigns
newton_mjwarpas the default, contradicting both that migration path and the generic example earlier on the page unless the Newton default is explicitly identified as intentional. - API: The user-visible default change is recorded in a major changelog fragment with migration guidance, while the existing
physx,isaacsim_physx, andovphysxselectors remain available. The affected stack and Agibot validation errors must be updated because they still tell users thatphysics=physxis the default rather than identifyingphysics=isaacsim_physxas the new concrete default. - Implementation: The preset rewiring and associated tests consistently preserve
physics=physxas the automatic selector and useisaacsim_physxfor defaults. The remaining implementation-facing issue is stale text in two backend compatibility guards; the centralized visualizer and optional-runtime install-hint paths otherwise align with the intended uv-managed workflow.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| num_substeps=1, | ||
| debug_mode=False, | ||
| ) | ||
| default: NewtonCfg = newton_mjwarp |
There was a problem hiding this comment.
🔵 Suggestion · Design Architecture — Migration recipe flips default to Newton
This "Adding Multi-Backend Support" recipe has a Before of self.sim.physics = PhysxCfg(bounce_threshold_velocity=0.2), so adding default: NewtonCfg = newton_mjwarp teaches readers that the migration silently changes their default backend to Newton. It also contradicts the generic example earlier in the same page (default: PhysxCfg = isaacsim_physx). Use isaacsim_physx here, or state explicitly that this task intentionally keeps a Newton default.
| ) | ||
| physx = PhysxAutoCfg(isaacsim_physx=isaacsim_physx) | ||
| default = physx | ||
| default = isaacsim_physx |
There was a problem hiding this comment.
🔵 Suggestion · Implementation — Guard message still calls physx the default
With default now isaacsim_physx, the surface-gripper guard message below ("Re-run this task with physics=physx (the default).") misstates the default; physx is now the opt-in automatic selector. The same stale wording exists in contrib/place/config/agibot/place_toy2box_rmp_rel_env_cfg.py. Update both messages to reference physics=isaacsim_physx.
Point Newton incompatibility diagnostics to the concrete Isaac Sim PhysX default instead of the opt-in automatic PhysX selector.
Explain that the multi-backend Reach example intentionally defaults to Newton and that migrations should preserve an established PhysX default with isaacsim_physx.
Derive the expected link velocity from the simulator-reported COM velocity after stepping. This keeps the assertion focused on the frame transformation instead of treating the commanded angular velocity as an exact post-step state.
| Tasks that previously used automatic ``PhysxAutoCfg`` selection by default now | ||
| use the concrete ``isaacsim_physx`` variant by default. Existing explicit | ||
| defaults, such as Newton, remain unchanged. The explicit ``physics=physx`` | ||
| selector opts into automatic PhysX-family selection at launch time: Isaac Sim | ||
| PhysX is used when a Kit renderer or Kit viewer is requested. For fully kit-less | ||
| runs, OvPhysX is used when the task configures an OvPhysX alternative; otherwise | ||
| selection falls back to Isaac Sim PhysX and requires Kit. This matches renderer | ||
| selection, where ``isaacsim_rtx`` is the concrete default and ``renderer=rtx`` | ||
| is automatic. |
There was a problem hiding this comment.
This is wrong, it should not list some chronology wrt the PhysxAutoCfg
Keep upstream deletions and preserve concrete PhysX defaults.
Describe current preset behavior without historical chronology.
Reconcile the PhysX supported-features documentation with upstream cable support while retaining the concrete default guidance.
Description
Make task configurations whose existing default aliased
PhysxAutoCfguse concreteisaacsim_physxinstead. Tasks with an explicit Newton or other backend default retain that default. The explicitphysics=physxselector remains automatic, so users can still request runtime-dependent selection between Isaac Sim PhysX and OvPhysX.This also fixes optional backend, visualizer, and RL framework installation guidance. Runtime errors and documentation use complete
uv run --extra ...commands only when an optional dependency is required; RSL-RL commands retain plainuv runbecause RSL-RL is included in the base environment. Manual pip fallbacks install published wheels directly instead of using invalid editable extras. The OvPhysX fallback intentionally omits a version so the diagnostic does not become stale when the runtime dependency changes.Previously, task defaults that aliased
PhysxAutoCfgcould resolve to OvPhysX when no explicit Kit signal was present, even though suffixless PhysX tasks were expected to retain Isaac Sim PhysX behavior. Separately, several failure messages suggested incompleteuv runinvocations or editable package commands that did not install the missing runtime into the active environment.No new dependencies are required.
Type of change
Screenshots
Not applicable.
Validation
uv run python -m pytest source/isaaclab_ov/test/test_ovrtx_renderer_contract.py source/isaaclab_tasks/test/contrib/stack/test_so101_stack_physics_cfg.py source/isaaclab_tasks/test/core/test_dr_legs_physics_presets.py source/isaaclab_tasks/test/core/test_preset_kit_decision.py source/isaaclab_tasks/test/core/test_reach_franka_presets.py source/isaaclab_tasks/test/core/test_runtime_compatibility.py(78 passed, 17 skipped)uv run --isolated --extra test -- make -C docs current-docsuv run python tools/update_environments_rst.py --check(127 training environments)uv run python tools/changelog/cli.py check changelog-upstream-developuv run --no-project python tools/skills/cli.py checkuv run isaaclab -fChecklist
CONTRIBUTORS.md