From a1b7e56747a79ec95bafd982ab6e658547b5304d Mon Sep 17 00:00:00 2001 From: Maximilian Krause <99733341+maxkra15@users.noreply.github.com> Date: Wed, 30 Sep 2026 03:04:36 +0200 Subject: [PATCH 1/3] Remove unsupported Franka Reorient task (#8168) ## Description Remove the unsupported `Isaac-Reorient-Franka` task registration, configuration, example, and browser entry. This is the requested breaking removal; downstream users who still need it must keep a local copy or migrate to a maintained reorientation task. This is part 1 of the split of #7829 and is independently reviewable. ## Validation - `test_lift_env_cfg.py`: 12 passed - `uv run --frozen isaaclab -f`: passed - Generated environment browser and changelog checks: passed ## Release backport - [x] Backport this pull request to the active release branch after it merges into `develop` --- docs/source/_static/css/environment-browser.js | 6 ++---- examples/heterogeneous_scene.py | 1 - .../maximiliank-remove-franka-reorient.major.rst | 5 +++++ .../core/lift/config/franka/__init__.py | 13 +------------ .../core/lift/config/franka/franka_env_cfg.py | 15 ++------------- .../isaaclab_tasks/test/core/test_lift_env_cfg.py | 9 ++++----- 6 files changed, 14 insertions(+), 35 deletions(-) create mode 100644 source/isaaclab_tasks/changelog.d/maximiliank-remove-franka-reorient.major.rst diff --git a/docs/source/_static/css/environment-browser.js b/docs/source/_static/css/environment-browser.js index dadfd2dca455..1bd8f03505b5 100644 --- a/docs/source/_static/css/environment-browser.js +++ b/docs/source/_static/css/environment-browser.js @@ -36,14 +36,13 @@ ["Isaac-Reach-Franka", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "diffik,diffik_abs,joint_pos,newton_ik", "tasks/manipulation/franka_reach.jpg", true, {"*": ["joint_pos"]}], ["Isaac-Reach-Franka-OSC", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "diffik_abs", "tasks/manipulation/franka_reach.jpg", false, {"*": ["diffik_abs"]}], ["Isaac-Reach-UR10", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "", "tasks/manipulation/ur10_reach.jpg", true], - ["Isaac-RenderBenchmark-Franka-Cabinet", "", "isaacsim_physx,newton_mjwarp", "isaacsim_rtx,newton_renderer,ovrtx", "albedo,depth,rgb,simple_shading_constant_diffuse,simple_shading_diffuse_mdl,simple_shading_full_mdl", "", false, {"*": ["rgb"]}], + ["Isaac-RenderBenchmark-Franka-Cabinet", "", "isaacsim_physx,newton_mjwarp,ovphysx", "isaacsim_rtx,newton_renderer,ovrtx", "albedo,depth,rgb,simple_shading_constant_diffuse,simple_shading_diffuse_mdl,simple_shading_full_mdl", "", false, {"*": ["rgb"]}], ["Isaac-Reorient-Cube-Allegro-Direct", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "", "tasks/manipulation/allegro_cube.jpg", true], ["Isaac-Reorient-Cube-Allegro", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "randomized,reset_only", "tasks/manipulation/allegro_cube.jpg", false, {"*": ["reset_only"]}], ["Isaac-Reorient-Cube-Shadow-Direct", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "", "tasks/manipulation/shadow_cube.jpg"], ["Isaac-Reorient-Cube-Shadow", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "asymmetric,randomized"], ["Isaac-Reorient-Cube-Shadow-Camera-Direct", "rl_games,rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "isaacsim_rtx,newton_renderer,ovrtx", "albedo,depth,full,rgb,semantic_segmentation,simple_shading_constant_diffuse,simple_shading_diffuse_mdl,simple_shading_full_mdl", "tasks/manipulation/shadow_cube.jpg", false, {"*": ["full"]}], ["Isaac-Reorient-Cube-Shadow-Camera", "rl_games,rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "isaacsim_rtx,newton_renderer,ovrtx", "albedo,depth,full,randomized,rgb,semantic_segmentation,simple_shading_constant_diffuse,simple_shading_diffuse_mdl,simple_shading_full_mdl", "", false, {"*": ["full"]}], - ["Isaac-Reorient-Franka", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "cube,shapes", "", false, {"*": ["shapes"]}], ["Isaac-Reorient-KukaAllegro", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "cube,shapes", "tasks/manipulation/kuka_allegro_reorient.jpg", false, {"*": ["shapes"]}], ["Isaac-Reorient-KukaAllegro-Camera", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "isaacsim_rtx,newton_renderer,ovrtx", "albedo128,albedo256,albedo64,cube,depth128,depth256,depth64,duo_camera,raycaster_depth128,raycaster_depth256,raycaster_depth64,rgb128,rgb256,rgb64,semantic_segmentation128,semantic_segmentation256,semantic_segmentation64,shapes,simple_shading_constant_diffuse128,simple_shading_constant_diffuse256,simple_shading_constant_diffuse64,simple_shading_diffuse_mdl128,simple_shading_diffuse_mdl256,simple_shading_diffuse_mdl64,simple_shading_full_mdl128,simple_shading_full_mdl256,simple_shading_full_mdl64,single_camera", "tasks/manipulation/kuka_allegro_reorient.jpg", false, {"*": ["rgb64", "shapes", "single_camera"]}], ["Isaac-Shadow-Handover-Direct", "rl_games,rsl_rl,skrl", "isaacsim_physx,newton_mjwarp,ovphysx", "", "", "tasks/manipulation/shadow_hand_over.jpg", false, {}, {"skrl": "MAPPO"}], @@ -99,7 +98,7 @@ ["IsaacContrib-Open-Drawer-Franka-IK-Abs", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "", ""], ["IsaacContrib-Open-Drawer-Franka-IK-Rel", "rsl_rl", "isaacsim_physx,newton_mjwarp,ovphysx", "", ""], ["IsaacContrib-Open-Drawer-OpenArm", "rl_games,rsl_rl", "", "", "", "tasks/manipulation/openarm_uni_open_drawer.jpg"], - ["IsaacContrib-PickPlace-FixedBaseUpperBodyIK-G1-Abs", "", "", "", "", "tasks/manipulation/g1_pick_place_fixed_base.jpg"], + ["IsaacContrib-PickPlace-FixedBaseUpperBodyIK-G1-Abs", "", "", "isaacsim_rtx,newton_renderer,ovrtx", "", "tasks/manipulation/g1_pick_place_fixed_base.jpg"], ["IsaacContrib-PickPlace-G1-InspireFTP-Abs", "", "", "", "", "tasks/manipulation/g1_pick_place.jpg"], ["IsaacContrib-PickPlace-GR1T2-Abs", "", "", "isaacsim_rtx,newton_renderer,ovrtx", "", "tasks/manipulation/gr-1_pick_place.jpg"], ["IsaacContrib-PickPlace-GR1T2-WaistEnabled-Abs", "", "", "", "", "tasks/manipulation/gr-1_pick_place_waist.jpg"], @@ -336,7 +335,6 @@ [/Reach-UR10/, "tasks/manipulation/ur10_reach.jpg"], [/Reorient-Cube-Allegro/, "tasks/manipulation/allegro_cube.jpg"], [/Reorient-Cube-Shadow/, "tasks/manipulation/shadow_cube.jpg"], - [/Reorient-Franka/, "tasks/manipulation/franka_lift.jpg"], [/Reorient-KukaAllegro/, "tasks/manipulation/kuka_allegro_reorient.jpg"], [/Shadow-Handover/, "tasks/manipulation/shadow_hand_over.jpg"], [/AnymalB/, "tasks/locomotion/anymal_b_flat.jpg"], diff --git a/examples/heterogeneous_scene.py b/examples/heterogeneous_scene.py index b7ec74d2f509..a4d8a334fe47 100644 --- a/examples/heterogeneous_scene.py +++ b/examples/heterogeneous_scene.py @@ -95,7 +95,6 @@ "IsaacContrib-Navigation-Flat-AnymalC", # arm and hand manipulation "Isaac-Lift-Franka", - "Isaac-Reorient-Franka", "Isaac-Lift-KukaAllegro", "Isaac-Reorient-KukaAllegro", "Isaac-Open-Drawer-Franka", diff --git a/source/isaaclab_tasks/changelog.d/maximiliank-remove-franka-reorient.major.rst b/source/isaaclab_tasks/changelog.d/maximiliank-remove-franka-reorient.major.rst new file mode 100644 index 000000000000..04a0cd995c0e --- /dev/null +++ b/source/isaaclab_tasks/changelog.d/maximiliank-remove-franka-reorient.major.rst @@ -0,0 +1,5 @@ +Removed +^^^^^^^ + +* Removed the unsupported ``Isaac-Reorient-Franka`` task and its environment configuration. Migrate custom + reorientation experiments to a maintained task or keep a local copy of the old configuration. diff --git a/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/__init__.py b/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/__init__.py index 6b677df0f69b..ce0db54e0196 100644 --- a/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/__init__.py +++ b/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/__init__.py @@ -3,7 +3,7 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Lift Franka reorient and lift environments.""" +"""Franka lift environment.""" import gymnasium as gym @@ -13,17 +13,6 @@ # Register Gym environments. ## -gym.register( - id="Isaac-Reorient-Franka", - entry_point="isaaclab.envs:ManagerBasedRLEnv", - disable_env_checker=True, - kwargs={ - "env_cfg_entry_point": f"{__name__}.franka_env_cfg:FrankaReorientEnvCfg", - "rsl_rl_cfg_entry_point": f"{agents.__name__}.rsl_rl_ppo_cfg:FrankaPPORunnerCfg", - "default_agent": "rsl_rl", - }, -) - gym.register( id="Isaac-Lift-Franka", entry_point="isaaclab.envs:ManagerBasedRLEnv", diff --git a/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/franka_env_cfg.py b/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/franka_env_cfg.py index ceaa21249b77..2d136cd8c31d 100644 --- a/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/franka_env_cfg.py +++ b/source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka/franka_env_cfg.py @@ -3,7 +3,7 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Configuration for the Franka lift and reorient environments.""" +"""Configuration for the Franka lift environment.""" from isaaclab.actuators import ImplicitActuatorCfg from isaaclab.assets import ArticulationCfg @@ -96,7 +96,7 @@ @configclass class FrankaSceneCfg(lift.SceneCfg): - """Franka scene for the lift and reorient tasks.""" + """Franka scene for the lift task.""" robot: ArticulationCfg = replace(FRANKA_PANDA_LIFT_CFG, prim_path="{ENV_REGEX_NS}/Robot") @@ -259,17 +259,6 @@ def __post_init__(self): self.terminations.abnormal_robot.params["asset_cfg"] = SceneEntityCfg("robot", joint_names="panda_joint.*") -@configclass -class FrankaReorientEnvCfg(FrankaMixinCfg, lift.ReorientEnvCfg): - """Franka object reorientation environment.""" - - def play_mode(self): - super().play_mode() - # evaluate at the datasheet gripper speed: without the closing-speed randomization the hand - # damping caps closing at the real hand's jaw-speed limit of 0.2 m/s - self.events.gripper_closing_speed = None - - @configclass class FrankaLiftEnvCfg(FrankaMixinCfg, lift.LiftEnvCfg): """Franka object lifting environment.""" diff --git a/source/isaaclab_tasks/test/core/test_lift_env_cfg.py b/source/isaaclab_tasks/test/core/test_lift_env_cfg.py index 6ea417797ae9..05f206cf6aa8 100644 --- a/source/isaaclab_tasks/test/core/test_lift_env_cfg.py +++ b/source/isaaclab_tasks/test/core/test_lift_env_cfg.py @@ -3,7 +3,7 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Behavioral tests for the unified dexterous Lift and Reorient tasks.""" +"""Behavioral tests for the dexterous Lift tasks.""" from types import SimpleNamespace @@ -18,7 +18,7 @@ from isaaclab.utils.warp import ProxyArray from isaaclab_tasks.core.lift import mdp -from isaaclab_tasks.core.lift.config.franka.franka_env_cfg import FrankaLiftEnvCfg, FrankaReorientEnvCfg +from isaaclab_tasks.core.lift.config.franka.franka_env_cfg import FrankaLiftEnvCfg from isaaclab_tasks.core.lift.config.franka_soft.franka_soft_env_cfg import FrankaSoftEnvCfg from isaaclab_tasks.core.lift.mdp.commands import pose_commands from isaaclab_tasks.core.lift.mdp.commands.pose_commands import ( @@ -65,10 +65,9 @@ def test_franka_soft_robot_physics_variant_matches_backend( assert cfg.scene.robot.spawn.variants == {"Physics": expected_physics} -@pytest.mark.parametrize("cfg_type", [FrankaLiftEnvCfg, FrankaReorientEnvCfg]) -def test_franka_rigid_tasks_select_collision_meshes_for_reset_clearance(cfg_type) -> None: +def test_franka_rigid_task_selects_collision_meshes_for_reset_clearance() -> None: """Reset validation keeps the original arm meshes when the asset defaults to capsules.""" - cfg = cfg_type() + cfg = FrankaLiftEnvCfg() stage = Usd.Stage.CreateInMemory() robot = stage.DefinePrim("/Robot", "Xform") colliders = robot.GetVariantSets().AddVariantSet("Colliders") From cb796d9df52717e398a9dc6dacd5a9e7750e759e Mon Sep 17 00:00:00 2001 From: Mustafa H <34825877+StafaH@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:08:33 -0700 Subject: [PATCH 2/3] [CI] Remove Kit-dependent performance smoke benchmarks (#8173) ## Description Kit-dependent combinations repeatedly fail the performance smoke job before producing usable measurements. Remove those six combinations and retain four Kit-free runs: Newton Cartpole, Newton G1, and Shadow Hand camera benchmarks using the Newton renderer with Newton or OvPhysX physics. Remove obsolete task thresholds, disabled hard-floor entries, Kit launch environment variables, and an unused step ID. Simplify status handling and reduce the job timeout to 450 minutes, accounting for three samples and one retry per sample. The retained combinations keep identical resolved thresholds and baseline contracts; Cartpole remains advisory-only. ## Type of change - Bug fix (CI configuration) ## Validation - All 8 performance comparison tests passed. - Repository formatting and lint checks passed against the PR base. - Benchmark shell steps passed `bash -n`. - Confirmed the four retained combinations resolve to the same thresholds before and after cleanup. - Full GPU benchmark execution remains for CI. ## Release backport - [ ] Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the contribution guidelines. - [x] I have run the pre-commit checks with `uv run isaaclab --format`. - [x] I have validated the changed behavior with focused coverage; no tests were changed. - [x] My name already exists in `CONTRIBUTORS.md`. - Documentation and package changelog fragments are not applicable: only CI workflow and tooling policy changed. --- .github/workflows/build.yaml | 25 +++++++++--------------- tools/perf_smoke_thresholds.json | 33 ++------------------------------ 2 files changed, 11 insertions(+), 47 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 971801ba80f5..03d84b31059d 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -969,9 +969,8 @@ jobs: performance-smoke: name: performance-smoke-benchmarks runs-on: [self-hosted, gpu] - # Ceiling is the sum of each leg's own bench_timeout_s x 3 + buffer - # Individual attempts are still bound tightly by `timeout` inside the loop. - timeout-minutes: 510 + # Four legs x three samples, including one retry each: 372 minutes + setup/comparison buffer. + timeout-minutes: 450 needs: [build, config] # Pull requests and manual dispatches run in gate mode. A push runs only on develop, # where it also records a baseline; pushes to main and release/** skip this job. @@ -991,19 +990,13 @@ jobs: dockerfile-path: docker/Dockerfile.base cache-tag: cache-base - # One row per combination: name|task|num_envs|bench_timeout_s|args + # Kit-free combinations only: name|task|num_envs|bench_timeout_s|args - name: Define benchmark matrix run: | cat > benchmark-legs.tsv <<'EOF' - cartpole-physx|Isaac-Cartpole-Direct|4096|600| cartpole-newton|Isaac-Cartpole-Direct|4096|600|physics=newton_mjwarp - gear-mesh-physx|IsaacContrib-Factory-GearMesh-Direct|512|900| - shadow-camera-physx-rtx|IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct|512|1200|renderer=isaacsim_rtx - shadow-camera-physx-newton-renderer|IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct|512|1200|renderer=newton_renderer - shadow-camera-newton-rtx|IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct|512|1200|physics=newton_mjwarp renderer=isaacsim_rtx shadow-camera-newton-renderer|IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct|512|1200|physics=newton_mjwarp renderer=newton_renderer shadow-camera-ovphysx-newton|IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct|512|1200|physics=ovphysx renderer=newton_renderer - g1-physx|Isaac-Velocity-Flat-G1|512|720| g1-newton|Isaac-Velocity-Flat-G1|512|720|physics=newton_mjwarp EOF @@ -1027,7 +1020,6 @@ jobs: perf-jit-warp${{ steps.warp_version.outputs.value }}- - name: Run runtime benchmarks - id: runtime_benchmark env: CI_IMAGE_TAG: ${{ needs.config.outputs.ci_image_tag }} run: | @@ -1049,9 +1041,6 @@ jobs: container="performance-smoke-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}-${leg}-${sample}-${attempt}" cleanup_container timeout "${bench_timeout_s}" docker run --name "$container" --gpus all --network=host --ipc=host \ - -e OMNI_KIT_ACCEPT_EULA=yes \ - -e ACCEPT_EULA=Y \ - -e ISAAC_SIM_HEADLESS=1 \ -e WARP_CACHE_PATH=/tmp/jit-cache/warp \ -e CUDA_CACHE_PATH=/tmp/jit-cache/nv \ -v "${GITHUB_WORKSPACE}/jit-cache:/tmp/jit-cache" \ @@ -1100,8 +1089,12 @@ jobs: fi fi done - echo "$([ "$leg_ok" = true ] && echo ok || echo failed)" > "benchmark-output/${leg}/status" - [ "$leg_ok" = true ] || failed_legs+=("$leg") + if [ "$leg_ok" = true ]; then + echo ok > "benchmark-output/${leg}/status" + else + echo failed > "benchmark-output/${leg}/status" + failed_legs+=("$leg") + fi echo "::endgroup::" done < benchmark-legs.tsv diff --git a/tools/perf_smoke_thresholds.json b/tools/perf_smoke_thresholds.json index e3d07e53c39b..c9a8b287c1dc 100644 --- a/tools/perf_smoke_thresholds.json +++ b/tools/perf_smoke_thresholds.json @@ -1,5 +1,5 @@ { - "_comment": "Perf-smoke gate POLICY. Thresholds live here, in git, so changing what fails a PR is reviewable and has history. Measured reference values (median, spread, sample count) live in the baseline store instead, because those are measurements and should not require a PR to update. Keys are gpu_model (canonical slug) -> task_id -> backend_key, where backend_key is '{physics}' or '{physics}_{render}'. hard_floor_fps is a catastrophic-failure detector, NOT a regression threshold: it should sit far below any plausible healthy value so it only fires when a run is fundamentally broken. Regression percentages are compared against the baseline median.", + "_comment": "Performance smoke policy. Relative thresholds are compared against baseline history. No hard FPS floors are calibrated for the Kit-free matrix.", "schema_version": 1, "defaults": { "warn_regression_pct": 5.0, @@ -7,39 +7,10 @@ "_todo": "Both defaults are placeholders. Set them from measured run-to-run spread on the real L40S runners, not from the desktop RTX 5090 sweep in temp/matrix_run (that host showed 13% CV on Cartpole, which is likely runner-specific)." }, "per_task_regression_pct": { - "_comment": "Overrides for tasks whose run-to-run noise makes the default threshold unusable. A threshold below the noise floor fires on noise; a threshold far above it never fires at all. Measured by benchmarking six real develop commits inside one pin window, sequentially, on one machine (temp/threshold_probe).", - "IsaacContrib-Factory-GearMesh-Direct": { - "_comment": "Commit-to-commit CV 2.3% across six develop commits (1194-1277 fps), against 1.18% pure run-to-run CV. Real drift is small and the default 5%/10% is defensible here. This is the sensitive probe: use it, not Cartpole, to detect small regressions." - }, + "_comment": "Overrides for tasks whose run-to-run noise makes the default threshold unusable.", "Isaac-Cartpole-Direct": { "advisory_only": true, "_comment": "Keep Cartpole advisory-only while spread is confirmed on runners." } - }, - "hard_floor_fps": { - "l40s": { - "Isaac-Cartpole-Direct": { - "physx": 100.0, - "newton_mjwarp": 0.0, - "_todo": "newton floor is 0.0 (disabled). RTX 5090 sweep measured ~887,748 fps mean. Pick a floor ~an order of magnitude below the L40S healthy value so it catches a broken run without tracking performance." - }, - "IsaacContrib-Factory-GearMesh-Direct": { - "physx": 30.0 - }, - "IsaacContrib-Reorient-Cube-Shadow-Camera-Benchmark-Direct": { - "physx_isaacsim_rtx": 20.0, - "physx_newton": 0.0, - "newton_mjwarp_isaacsim_rtx": 0.0, - "newton_mjwarp_newton": 0.0, - "ovphysx_newton": 0.0, - "_todo": "Four of five floors are 0.0 (disabled)." - }, - "Isaac-Velocity-Flat-G1": { - "physx": 40.0, - "newton_mjwarp": 0.0, - "_todo": "newton floor is 0.0 (disabled). RTX 5090 sweep measured ~26,936 fps mean." - } - }, - "_comment": "Keys are gpu_model, then task, then backend key. The backend key is the physics backend, or physics_render when a renderer is in use, spelled exactly as the benchmark reports it: physx, newton_mjwarp, physx_isaacsim_rtx, newton_mjwarp_newton. A key that matches nothing never applies." } } From 998bf887af42c46d13207fd0a95b47f2559f9259 Mon Sep 17 00:00:00 2001 From: yusuf Date: Tue, 29 Sep 2026 21:47:15 -0400 Subject: [PATCH 3/3] Match Newton rigid object collection bodies exactly (#8175) # Description The Newton `RigidObjectCollection` merged its members' prim path expressions into one wildcard glob (`_build_combined_pattern`), because older Newton could not accept a list of patterns. That approximation has two failures: - It over-matches rigid bodies with similar names. With members `Object_A` and `Object_B`, the glob `/World/Env_*/Object_*` also selects an unrelated `Object_Target` in the same environment, and initialization fails with `RuntimeError: ... matched 3 objects per world, but the collection config contains 2`. - It cannot represent members at different hierarchy depths. `Object_A` and `Shelf/Object_B` fail with `ValueError: Cannot build combined pattern: path expressions have different segment counts`. Newton 1.6.0, the pinned version, accepts the individual patterns: `ArticulationView` matches each label against a list of globs. This PR passes the members' patterns directly, as suggested in #6147, and removes the obsolete wildcard-merging workaround. This matches the PhysX collection, which already passes the list to `create_rigid_body_view`. The per-world count check is kept. Newton matches each body once, so two members whose prim paths resolve to the same prim produce one match per world. Without the check, a 4-environment scene with such an aliased pair initializes without error, but it reports `num_instances=2` and a pose buffer of shape `(4, 1, 3)`. With the check, it raises a clear error. Fixes #6147 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Test Extended the existing Newton `test_initialization` in `source/isaaclab_newton/test/assets/test_rigid_object_collection.py`; no separate regression test or fixture was added. The cases cover a single member, an unrelated `Object_Target` alongside `Object_A/B/C`, and members at different depths (`Object_A` and `Shelf/Object_B`). All run on CPU and CUDA and check each body's name-to-pose mapping as well as view counts and buffer shapes. The two regression cases fail against the original implementation at `eb6617cda` and pass with the fix. Local collection-suite validation: | Backend | Devices | Result | | --- | --- | --- | | Newton 1.6.0 | CPU and CUDA | 22 passed | | OVPhysX 0.6.3 | CPU and CUDA | 28 passed | | Kit PhysX | CPU and CUDA | 38 passed | Separate local cross-backend checks also passed both path cases on each backend, including targeted pose writes, stepping, and reset. The consolidated Newton suite was rerun after the test cleanup; the PhysX and OVPhysX implementations were unchanged. `uv run --no-sync isaaclab -f` passed, including the changelog check. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `uv run isaaclab --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have validated the changed behavior with focused coverage and applied the [test-audit criteria](https://github.com/isaac-sim/IsaacLab/blob/develop/skills/developer/test-audit/SKILL.md) to any test changes - [x] I have added a changelog fragment under `source//changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump package versions; CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Yusuf Guenena Co-authored-by: Octi Zhang --- CONTRIBUTORS.md | 1 + .../newton-collection-path-documentation.skip | 1 + .../rigid_object_collection_cfg.py | 5 -- .../fix-newton-collection-exact-match.rst | 6 ++ .../rigid_object_collection.py | 57 ++----------------- .../assets/test_rigid_object_collection.py | 52 ++++++++++------- 6 files changed, 43 insertions(+), 79 deletions(-) create mode 100644 source/isaaclab/changelog.d/newton-collection-path-documentation.skip create mode 100644 source/isaaclab_newton/changelog.d/fix-newton-collection-exact-match.rst diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index bf37ef896e66..587be8fb6d2f 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -229,6 +229,7 @@ Guidelines for modifications: * Yuguo Shan * Yujian Zhang * Yun Liu +* Yusuf Guenena * YuTeh Shen * Zehao Wang * Zeng Qingcheng diff --git a/source/isaaclab/changelog.d/newton-collection-path-documentation.skip b/source/isaaclab/changelog.d/newton-collection-path-documentation.skip new file mode 100644 index 000000000000..ccc05452ffd4 --- /dev/null +++ b/source/isaaclab/changelog.d/newton-collection-path-documentation.skip @@ -0,0 +1 @@ +Removed the obsolete Newton collection path restriction from the configuration documentation. diff --git a/source/isaaclab/isaaclab/assets/rigid_object_collection/rigid_object_collection_cfg.py b/source/isaaclab/isaaclab/assets/rigid_object_collection/rigid_object_collection_cfg.py index 5ad3027e2745..b4805547e6f4 100644 --- a/source/isaaclab/isaaclab/assets/rigid_object_collection/rigid_object_collection_cfg.py +++ b/source/isaaclab/isaaclab/assets/rigid_object_collection/rigid_object_collection_cfg.py @@ -27,9 +27,4 @@ class RigidObjectCollectionCfg: """Dictionary of rigid object configurations to spawn. The keys are the names for the objects, which are used as unique identifiers throughout the code. - - .. note:: - - With the Newton backend, the configured root prim paths must share a combined prefix-and-suffix - pattern that does not match other rigid objects in the same world. """ diff --git a/source/isaaclab_newton/changelog.d/fix-newton-collection-exact-match.rst b/source/isaaclab_newton/changelog.d/fix-newton-collection-exact-match.rst new file mode 100644 index 000000000000..d99377614559 --- /dev/null +++ b/source/isaaclab_newton/changelog.d/fix-newton-collection-exact-match.rst @@ -0,0 +1,6 @@ +Fixed +^^^^^ + +* Fixed :class:`~isaaclab_newton.assets.RigidObjectCollection` selecting rigid bodies outside the + collection whose names share the members' prefix or suffix, and rejecting members at different + path depths. The collection view now matches each configured prim path exactly. diff --git a/source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py b/source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py index 1aa1e3ede20b..5b1cbafb9978 100644 --- a/source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py +++ b/source/isaaclab_newton/isaaclab_newton/assets/rigid_object_collection/rigid_object_collection.py @@ -5,7 +5,6 @@ from __future__ import annotations -import os import warnings from collections.abc import Sequence from typing import TYPE_CHECKING @@ -1207,22 +1206,19 @@ def has_rigid_body_api(prim) -> bool: root_prim_path_exprs.append(sim_utils.path_expr_to_glob(root_expr)) self._body_names_list.append(name) - # Build a single pattern that matches ALL body types by wildcarding the differing path segment. - combined_pattern = self._build_combined_pattern(root_prim_path_exprs) - - # Create a single ArticulationView matching all body types. + # Create a single ArticulationView matching exactly the configured bodies. # The 2nd dimension (matches per world) corresponds to the body types. self._root_view = ArticulationView( SimulationManager.get_model(), - combined_pattern, + root_prim_path_exprs, verbose=False, ) + # Newton matches each body once, so members whose prim paths alias the same prim come up short here. if self._root_view.count_per_world != self.num_bodies: raise RuntimeError( - f"Rigid object collection pattern {combined_pattern!r} matched " + f"Rigid object collection patterns {root_prim_path_exprs} matched " f"{self._root_view.count_per_world} objects per world, but the collection config contains " - f"{self.num_bodies}. Ensure collection prim paths share a prefix or suffix that distinguishes them " - "from other rigid objects." + f"{self.num_bodies}." ) # container for data access @@ -1345,49 +1341,6 @@ def _resolve_body_mask(self, body_mask: wp.array | None) -> wp.array | torch.Ten body_ids = self._ALL_BODY_INDICES return body_ids - @staticmethod - def _build_combined_pattern(prim_path_exprs: list[str]) -> str: - """Build a single fnmatch pattern that matches all body types. - - Compares path segments across all expressions and wildcards only the differing portion of each segment. - For example, given:: - - ["/World/Env_*/Object_A", "/World/Env_*/Object_B"] - - produces ``"/World/Env_*/Object_*"``. Retaining common prefixes and suffixes avoids selecting - unrelated sibling articulations from the same world. - - Args: - prim_path_exprs: List of prim path expressions, one per body type. - - Returns: - A single fnmatch pattern string. - - Raises: - ValueError: If the expressions have different numbers of path segments. - """ - if len(prim_path_exprs) == 1: - return prim_path_exprs[0] - - split_paths = [p.split("/") for p in prim_path_exprs] - lengths = {len(s) for s in split_paths} - if len(lengths) != 1: - raise ValueError( - f"Cannot build combined pattern: path expressions have different segment counts: {prim_path_exprs}" - ) - - combined_segments = [] - for segments in zip(*split_paths): - unique = set(segments) - if len(unique) == 1: - combined_segments.append(segments[0]) - else: - common_prefix = os.path.commonprefix(segments) - remaining_segments = [segment[len(common_prefix) :] for segment in segments] - common_suffix = os.path.commonprefix([segment[::-1] for segment in remaining_segments])[::-1] - combined_segments.append(f"{common_prefix}*{common_suffix}") - return "/".join(combined_segments) - """ Internal simulation callbacks. """ diff --git a/source/isaaclab_newton/test/assets/test_rigid_object_collection.py b/source/isaaclab_newton/test/assets/test_rigid_object_collection.py index 8724ad8ac51d..94627b73764b 100644 --- a/source/isaaclab_newton/test/assets/test_rigid_object_collection.py +++ b/source/isaaclab_newton/test/assets/test_rigid_object_collection.py @@ -66,17 +66,19 @@ def generate_cubes_scene( kinematic_enabled: bool = False, device: str = "cuda:0", spawn_unrelated_sibling: bool = False, + object_paths: tuple[str, ...] | None = None, ) -> tuple[RigidObjectCollection, torch.Tensor]: """Generate a scene with the provided number of cubes. Args: num_envs: Number of envs to generate. - num_cubes: Number of cubes to generate. + num_cubes: Number of cubes to generate when object_paths is not specified. height: Height of the cubes. has_api: Whether the cubes have a rigid body API on them. kinematic_enabled: Whether the cubes are kinematic. device: Device to use for the simulation. spawn_unrelated_sibling: Whether to spawn a rigid body outside the collection in each environment. + object_paths: Member paths relative to each environment. Defaults to Object_0, Object_1, etc. Returns: A tuple containing the rigid object collection representing the cubes and the origins of the cubes. @@ -100,23 +102,21 @@ def generate_cubes_scene( # create the rigid object configs cube_config_dict = {} - for i in range(num_cubes): + if object_paths is None: + object_paths = tuple(f"Object_{i}" for i in range(num_cubes)) + for i, path in enumerate(object_paths): cube_object_cfg = RigidObjectCfg( - prim_path=f"/World/Env_[^/]*/Object_{i}", + prim_path=f"/World/Env_[^/]*/{path}", spawn=clone(spawn_cfg), init_state=RigidObjectCfg.InitialStateCfg(pos=(0.0, 3 * i, height)), ) cube_config_dict[f"cube_{i}"] = cube_object_cfg cfgs = list(cube_config_dict.values()) if spawn_unrelated_sibling: - cfgs.append(AssetBaseCfg(prim_path="/World/Env_[^/]*/UnrelatedObject")) + cfgs.append(AssetBaseCfg(prim_path="/World/Env_[^/]*/Object_Target")) clone_plan_from_env_0(CloneCfg(clone_template="/World/Env_{}"), cfgs, num_envs, 3.0, positions=origins) if spawn_unrelated_sibling: - spawn_cfg.func( - "/World/Env_0/UnrelatedObject", - spawn_cfg, - translation=(0.0, -3.0, height), - ) + spawn_cfg.func("/World/Env_0/Object_Target", spawn_cfg, translation=(0.0, -3.0, height)) # create the rigid object collection cube_object_collection_cfg = RigidObjectCollectionCfg(rigid_objects=cube_config_dict) cube_object_collection = RigidObjectCollection(cfg=cube_object_collection_cfg) @@ -124,21 +124,23 @@ def generate_cubes_scene( return cube_object_collection, torch.as_tensor(origins, device=device) -@pytest.mark.parametrize(("num_envs", "num_cubes", "spawn_unrelated_sibling"), [(1, 1, False), (2, 3, True)]) +@pytest.mark.parametrize( + ("num_envs", "object_paths", "spawn_unrelated_sibling"), + [ + (1, ("Object_A",), False), + (2, ("Object_A", "Object_B", "Object_C"), True), + (2, ("Object_A", "Shelf/Object_B"), False), + ], + ids=["single_member", "sibling_sharing_prefix", "nested_member"], +) @pytest.mark.parametrize("device", test_devices()) -def test_initialization(num_envs, num_cubes, spawn_unrelated_sibling, device): - """Test initialization for prim with rigid body API at the provided prim path. - - With an unrelated rigid body next to the cubes in each environment, the collection view must still - select only its configured rigid objects. - """ - with _newton_sim_context(device, auto_add_lighting=True) as sim: +def test_initialization(num_envs, object_paths, spawn_unrelated_sibling, device): + """Bind only configured members, including nested paths, and preserve each body's name-to-pose mapping.""" + num_cubes = len(object_paths) + with _newton_sim_context(device, gravity_enabled=False, auto_add_lighting=True) as sim: sim._app_control_on_stop_handle = None - object_collection, _ = generate_cubes_scene( - num_envs=num_envs, - num_cubes=num_cubes, - device=device, - spawn_unrelated_sibling=spawn_unrelated_sibling, + object_collection, origins = generate_cubes_scene( + num_envs, device=device, spawn_unrelated_sibling=spawn_unrelated_sibling, object_paths=object_paths ) # Check that the framework doesn't hold excessive strong references. @@ -161,6 +163,12 @@ def test_initialization(num_envs, num_cubes, spawn_unrelated_sibling, device): assert object_collection.data.body_mass.torch.shape == (num_envs, num_cubes) assert object_collection.data.body_inertia.torch.shape == (num_envs, num_cubes, 9) + for name, member_cfg in object_collection.cfg.rigid_objects.items(): + body_ids, _ = object_collection.find_bodies(name) + body_pos = object_collection.data.body_link_pos_w.torch[:, body_ids[0]] - origins + expected_pos = torch.tensor(member_cfg.init_state.pos, device=device).expand(num_envs, 3) + torch.testing.assert_close(body_pos, expected_pos, atol=1e-5, rtol=0.0) + @pytest.mark.parametrize("device", test_devices()) def test_set_body_inertial_properties_updates_inverses(device):