[Backport release/3.0.0] Centralize shared physics preset ownership - #7312
Conversation
# Description Centralize backend physics presets at task-family composition roots so robot-specific files own only genuinely robot-specific scalar configuration. Standardize Newton MJWarp velocity environments on two physics substeps at the shared family boundary. - Move the Kuka Allegro OvPhysX capacity settings into the shared lift `PhysicsCfg` and remove `KukaAllegroPhysicsCfg`. - Move the SO-101 articulation contact ordering setting into the shared stack `PhysicsCfg` and remove `SO101StackPhysicsCfg`. - Make `RoughPhysicsCfg` own the shared PhysX, MJWarp, and Kamino alternatives once. - Set the shared velocity MJWarp configuration to `num_substeps=2` and remove every robot-specific substep assignment. - Remove the nine flat-locomotion `PhysicsCfg(RoughPhysicsCfg)` wrappers. Their environment configs now mutate only robot-specific MJWarp values on the inherited `RoughPhysicsCfg` instance. - Preserve shared Newton contact, collision-pipeline, and shape settings instead of replacing the whole shared alternative in robot-specific files. - Add an `isaaclab_tasks` major changelog fragment for the shared two-substep behavior change. No additional dependencies are required. ## Type of change - Refactor: centralize physics-preset ownership. - Breaking behavior change: Newton MJWarp velocity environments now use two physics substeps. ## Validation - Existing focused lift, stack, velocity, and camera preset tests: 15 passed before the substep follow-up. - Focused velocity Newton config test after the substep follow-up: 1 passed. - Registered task preset-selection check: passed across the task registry. - Runtime configuration compatibility checks: 31 passed. - Structural audit confirms there is no `PhysicsCfg(SomeOtherPhysicsCfg)` inheritance under `isaaclab_tasks`. - Structural audit confirms no core/contrib robot-specific velocity config assigns `.num_substeps`. - Direct config audit confirms all ten affected concrete core/contrib velocity configs resolve MJWarp with two substeps. - `uv run --frozen isaaclab -f`: Ruff, formatting, and applicable source checks passed; the repository-wide changelog gate still reports pre-existing missing or modified fragments in untouched packages. - Additional preset CLI run: 26 passed; one could not complete because Isaac Sim requested interactive NVIDIA EULA acceptance, which was not accepted automatically. ## Screenshots Not applicable. ## Checklist - [x] Physics preset ownership is centralized without a compatibility layer. - [x] The shared two-substep behavior change is recorded explicitly. - [x] Existing tests cover preset construction and selection. - [x] No test files were added or changed. - [x] An `isaaclab_tasks` major changelog fragment is included. - [x] No additional documentation changes are needed. (cherry picked from commit f86b4e1)
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The preset centralization is structurally consistent, but the release notes do not cover all resulting compatibility and behavior changes. Removed module-level preset classes lack migration guidance, and the former SO-101-specific PhysX contact ordering is now applied to every stack-family environment without documentation.
- Design and architecture: Moving preset ownership to task-family roots removes duplicated configuration, but adding
solve_articulation_contact_last=Trueto the shared stackPhysicsCfgbroadens an SO-101-specific workaround to allStackEnvCfgderivatives. This family-wide dynamics change should either be scoped back to SO-101 or explicitly recorded. - API: The change removes nine module-level flat-velocity
PhysicsCfgclasses along withKukaAllegroPhysicsCfgandSO101StackPhysicsCfg. Because downstream imports can break, the major changelog fragment should identify the removed symbols, their shared-preset replacements, and migration steps, including how to retain the previous single-substep behavior. - Implementation: The per-robot MJWarp solver overrides and backend defaults are carried forward through
__post_init__mutations, while shared velocity configs establish the new two-substep behavior. The remaining implementation concern is the newly broadened stack PhysX contact-solve setting and its undocumented impact on other stack robots.
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| Changed | ||
| ^^^^^^^ | ||
|
|
||
| * **Breaking:** Changed Newton MJWarp velocity environments to use two physics substeps from their |
There was a problem hiding this comment.
🟡 Warning · Api — Changelog omits removed preset classes
This fragment covers only substeps, but the change also deletes importable PhysicsCfg classes from nine flat_env_cfg modules plus KukaAllegroPhysicsCfg and SO101StackPhysicsCfg, and makes flat velocity envs inherit the family collision/shape settings. Downstream imports of those symbols now fail with no documented replacement. List the removed classes, their shared-preset replacements, and how to restore the previous single-substep configuration.
| gpu_total_aggregate_pairs_capacity=2**21, | ||
| friction_correlation_distance=0.00625, | ||
| # Let object contacts stall position-driven grippers before they tunnel through a grasp. | ||
| solve_articulation_contact_last=True, |
There was a problem hiding this comment.
🟡 Warning · Design Architecture — SO-101 contact workaround applied family-wide
solve_articulation_contact_last was previously scoped to SO101StackPhysicsCfg with a docstring describing it as an SO-101 gripper workaround. Placing it on the shared stack PhysicsCfg changes PhysX contact solve ordering for every environment deriving from StackEnvCfg, altering grasp dynamics for other stack robots. Either keep the flag on the SO-101 config or record this family-wide default change in the changelog fragment.
Greptile SummaryThe PR centralizes stack, lift, and velocity physics presets at task-family composition roots and updates Newton visualizer goldens for the shared two-substep behavior.
Confidence Score: 4/5The PR should not merge until core lift configurations remain importable when the optional OvPhysX package is not installed. Moving the OvPhysX import into the shared lift module makes registered Franka configurations depend on a package that isaaclab_tasks does not declare, causing configuration loading to fail before backend selection. Files Needing Attention: source/isaaclab_tasks/isaaclab_tasks/core/lift/lift_env_cfg.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Task[Registered task configuration] --> Family[Shared task-family physics preset]
Family --> PhysX[Isaac Sim PhysX]
Family --> OvPhysX[OvPhysX]
Family --> MJWarp[Newton MJWarp]
Family --> Kamino[Newton Kamino]
Robot[Robot-specific flat configuration] -->|Mutates solver limits| Family
Reviews (1): Last reviewed commit: "Centralize shared physics preset ownersh..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -6,6 +6,7 @@ | |||
| from dataclasses import MISSING | |||
|
|
|||
| from isaaclab_newton.physics import MJWarpSolverCfg, NewtonCfg, NewtonCollisionPipelineCfg, NewtonShapeCfg | |||
There was a problem hiding this comment.
If isaaclab_tasks is installed without the separately packaged isaaclab_ov backend, this unconditional import fails before physics preset selection, causing every core lift configuration that imports the shared module—including Franka tasks using Newton or Isaac Sim PhysX—to raise ModuleNotFoundError during loading.
Knowledge Base Used:
3 participants
Description
Backports #7301 to
release/3.0.0by cherry-picking the canonical merged commitf86b4e1ad66e637e7770a727e181ffe150a2077b.This centralizes backend physics presets at the task-family composition roots:
The cherry-pick applied without conflicts, and all 21 affected files are byte-for-byte identical to the merged #7301 versions.
Validation
uv run --extra test --frozen python -m pytest -q source/isaaclab_tasks/test/core/test_velocity_newton_cfg.py source/isaaclab_tasks/test/core/test_lift_kuka_ovphysx_presets.py source/isaaclab_tasks/test/core/test_lift_env_cfg.py— 8 passed.num_substeps=2; SO-101 and Kuka inherit the intended shared physics values.PhysicsCfg(RoughPhysicsCfg),KukaAllegroPhysicsCfg, orSO101StackPhysicsCfgremains in task code; robot-specific velocity configs do not assign.num_substeps.uv run --frozen isaaclab -f— all hooks passed, including changelog and Git LFS checks.