From 6018d90c3ff8670dc9899708bc4a59f49f0782cc Mon Sep 17 00:00:00 2001 From: zhx06 Date: Mon, 15 Jun 2026 10:15:01 -0700 Subject: [PATCH 1/3] unification of placement result --- isaaclab_arena/relations/object_placer.py | 15 +-- isaaclab_arena/relations/placement_result.py | 63 +++++++--- .../relations/pooled_object_placer.py | 5 +- .../tests/test_heterogeneous_placement.py | 12 +- .../test_object_placer_reproducibility.py | 8 +- isaaclab_arena/tests/test_placement_events.py | 4 +- isaaclab_arena/tests/test_placement_result.py | 114 ++++++++++++++++++ 7 files changed, 176 insertions(+), 45 deletions(-) create mode 100644 isaaclab_arena/tests/test_placement_result.py diff --git a/isaaclab_arena/relations/object_placer.py b/isaaclab_arena/relations/object_placer.py index d8d279b18..ff6e49a1a 100644 --- a/isaaclab_arena/relations/object_placer.py +++ b/isaaclab_arena/relations/object_placer.py @@ -11,7 +11,7 @@ from isaaclab_arena.relations.bounding_box_helpers import assign_variants_for_envs, build_per_env_bounding_boxes from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams -from isaaclab_arena.relations.placement_result import MultiEnvPlacementResult, PlacementResult +from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.placement_validation import PlacementCheck, PlacementValidationResults from isaaclab_arena.relations.relation_solver import RelationSolver from isaaclab_arena.relations.relations import ( @@ -78,7 +78,7 @@ def place( self, objects: list[ObjectBase], num_envs: int = 1, - ) -> PlacementResult | MultiEnvPlacementResult: + ) -> PlacementResult: """Place objects according to their spatial relations. Every environment is solved against its own per-env bounding boxes and @@ -93,8 +93,8 @@ def place( placement (one layout per env). Returns: - PlacementResult when num_envs == 1, otherwise a - MultiEnvPlacementResult with one layout per environment. + A PlacementResult. For multi-env runs, iterate ``result.results`` + to access each per-env layout. """ anchor_objects_set, generator = self._prepare_placement(objects) max_attempts = self.params.max_placement_attempts @@ -123,9 +123,7 @@ def place( orientations_per_env = [r.orientations for r in results_per_env] self._apply_poses(positions_per_env, anchor_objects_set, orientations_per_env) - if num_envs == 1: - return results_per_env[0] - return MultiEnvPlacementResult(results=results_per_env) + return PlacementResult.from_per_env(results_per_env) def place_ranked_per_env( self, @@ -652,9 +650,6 @@ def _validate_placement( Args: positions: Dictionary mapping objects to their solved (x, y, z) positions. env_bboxes: Per-object bboxes for the current env, each with shape (1, 3). - - Returns: - PlacementValidationResults containing the validation results. """ no_overlap = self._validate_no_overlap(positions, env_bboxes) on_relation = self._validate_on_relations(positions, env_bboxes) diff --git a/isaaclab_arena/relations/placement_result.py b/isaaclab_arena/relations/placement_result.py index 6f79b9e80..b57271882 100644 --- a/isaaclab_arena/relations/placement_result.py +++ b/isaaclab_arena/relations/placement_result.py @@ -15,7 +15,11 @@ @dataclass class PlacementResult: - """Result of an ObjectPlacer.place() call.""" + """Result of an ObjectPlacer.place() call (single-env or multi-env). + + Use fields directly for single-env; iterate ``results`` for multi-env. + On wrappers built by ``from_per_env``, top-level fields mirror ``results[0]``. + """ validation_results: PlacementValidationResults """Validation checklist for the placement.""" @@ -33,30 +37,49 @@ class PlacementResult: """Per-object yaw (radians) about the world up (Z) axis, composed on top of each object's base rotation. Keyed by object, like positions. Empty when unrotated.""" - @property - def success(self) -> bool: - """True when this layout passed every validation check. + _per_env_results: list[PlacementResult] | None = field( + default=None, init=False, repr=False, compare=False + ) + # dataclasses.replace() resets this to None; use from_per_env(self.results) to copy a wrapper. - Soft selection: place() always returns the best-ranked layout per env, even when no - candidate validated. Callers check success to distinguish a validated layout from a - lowest-loss fallback; failed_items on the checklist says which checks failed. - """ - return self.validation_results.do_all_required_validation_checks_pass() + @classmethod + def from_per_env(cls, results: list[PlacementResult]) -> PlacementResult: + """Wrap per-env leaf results into a single PlacementResult. + Args: + results: One leaf PlacementResult per environment (not itself a wrapper). -@dataclass -class MultiEnvPlacementResult: - """Result of an ObjectPlacer.place() call for multiple environments.""" + Returns: + A PlacementResult whose top-level fields mirror ``results[0]``; + iterate ``results`` to access all per-env layouts. + """ + assert results, "from_per_env requires at least one result" + assert all(r._per_env_results is None for r in results), ( + "from_per_env requires bare PlacementResult leaves; wrapping a wrapper is not supported" + ) + first = results[0] + obj = cls( + validation_results=first.validation_results, + positions=first.positions, + final_loss=first.final_loss, + attempts=first.attempts, + orientations=first.orientations, + ) + obj._per_env_results = results + return obj - results: list[PlacementResult] - """One PlacementResult per environment (same length as num_envs).""" + @property + def results(self) -> list[PlacementResult]: + """Per-env result list; ``[self]`` for a directly-constructed leaf.""" + return self._per_env_results if self._per_env_results is not None else [self] @property def success(self) -> bool: - """True if every environment's placement succeeded.""" - return all(r.success for r in self.results) + """True when every env passed all required validation checks. - @property - def attempts(self) -> int: - """Number of attempts (same for all envs in the batched run).""" - return self.results[0].attempts if self.results else 0 + place() always returns a best-ranked layout per env even when validation fails; + check this to distinguish a validated layout from a lowest-loss fallback. + """ + if self._per_env_results is not None: + return all(r.success for r in self._per_env_results) + return self.validation_results.do_all_required_validation_checks_pass() diff --git a/isaaclab_arena/relations/pooled_object_placer.py b/isaaclab_arena/relations/pooled_object_placer.py index d13211b55..5df897354 100644 --- a/isaaclab_arena/relations/pooled_object_placer.py +++ b/isaaclab_arena/relations/pooled_object_placer.py @@ -12,7 +12,7 @@ from isaaclab_arena.relations.bounding_box_helpers import has_heterogeneous_objects from isaaclab_arena.relations.object_placer import ObjectPlacer from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams -from isaaclab_arena.relations.placement_result import MultiEnvPlacementResult, PlacementResult +from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.utils.random import get_rngs if TYPE_CHECKING: @@ -188,8 +188,7 @@ def _solve_reusable_layouts(self, num_layouts: int, allow_fallback: bool = False with torch.inference_mode(False): result = self._placer.place(self._objects, num_envs=num_layouts) - # place() returns a single PlacementResult only when num_envs == 1. - all_layouts = result.results if isinstance(result, MultiEnvPlacementResult) else [result] + all_layouts = result.results valid_layouts = [layout for layout in all_layouts if layout.success] if len(valid_layouts) < num_layouts: diff --git a/isaaclab_arena/tests/test_heterogeneous_placement.py b/isaaclab_arena/tests/test_heterogeneous_placement.py index 5392b3e9a..6101907a3 100644 --- a/isaaclab_arena/tests/test_heterogeneous_placement.py +++ b/isaaclab_arena/tests/test_heterogeneous_placement.py @@ -17,7 +17,7 @@ from isaaclab_arena.relations.bounding_box_helpers import build_per_env_bounding_boxes, get_bounding_box_per_env from isaaclab_arena.relations.object_placer import ObjectPlacer from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams -from isaaclab_arena.relations.placement_result import MultiEnvPlacementResult, PlacementResult +from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.placement_validation import PlacementValidationResults from isaaclab_arena.relations.pooled_object_placer import PooledObjectPlacer from isaaclab_arena.relations.relation_solver import RelationSolver @@ -252,7 +252,7 @@ def test_object_placer_heterogeneous_produces_per_env_results(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) assert len(result.results) == num_envs for r in result.results: assert hetero_box in r.positions @@ -283,7 +283,7 @@ def test_object_placer_heterogeneous_z_height_matches_variant(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) # Both envs should have solved z near the desk top + clearance (0.11). # The On loss targets: z = parent_top + clearance - child_min_z = 0.1 + 0.01 - 0.0 = 0.11 for env_idx, r in enumerate(result.results): @@ -329,7 +329,7 @@ def test_mixed_heterogeneous_and_homogeneous_placement(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) assert len(result.results) == num_envs for env_idx, r in enumerate(result.results): @@ -365,7 +365,7 @@ def test_heterogeneous_placement_always_returns_per_env_results(): placer = ObjectPlacer(params=params) result = placer.place([desk, hetero], num_envs=4) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) assert len(result.results) == 4 @@ -424,7 +424,7 @@ def test_object_placer_homogeneous_objects_return_multi_env_result(): placer = ObjectPlacer(params=params) result = placer.place([desk, box], num_envs=2) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) assert len(result.results) == 2 assert all(r.success for r in result.results) diff --git a/isaaclab_arena/tests/test_object_placer_reproducibility.py b/isaaclab_arena/tests/test_object_placer_reproducibility.py index 690018e12..748fd501d 100644 --- a/isaaclab_arena/tests/test_object_placer_reproducibility.py +++ b/isaaclab_arena/tests/test_object_placer_reproducibility.py @@ -12,7 +12,7 @@ from isaaclab_arena.assets.dummy_object import DummyObject from isaaclab_arena.relations.object_placer import ObjectPlacer from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams -from isaaclab_arena.relations.placement_result import MultiEnvPlacementResult, PlacementResult +from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.pooled_object_placer import PooledObjectPlacer from isaaclab_arena.relations.relation_solver import RelationSolver from isaaclab_arena.relations.relation_solver_params import RelationSolverParams @@ -142,7 +142,7 @@ def test_object_placer_different_seeds_produce_different_results(): def test_object_placer_multi_env_returns_multi_env_result(): - """Test that ObjectPlacer.place with num_envs>1 returns MultiEnvPlacementResult.""" + """Test that ObjectPlacer.place with num_envs>1 returns PlacementResult with per-env results.""" num_envs = 4 solver_params = RelationSolverParams(max_iters=200, convergence_threshold=1e-3) desk, box1, box2 = _create_test_objects() @@ -150,7 +150,7 @@ def test_object_placer_multi_env_returns_multi_env_result(): placer = ObjectPlacer(params=ObjectPlacerParams(placement_seed=42, solver_params=solver_params)) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) assert len(result.results) == num_envs for r in result.results: assert isinstance(r, PlacementResult) @@ -169,7 +169,7 @@ def test_object_placer_multi_env_produces_different_positions(): placer = ObjectPlacer(params=ObjectPlacerParams(placement_seed=42, solver_params=solver_params)) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) # At least one pair of envs should have different positions for a non-anchor object. positions_box1 = [result.results[e].positions[box1] for e in range(num_envs)] any_different = any(positions_box1[i] != positions_box1[j] for i in range(num_envs) for j in range(i + 1, num_envs)) diff --git a/isaaclab_arena/tests/test_placement_events.py b/isaaclab_arena/tests/test_placement_events.py index 1440d8070..e1098a06a 100644 --- a/isaaclab_arena/tests/test_placement_events.py +++ b/isaaclab_arena/tests/test_placement_events.py @@ -84,7 +84,7 @@ def test_placement_without_seed_multi_env_gives_different_layouts(): from isaaclab_arena.relations.object_placer import ObjectPlacer from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams - from isaaclab_arena.relations.placement_result import MultiEnvPlacementResult + from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.relation_solver_params import RelationSolverParams num_envs = 4 @@ -99,7 +99,7 @@ def test_placement_without_seed_multi_env_gives_different_layouts(): placer = ObjectPlacer(params=params) result = placer.place([desk, box1, box2], num_envs=num_envs) - assert isinstance(result, MultiEnvPlacementResult) + assert isinstance(result, PlacementResult) positions_box1 = [result.results[env_idx].positions[box1] for env_idx in range(num_envs)] any_different = any(positions_box1[i] != positions_box1[j] for i in range(num_envs) for j in range(i + 1, num_envs)) assert any_different, "Unseeded multi-env placement should produce different positions across environments" diff --git a/isaaclab_arena/tests/test_placement_result.py b/isaaclab_arena/tests/test_placement_result.py new file mode 100644 index 000000000..d9c57dbe1 --- /dev/null +++ b/isaaclab_arena/tests/test_placement_result.py @@ -0,0 +1,114 @@ +# Copyright (c) 2025-2026, The Isaac Lab Arena Project Developers (https://github.com/isaac-sim/IsaacLab-Arena/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the unified PlacementResult type.""" + +import pytest + +from isaaclab_arena.relations.placement_result import PlacementResult +from isaaclab_arena.relations.placement_validation import PlacementCheck, PlacementValidationResults + + +def _make_leaf(*, success: bool = True, final_loss: float = 0.0, attempts: int = 1) -> PlacementResult: + """Create a directly-constructed (leaf) PlacementResult for testing.""" + return PlacementResult( + validation_results=PlacementValidationResults( + validation_results={PlacementCheck.NO_OVERLAP: success}, + required_checks={PlacementCheck.NO_OVERLAP}, + ), + positions={}, + final_loss=final_loss, + attempts=attempts, + ) + + +def test_leaf_results_returns_self(): + """Directly-constructed result's .results returns [self].""" + leaf = _make_leaf() + assert leaf.results == [leaf] + assert leaf.results[0] is leaf + + +def test_leaf_success_delegates_to_validation(): + passing = _make_leaf(success=True) + assert passing.success is True + + failing = _make_leaf(success=False) + assert failing.success is False + + +def test_from_per_env_results_returns_original_list(): + leaves = [_make_leaf() for _ in range(3)] + wrapper = PlacementResult.from_per_env(leaves) + assert len(wrapper.results) == 3 + for i, r in enumerate(wrapper.results): + assert r is leaves[i] + + +def test_from_per_env_single_preserves_identity(): + """from_per_env([single]).results[0] is the original single leaf.""" + single = _make_leaf() + wrapper = PlacementResult.from_per_env([single]) + assert wrapper.results[0] is single + + +def test_from_per_env_top_level_fields_mirror_first(): + """Wrapper's top-level fields come from results[0], not results[1].""" + leaf_a = _make_leaf(final_loss=1.5, attempts=10) + leaf_b = _make_leaf(final_loss=3.0, attempts=20) + wrapper = PlacementResult.from_per_env([leaf_a, leaf_b]) + assert wrapper.validation_results is leaf_a.validation_results + assert wrapper.positions is leaf_a.positions + assert wrapper.final_loss == 1.5 + assert wrapper.attempts == 10 + assert wrapper.orientations is leaf_a.orientations + + +def test_success_multi_env_all_pass(): + leaves = [_make_leaf(success=True) for _ in range(4)] + wrapper = PlacementResult.from_per_env(leaves) + assert wrapper.success is True + + +def test_success_multi_env_mixed(): + """When some envs fail, wrapper.success is False.""" + leaves = [_make_leaf(success=True), _make_leaf(success=False), _make_leaf(success=True)] + wrapper = PlacementResult.from_per_env(leaves) + assert wrapper.success is False + + +def test_success_multi_env_all_fail(): + leaves = [_make_leaf(success=False) for _ in range(2)] + wrapper = PlacementResult.from_per_env(leaves) + assert wrapper.success is False + + +def test_from_per_env_empty_list_raises(): + with pytest.raises(AssertionError, match="from_per_env requires at least one result"): + PlacementResult.from_per_env([]) + + +def test_from_per_env_rejects_nested_wrappers(): + """Wrapping a wrapper is not supported and should raise.""" + leaf = _make_leaf() + wrapper = PlacementResult.from_per_env([leaf]) + with pytest.raises(AssertionError, match="wrapping a wrapper is not supported"): + PlacementResult.from_per_env([wrapper]) + + +def test_dataclasses_replace_strips_per_env_results(): + """dataclasses.replace() does not preserve _per_env_results (init=False field). + + Rebuild via from_per_env(original.results) instead. + """ + import dataclasses + + leaves = [_make_leaf(success=True), _make_leaf(success=False)] + wrapper = PlacementResult.from_per_env(leaves) + assert wrapper.success is False + + copied = dataclasses.replace(wrapper) + assert copied._per_env_results is None + assert copied.success is True From d20ca809b8bb70679167175f6f4f2e9f0d371736 Mon Sep 17 00:00:00 2001 From: zhx06 Date: Mon, 15 Jun 2026 10:58:00 -0700 Subject: [PATCH 2/3] address comments --- isaaclab_arena/relations/object_placer.py | 2 +- isaaclab_arena/relations/placement_result.py | 22 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/isaaclab_arena/relations/object_placer.py b/isaaclab_arena/relations/object_placer.py index ff6e49a1a..f67ca7ca9 100644 --- a/isaaclab_arena/relations/object_placer.py +++ b/isaaclab_arena/relations/object_placer.py @@ -93,7 +93,7 @@ def place( placement (one layout per env). Returns: - A PlacementResult. For multi-env runs, iterate ``result.results`` + A PlacementResult. For multi-env runs, iterate result.results to access each per-env layout. """ anchor_objects_set, generator = self._prepare_placement(objects) diff --git a/isaaclab_arena/relations/placement_result.py b/isaaclab_arena/relations/placement_result.py index b57271882..69671949d 100644 --- a/isaaclab_arena/relations/placement_result.py +++ b/isaaclab_arena/relations/placement_result.py @@ -17,8 +17,10 @@ class PlacementResult: """Result of an ObjectPlacer.place() call (single-env or multi-env). - Use fields directly for single-env; iterate ``results`` for multi-env. - On wrappers built by ``from_per_env``, top-level fields mirror ``results[0]``. + Use fields directly for single-env; iterate results for multi-env. + On wrappers built by from_per_env, top-level fields mirror results[0]. + dataclasses.replace() does not preserve the wrapper; use + from_per_env(original.results) to copy one. """ validation_results: PlacementValidationResults @@ -37,9 +39,7 @@ class PlacementResult: """Per-object yaw (radians) about the world up (Z) axis, composed on top of each object's base rotation. Keyed by object, like positions. Empty when unrotated.""" - _per_env_results: list[PlacementResult] | None = field( - default=None, init=False, repr=False, compare=False - ) + _per_env_results: list[PlacementResult] | None = field(default=None, init=False, repr=False, compare=False) # dataclasses.replace() resets this to None; use from_per_env(self.results) to copy a wrapper. @classmethod @@ -50,13 +50,13 @@ def from_per_env(cls, results: list[PlacementResult]) -> PlacementResult: results: One leaf PlacementResult per environment (not itself a wrapper). Returns: - A PlacementResult whose top-level fields mirror ``results[0]``; - iterate ``results`` to access all per-env layouts. + A PlacementResult whose top-level fields mirror results[0]; + iterate results to access all per-env layouts. """ assert results, "from_per_env requires at least one result" - assert all(r._per_env_results is None for r in results), ( - "from_per_env requires bare PlacementResult leaves; wrapping a wrapper is not supported" - ) + assert all( + r._per_env_results is None for r in results + ), "from_per_env requires bare PlacementResult leaves; wrapping a wrapper is not supported" first = results[0] obj = cls( validation_results=first.validation_results, @@ -70,7 +70,7 @@ def from_per_env(cls, results: list[PlacementResult]) -> PlacementResult: @property def results(self) -> list[PlacementResult]: - """Per-env result list; ``[self]`` for a directly-constructed leaf.""" + """Per-env result list; [self] for a directly-constructed leaf.""" return self._per_env_results if self._per_env_results is not None else [self] @property From 90676a29d658bdbf1756555a9c2bb4ab183ecad9 Mon Sep 17 00:00:00 2001 From: zhx06 Date: Tue, 16 Jun 2026 09:54:55 -0700 Subject: [PATCH 3/3] remove multi-env Signed-off-by: zhx06 --- isaaclab_arena/relations/object_placer.py | 10 +- isaaclab_arena/relations/placement_result.py | 49 +------- .../relations/pooled_object_placer.py | 4 +- .../tests/test_heterogeneous_placement.py | 24 ++-- .../tests/test_object_placer_init.py | 3 +- .../test_object_placer_reproducibility.py | 26 ++-- isaaclab_arena/tests/test_placement_events.py | 13 +- isaaclab_arena/tests/test_placement_result.py | 118 ++++-------------- .../relations/dummy_object_placer_notebook.py | 4 +- .../isaac_sim_no_collision_notebook.py | 2 +- 10 files changed, 68 insertions(+), 185 deletions(-) diff --git a/isaaclab_arena/relations/object_placer.py b/isaaclab_arena/relations/object_placer.py index f67ca7ca9..2515a6e30 100644 --- a/isaaclab_arena/relations/object_placer.py +++ b/isaaclab_arena/relations/object_placer.py @@ -78,7 +78,7 @@ def place( self, objects: list[ObjectBase], num_envs: int = 1, - ) -> PlacementResult: + ) -> list[PlacementResult]: """Place objects according to their spatial relations. Every environment is solved against its own per-env bounding boxes and @@ -93,8 +93,7 @@ def place( placement (one layout per env). Returns: - A PlacementResult. For multi-env runs, iterate result.results - to access each per-env layout. + One PlacementResult per environment. """ anchor_objects_set, generator = self._prepare_placement(objects) max_attempts = self.params.max_placement_attempts @@ -123,7 +122,7 @@ def place( orientations_per_env = [r.orientations for r in results_per_env] self._apply_poses(positions_per_env, anchor_objects_set, orientations_per_env) - return PlacementResult.from_per_env(results_per_env) + return results_per_env def place_ranked_per_env( self, @@ -650,6 +649,9 @@ def _validate_placement( Args: positions: Dictionary mapping objects to their solved (x, y, z) positions. env_bboxes: Per-object bboxes for the current env, each with shape (1, 3). + + Returns: + PlacementValidationResults with the overlap and on-relation checks. """ no_overlap = self._validate_no_overlap(positions, env_bboxes) on_relation = self._validate_on_relations(positions, env_bboxes) diff --git a/isaaclab_arena/relations/placement_result.py b/isaaclab_arena/relations/placement_result.py index 69671949d..13cd9b252 100644 --- a/isaaclab_arena/relations/placement_result.py +++ b/isaaclab_arena/relations/placement_result.py @@ -15,13 +15,7 @@ @dataclass class PlacementResult: - """Result of an ObjectPlacer.place() call (single-env or multi-env). - - Use fields directly for single-env; iterate results for multi-env. - On wrappers built by from_per_env, top-level fields mirror results[0]. - dataclasses.replace() does not preserve the wrapper; use - from_per_env(original.results) to copy one. - """ + """Solved object layout for one environment.""" validation_results: PlacementValidationResults """Validation checklist for the placement.""" @@ -39,47 +33,10 @@ class PlacementResult: """Per-object yaw (radians) about the world up (Z) axis, composed on top of each object's base rotation. Keyed by object, like positions. Empty when unrotated.""" - _per_env_results: list[PlacementResult] | None = field(default=None, init=False, repr=False, compare=False) - # dataclasses.replace() resets this to None; use from_per_env(self.results) to copy a wrapper. - - @classmethod - def from_per_env(cls, results: list[PlacementResult]) -> PlacementResult: - """Wrap per-env leaf results into a single PlacementResult. - - Args: - results: One leaf PlacementResult per environment (not itself a wrapper). - - Returns: - A PlacementResult whose top-level fields mirror results[0]; - iterate results to access all per-env layouts. - """ - assert results, "from_per_env requires at least one result" - assert all( - r._per_env_results is None for r in results - ), "from_per_env requires bare PlacementResult leaves; wrapping a wrapper is not supported" - first = results[0] - obj = cls( - validation_results=first.validation_results, - positions=first.positions, - final_loss=first.final_loss, - attempts=first.attempts, - orientations=first.orientations, - ) - obj._per_env_results = results - return obj - - @property - def results(self) -> list[PlacementResult]: - """Per-env result list; [self] for a directly-constructed leaf.""" - return self._per_env_results if self._per_env_results is not None else [self] - @property def success(self) -> bool: - """True when every env passed all required validation checks. + """True when all required validation checks pass. - place() always returns a best-ranked layout per env even when validation fails; - check this to distinguish a validated layout from a lowest-loss fallback. + place() returns a best-loss fallback even on failure; check this to tell validated from fallback. """ - if self._per_env_results is not None: - return all(r.success for r in self._per_env_results) return self.validation_results.do_all_required_validation_checks_pass() diff --git a/isaaclab_arena/relations/pooled_object_placer.py b/isaaclab_arena/relations/pooled_object_placer.py index 5df897354..640195743 100644 --- a/isaaclab_arena/relations/pooled_object_placer.py +++ b/isaaclab_arena/relations/pooled_object_placer.py @@ -186,9 +186,7 @@ def _solve_reusable_layouts(self, num_layouts: int, allow_fallback: bool = False """ self._prepare_seeded_solve(num_layouts * self._placer.params.max_placement_attempts) with torch.inference_mode(False): - result = self._placer.place(self._objects, num_envs=num_layouts) - - all_layouts = result.results + all_layouts = self._placer.place(self._objects, num_envs=num_layouts) valid_layouts = [layout for layout in all_layouts if layout.success] if len(valid_layouts) < num_layouts: diff --git a/isaaclab_arena/tests/test_heterogeneous_placement.py b/isaaclab_arena/tests/test_heterogeneous_placement.py index 6101907a3..49396c40c 100644 --- a/isaaclab_arena/tests/test_heterogeneous_placement.py +++ b/isaaclab_arena/tests/test_heterogeneous_placement.py @@ -252,9 +252,8 @@ def test_object_placer_heterogeneous_produces_per_env_results(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, PlacementResult) - assert len(result.results) == num_envs - for r in result.results: + assert len(result) == num_envs + for r in result: assert hetero_box in r.positions @@ -283,10 +282,8 @@ def test_object_placer_heterogeneous_z_height_matches_variant(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, PlacementResult) - # Both envs should have solved z near the desk top + clearance (0.11). - # The On loss targets: z = parent_top + clearance - child_min_z = 0.1 + 0.01 - 0.0 = 0.11 - for env_idx, r in enumerate(result.results): + assert len(result) == num_envs + for env_idx, r in enumerate(result): z = r.positions[hetero][2] assert abs(z - 0.11) < 0.05, f"Env {env_idx}: z={z:.4f}, expected ~0.11" @@ -329,10 +326,9 @@ def test_mixed_heterogeneous_and_homogeneous_placement(): placer = ObjectPlacer(params=params) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, PlacementResult) - assert len(result.results) == num_envs + assert len(result) == num_envs - for env_idx, r in enumerate(result.results): + for env_idx, r in enumerate(result): assert obj_a in r.positions and obj_x in r.positions # Verify z-height is near desk top + clearance for both objects. for obj in (obj_a, obj_x): @@ -365,8 +361,7 @@ def test_heterogeneous_placement_always_returns_per_env_results(): placer = ObjectPlacer(params=params) result = placer.place([desk, hetero], num_envs=4) - assert isinstance(result, PlacementResult) - assert len(result.results) == 4 + assert len(result) == 4 def test_object_placer_place_ranked_per_env_returns_sorted_env_lists(): @@ -424,9 +419,8 @@ def test_object_placer_homogeneous_objects_return_multi_env_result(): placer = ObjectPlacer(params=params) result = placer.place([desk, box], num_envs=2) - assert isinstance(result, PlacementResult) - assert len(result.results) == 2 - assert all(r.success for r in result.results) + assert len(result) == 2 + assert all(r.success for r in result) # --------------------------------------------------------------------------- diff --git a/isaaclab_arena/tests/test_object_placer_init.py b/isaaclab_arena/tests/test_object_placer_init.py index e26883e1e..3510a03c5 100644 --- a/isaaclab_arena/tests/test_object_placer_init.py +++ b/isaaclab_arena/tests/test_object_placer_init.py @@ -222,7 +222,8 @@ def _run(): bounding_box=AxisAlignedBoundingBox(min_point=(0.0, 0.0, 0.0), max_point=(0.2, 0.2, 0.2)), ) box.add_relation(On(desk, clearance_m=0.01)) - return ObjectPlacer(params=params).place([desk, box]) + (result,) = ObjectPlacer(params=params).place([desk, box]) + return result result1 = _run() result2 = _run() diff --git a/isaaclab_arena/tests/test_object_placer_reproducibility.py b/isaaclab_arena/tests/test_object_placer_reproducibility.py index 748fd501d..10122fb0f 100644 --- a/isaaclab_arena/tests/test_object_placer_reproducibility.py +++ b/isaaclab_arena/tests/test_object_placer_reproducibility.py @@ -97,13 +97,13 @@ def test_object_placer_same_seed_produces_identical_result(): desk1, box1_run1, box2_run1 = _create_test_objects() objects1 = [desk1, box1_run1, box2_run1] placer1 = ObjectPlacer(params=ObjectPlacerParams(placement_seed=seed, solver_params=solver_params)) - result1 = placer1.place(objects=objects1) + (result1,) = placer1.place(objects=objects1) # Run 2 desk2, box1_run2, box2_run2 = _create_test_objects() objects2 = [desk2, box1_run2, box2_run2] placer2 = ObjectPlacer(params=ObjectPlacerParams(placement_seed=seed, solver_params=solver_params)) - result2 = placer2.place(objects=objects2) + (result2,) = placer2.place(objects=objects2) # Compare by name for obj1, obj2 in zip(objects1, objects2): @@ -121,13 +121,13 @@ def test_object_placer_different_seeds_produce_different_results(): desk1, box1_run1, box2_run1 = _create_test_objects() objects1 = [desk1, box1_run1, box2_run1] placer1 = ObjectPlacer(params=ObjectPlacerParams(placement_seed=42, solver_params=solver_params)) - result1 = placer1.place(objects=objects1) + (result1,) = placer1.place(objects=objects1) # Run 2 with seed 123 desk2, box1_run2, box2_run2 = _create_test_objects() objects2 = [desk2, box1_run2, box2_run2] placer2 = ObjectPlacer(params=ObjectPlacerParams(placement_seed=123, solver_params=solver_params)) - result2 = placer2.place(objects=objects2) + (result2,) = placer2.place(objects=objects2) # Check that at least one non-anchor position differs any_different = False @@ -142,7 +142,7 @@ def test_object_placer_different_seeds_produce_different_results(): def test_object_placer_multi_env_returns_multi_env_result(): - """Test that ObjectPlacer.place with num_envs>1 returns PlacementResult with per-env results.""" + """place() with num_envs>1 returns one PlacementResult per env.""" num_envs = 4 solver_params = RelationSolverParams(max_iters=200, convergence_threshold=1e-3) desk, box1, box2 = _create_test_objects() @@ -150,9 +150,8 @@ def test_object_placer_multi_env_returns_multi_env_result(): placer = ObjectPlacer(params=ObjectPlacerParams(placement_seed=42, solver_params=solver_params)) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, PlacementResult) - assert len(result.results) == num_envs - for r in result.results: + assert len(result) == num_envs + for r in result: assert isinstance(r, PlacementResult) assert box1 in r.positions assert box2 in r.positions @@ -161,7 +160,7 @@ def test_object_placer_multi_env_returns_multi_env_result(): def test_object_placer_multi_env_produces_different_positions(): - """Test that multi-env placement produces different positions across environments.""" + """Multi-env placement produces different positions across envs.""" num_envs = 4 solver_params = RelationSolverParams(max_iters=200, convergence_threshold=1e-3) desk, box1, box2 = _create_test_objects() @@ -169,9 +168,8 @@ def test_object_placer_multi_env_produces_different_positions(): placer = ObjectPlacer(params=ObjectPlacerParams(placement_seed=42, solver_params=solver_params)) result = placer.place(objects, num_envs=num_envs) - assert isinstance(result, PlacementResult) - # At least one pair of envs should have different positions for a non-anchor object. - positions_box1 = [result.results[e].positions[box1] for e in range(num_envs)] + assert len(result) == num_envs + positions_box1 = [result[e].positions[box1] for e in range(num_envs)] any_different = any(positions_box1[i] != positions_box1[j] for i in range(num_envs) for j in range(i + 1, num_envs)) assert any_different, "Multi-env placement should produce different positions across environments" @@ -301,7 +299,7 @@ def test_random_yaw_init_applied_yaw_matches_selected_candidate(): placer = ObjectPlacer( params=ObjectPlacerParams(placement_seed=11, solver_params=solver_params, random_yaw_init=True) ) - result = placer.place([desk, box1, box2], num_envs=1) + (result,) = placer.place([desk, box1, box2], num_envs=1) for box in (box1, box2): applied = _yaw_rad_from_quat(box.get_initial_pose().rotation_xyzw) assert abs(wrap_angle_to_pi(applied - result.orientations[box])) < 1e-5 @@ -316,7 +314,7 @@ def test_random_yaw_init_composes_marker_yaw(): placer = ObjectPlacer( params=ObjectPlacerParams(placement_seed=3, solver_params=solver_params, random_yaw_init=True) ) - result = placer.place([desk, box1, box2], num_envs=1) + (result,) = placer.place([desk, box1, box2], num_envs=1) applied = _yaw_rad_from_quat(box1.get_initial_pose().rotation_xyzw) assert abs(wrap_angle_to_pi(applied - (marker_yaw + result.orientations[box1]))) < 1e-5 diff --git a/isaaclab_arena/tests/test_placement_events.py b/isaaclab_arena/tests/test_placement_events.py index e1098a06a..aefa013bc 100644 --- a/isaaclab_arena/tests/test_placement_events.py +++ b/isaaclab_arena/tests/test_placement_events.py @@ -65,11 +65,11 @@ def test_successive_placements_without_seed_produce_different_layouts(): desk1, box1_a, box2_a = _create_test_objects() placer_a = ObjectPlacer(params=params) - result_a = placer_a.place([desk1, box1_a, box2_a], num_envs=1) + (result_a,) = placer_a.place([desk1, box1_a, box2_a], num_envs=1) desk2, box1_b, box2_b = _create_test_objects() placer_b = ObjectPlacer(params=params) - result_b = placer_b.place([desk2, box1_b, box2_b], num_envs=1) + (result_b,) = placer_b.place([desk2, box1_b, box2_b], num_envs=1) any_different = False for obj_a, obj_b in zip([box1_a, box2_a], [box1_b, box2_b]): @@ -84,7 +84,6 @@ def test_placement_without_seed_multi_env_gives_different_layouts(): from isaaclab_arena.relations.object_placer import ObjectPlacer from isaaclab_arena.relations.object_placer_params import ObjectPlacerParams - from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.relation_solver_params import RelationSolverParams num_envs = 4 @@ -99,8 +98,8 @@ def test_placement_without_seed_multi_env_gives_different_layouts(): placer = ObjectPlacer(params=params) result = placer.place([desk, box1, box2], num_envs=num_envs) - assert isinstance(result, PlacementResult) - positions_box1 = [result.results[env_idx].positions[box1] for env_idx in range(num_envs)] + assert len(result) == num_envs + positions_box1 = [result[env_idx].positions[box1] for env_idx in range(num_envs)] any_different = any(positions_box1[i] != positions_box1[j] for i in range(num_envs) for j in range(i + 1, num_envs)) assert any_different, "Unseeded multi-env placement should produce different positions across environments" @@ -121,11 +120,11 @@ def test_successive_seeded_placements_produce_same_layout(): desk1, box1_a, box2_a = _create_test_objects() placer_a = ObjectPlacer(params=params) - result_a = placer_a.place([desk1, box1_a, box2_a], num_envs=1) + (result_a,) = placer_a.place([desk1, box1_a, box2_a], num_envs=1) desk2, box1_b, box2_b = _create_test_objects() placer_b = ObjectPlacer(params=params) - result_b = placer_b.place([desk2, box1_b, box2_b], num_envs=1) + (result_b,) = placer_b.place([desk2, box1_b, box2_b], num_envs=1) for obj_a, obj_b in zip([box1_a, box2_a], [box1_b, box2_b]): assert ( diff --git a/isaaclab_arena/tests/test_placement_result.py b/isaaclab_arena/tests/test_placement_result.py index d9c57dbe1..3b17f8354 100644 --- a/isaaclab_arena/tests/test_placement_result.py +++ b/isaaclab_arena/tests/test_placement_result.py @@ -3,112 +3,46 @@ # # SPDX-License-Identifier: Apache-2.0 -"""Unit tests for the unified PlacementResult type.""" - -import pytest +"""Unit tests for PlacementResult.""" from isaaclab_arena.relations.placement_result import PlacementResult from isaaclab_arena.relations.placement_validation import PlacementCheck, PlacementValidationResults -def _make_leaf(*, success: bool = True, final_loss: float = 0.0, attempts: int = 1) -> PlacementResult: - """Create a directly-constructed (leaf) PlacementResult for testing.""" +def _make_result( + *, + required: dict[PlacementCheck, bool] | None = None, + optional: dict[PlacementCheck, bool] | None = None, + final_loss: float = 0.0, +) -> PlacementResult: + required = required or {PlacementCheck.NO_OVERLAP: True} + all_checks = dict(required) + if optional: + all_checks.update(optional) return PlacementResult( validation_results=PlacementValidationResults( - validation_results={PlacementCheck.NO_OVERLAP: success}, - required_checks={PlacementCheck.NO_OVERLAP}, + validation_results=all_checks, + required_checks=set(required), ), positions={}, final_loss=final_loss, - attempts=attempts, + attempts=1, ) -def test_leaf_results_returns_self(): - """Directly-constructed result's .results returns [self].""" - leaf = _make_leaf() - assert leaf.results == [leaf] - assert leaf.results[0] is leaf - - -def test_leaf_success_delegates_to_validation(): - passing = _make_leaf(success=True) - assert passing.success is True - - failing = _make_leaf(success=False) - assert failing.success is False - - -def test_from_per_env_results_returns_original_list(): - leaves = [_make_leaf() for _ in range(3)] - wrapper = PlacementResult.from_per_env(leaves) - assert len(wrapper.results) == 3 - for i, r in enumerate(wrapper.results): - assert r is leaves[i] - - -def test_from_per_env_single_preserves_identity(): - """from_per_env([single]).results[0] is the original single leaf.""" - single = _make_leaf() - wrapper = PlacementResult.from_per_env([single]) - assert wrapper.results[0] is single - - -def test_from_per_env_top_level_fields_mirror_first(): - """Wrapper's top-level fields come from results[0], not results[1].""" - leaf_a = _make_leaf(final_loss=1.5, attempts=10) - leaf_b = _make_leaf(final_loss=3.0, attempts=20) - wrapper = PlacementResult.from_per_env([leaf_a, leaf_b]) - assert wrapper.validation_results is leaf_a.validation_results - assert wrapper.positions is leaf_a.positions - assert wrapper.final_loss == 1.5 - assert wrapper.attempts == 10 - assert wrapper.orientations is leaf_a.orientations - +def test_success_true_when_all_required_pass(): + result = _make_result(required={PlacementCheck.NO_OVERLAP: True, PlacementCheck.ON_RELATION: True}) + assert result.success is True -def test_success_multi_env_all_pass(): - leaves = [_make_leaf(success=True) for _ in range(4)] - wrapper = PlacementResult.from_per_env(leaves) - assert wrapper.success is True +def test_success_false_when_required_fails(): + result = _make_result(required={PlacementCheck.NO_OVERLAP: False}) + assert result.success is False -def test_success_multi_env_mixed(): - """When some envs fail, wrapper.success is False.""" - leaves = [_make_leaf(success=True), _make_leaf(success=False), _make_leaf(success=True)] - wrapper = PlacementResult.from_per_env(leaves) - assert wrapper.success is False - -def test_success_multi_env_all_fail(): - leaves = [_make_leaf(success=False) for _ in range(2)] - wrapper = PlacementResult.from_per_env(leaves) - assert wrapper.success is False - - -def test_from_per_env_empty_list_raises(): - with pytest.raises(AssertionError, match="from_per_env requires at least one result"): - PlacementResult.from_per_env([]) - - -def test_from_per_env_rejects_nested_wrappers(): - """Wrapping a wrapper is not supported and should raise.""" - leaf = _make_leaf() - wrapper = PlacementResult.from_per_env([leaf]) - with pytest.raises(AssertionError, match="wrapping a wrapper is not supported"): - PlacementResult.from_per_env([wrapper]) - - -def test_dataclasses_replace_strips_per_env_results(): - """dataclasses.replace() does not preserve _per_env_results (init=False field). - - Rebuild via from_per_env(original.results) instead. - """ - import dataclasses - - leaves = [_make_leaf(success=True), _make_leaf(success=False)] - wrapper = PlacementResult.from_per_env(leaves) - assert wrapper.success is False - - copied = dataclasses.replace(wrapper) - assert copied._per_env_results is None - assert copied.success is True +def test_success_ignores_failed_optional_check(): + result = _make_result( + required={PlacementCheck.NO_OVERLAP: True}, + optional={PlacementCheck.PHYSICS_SETTLED: False}, + ) + assert result.success is True diff --git a/isaaclab_arena_examples/relations/dummy_object_placer_notebook.py b/isaaclab_arena_examples/relations/dummy_object_placer_notebook.py index d67969157..66826f5a0 100644 --- a/isaaclab_arena_examples/relations/dummy_object_placer_notebook.py +++ b/isaaclab_arena_examples/relations/dummy_object_placer_notebook.py @@ -80,7 +80,7 @@ def run_dummy_object_placer_demo(): # Place objects using ObjectPlacer (anchor is auto-detected via IsAnchor relation) placer = ObjectPlacer(params=ObjectPlacerParams()) - result = placer.place(objects=all_objects) + (result,) = placer.place(objects=all_objects) # Visualization visualizer = RelationSolverVisualizer( @@ -142,7 +142,7 @@ def run_dummy_multi_anchor_demo(): # Place objects (verbose=True shows anchors and optimizable objects) placer = ObjectPlacer(params=ObjectPlacerParams(verbose=True)) - result = placer.place(objects=all_objects) + (result,) = placer.place(objects=all_objects) print("\nFinal positions:") for obj, pos in result.positions.items(): diff --git a/isaaclab_arena_examples/relations/isaac_sim_no_collision_notebook.py b/isaaclab_arena_examples/relations/isaac_sim_no_collision_notebook.py index 2fc6a11a7..74006d7cc 100644 --- a/isaaclab_arena_examples/relations/isaac_sim_no_collision_notebook.py +++ b/isaaclab_arena_examples/relations/isaac_sim_no_collision_notebook.py @@ -129,7 +129,7 @@ def apply_overlapping_pose_then_solve_and_display(): env.unwrapped.sim.render() # Run the solver and apply solved poses to the sim. placer = ObjectPlacer() - result = placer.place(objects=objects_with_relations) + (result,) = placer.place(objects=objects_with_relations) for obj in placeable_objects: if obj.name not in env.unwrapped.scene.rigid_objects: print(