Mesh-based Non-collision Constraints

[zhx06]

Summary

Add mesh-based non-collision constraints via sphere-to-SDF, unified with the existing AABB path

Detailed description

• Introduces CollisionMode.MESH as an alternative to AABB for no-overlap constraints, using greedy sphere decomposition + differentiable Warp SDF queries against actual collision geometry.
• Solver falls back to AABB for pairs where either object lacks a mesh. Validator mirrors this.
• Unifies naming and computational flow between AABB and mesh modes: both use subject/obstacle terminology, forward/reverse directed pairs, and broadphase gating.
• MeshPairCache dataclass and MeshPairEntry NamedTuple give the mesh path the same collect-then-batch structure as the AABB vectorized path.
• Environment-level placer_params is the sole configuration source for collision_mode and random_yaw_init (the --random_yaw_init CLI flag is removed; these are environment-level decisions, not runtime toggles).
• Shared yaw rotation utilities (rotate_points_by_yaw, centers_in_target_frame) in utils/pose.py serve both the GPU solver and CPU validator, eliminating duplicate rotation math.

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot — PR #771

Mesh-based Non-collision Constraints

Summary

This PR adds sphere-to-SDF mesh collision support as an alternative to AABB overlap detection. The architecture is clean — CollisionMode.MESH integrates well into the existing NoCollisionLossStrategy dispatch, and the greedy sphere decomposition + Warp SDF kernel approach is sound. The test suite is thorough (542 lines!) with good coverage of dispatch routing, gradient flow, and integration.

Findings

#	Severity	Finding
1	🟡 Warning	Validator creates fresh WarpMeshManager per call — cache never reused
2	🟡 Warning	Scale applied post-transform in extract_trimesh_from_usd may be incorrect for nested prims
3	🔵 Suggestion	object_base.py abstract method has no explicit return None
4	🔵 Suggestion	Sentinel warning pattern on function object is not thread-safe
5	🔵 Suggestion	Consider documenting the rotated-anchor limitation more prominently

See inline comments for details.

---

Update (5e86ed0a): Reviewed incremental changes since 655ac73.

Addressed Findings

• ✅ Finding #1 resolved — _get_cpu_mesh_manager() now lazily creates and caches the WarpMeshManager on the instance, eliminating redundant allocations per validation call. Good fix.
• ✅ Finding #2 resolved — Removed erroneous .T transpose on ComputeLocalToWorldTransform in usd_helpers.py. USD returns row-major matrices; the transpose was producing incorrect vertex transforms for nested prims.

Other Changes

• Validation logic refactored (_validate_placement): Mesh mode now skips AABB validation entirely (else branch). Previously both checks ran in mesh mode — the AABB check was redundant and could produce false negatives for non-convex shapes. Clean improvement.
• Test suite trimmed: Removed test_sphere_count_respects_budget, test_cache_key_differs_for_different_meshes, test_dispatch_falls_back_when_obj_is_none, and test_mesh_zero_loss_separated_cylinders. These removals look intentional (simplified scope / covered elsewhere), though removing cache-key differentiation test reduces regression coverage on the caching layer.

Remaining Observations

• Findings #3–#5 from original review remain unaddressed (low priority, suggestions only).
• The new _get_cpu_mesh_manager uses hasattr check — works fine but Optional attribute initialized in __init__ would be more explicit.

Overall: Good incremental improvement. The two main warnings from the initial review are resolved. No new concerns.

---

Update (729d892c): Reviewed incremental changes since 5e86ed0a.

Changes in this push (2 files)

1. relation_loss_strategies.py — Added parent_pos_resolved.expand(batch_size, -1) before the per-batch loop. This fixes a shape mismatch when parent_pos_resolved is not already batch-expanded (e.g., single parent broadcast to multiple children). Correct fix.

2. warp_mesh_manager.py — Wrapped getattr(obj, "scale", ...) in tuple() for cache key computation. This prevents unhashable types (e.g., numpy arrays or torch tensors returned by .scale) from breaking the dict lookup. Necessary bugfix.

Assessment

Both changes are small, targeted bugfixes. No new concerns introduced. All previous suggestions (#3–#5) remain low-priority and unaddressed.

[greptile-apps]

Greptile Summary

This PR introduces mesh-based non-collision constraints as a new CollisionMode.MESH path alongside the existing AABB solver. Sphere decomposition (greedy_sphere_decomposition) approximates each object's geometry, and differentiable Warp SDF queries against BVH-accelerated meshes replace the volume-overlap loss for pairs where both objects have collision meshes; AABB remains the fallback for mesh-less pairs.

• New infrastructure: WarpMeshAndSphereCache manages BVH construction and sphere caching; MeshPairCache / MeshPairEntry batch all per-pair data for a single vectorised kernel launch per environment; warp_sdf_kernels.py provides a torch.autograd.Function bridge so gradients flow back through Warp\u2019s SDF into the Adam optimizer.
• Solver/placer integration: RelationSolver._compute_no_overlap_loss_mesh runs a per-env loop with a yaw-aware AABB broadphase gate, while ObjectPlacer._validate_no_overlap_mesh performs the same check at placement-validation time using a separate CPU-device mesh manager.
• Configuration unified: ObjectPlacerParams is now the primary config source; arena_env_builder reads arena_env.placer_params and the --random_yaw_init CLI flag is removed in favour of setting ObjectPlacerParams.random_yaw_init directly.

Confidence Score: 2/5

Not safe to merge: multiple correctness defects in the collision loss, validation, and geometry extraction paths from prior review rounds remain unaddressed.

The world-transform application in extract_trimesh_from_usd silently drops translation for non-identity prims. clamp_sdf_sentinel inverts its stated intent, inflating loss for every degenerate BVH hit. The AABB gate in _validate_placement rejects valid MESH-mode layouts. _rotate_bbox_extents rotates around bbox center instead of object origin. The anchor parent_pos_resolved shape mismatch crashes for batch_size > 1.

isaaclab_arena/utils/usd_helpers.py, isaaclab_arena/relations/warp_sdf_kernels.py, isaaclab_arena/relations/object_placer.py, isaaclab_arena/relations/relation_solver.py, isaaclab_arena/relations/relation_loss_strategies.py

Important Files Changed

Filename	Overview
isaaclab_arena/relations/warp_sdf_kernels.py	New file: Warp SDF kernels and autograd bridge. clamp_sdf_sentinel has inverted logic (clamping to 0 produces spurious positive loss instead of zero loss for degenerate BVH hits).
isaaclab_arena/relations/warp_mesh_manager.py	New file: BVH mesh cache and greedy sphere decomposition. Sphere candidates are deterministically biased toward larger faces; scale from a list would be unhashable.
isaaclab_arena/relations/relation_solver.py	Major additions: _compute_no_overlap_loss_mesh, _prepare_mesh_collision_cache, _collect_mesh_pairs, _finalize_mesh_cache. Pre-existing flagged issues: null-dereference on _mesh_manager, _rotate_bbox_extents uses bbox-center not object-origin.
isaaclab_arena/relations/object_placer.py	Major additions: _validate_no_overlap_mesh, _spheres_penetrate_mesh, _pair_aabb_overlaps, _centers_in_target_frame. Pre-existing flagged issues: AABB gate rejects valid MESH-mode placements, AABB fallback uses stale default bbox.
isaaclab_arena/relations/mesh_pair_cache.py	New file: typed dataclass holding precomputed per-pair collision data. Well-structured with comprehensive post_init validation; pair_sphere_count stored as float32 is non-idiomatic but functional.
isaaclab_arena/utils/usd_helpers.py	New extract_trimesh_from_usd: consumes all mesh prims without filtering for collision vs visual purpose; world-transform application drops translation for non-identity transforms.
isaaclab_arena/environments/arena_env_builder.py	Refactored placement config to accept placer_params from arena_env; previously flagged merge-conflict markers resolved; --random_yaw_init CLI flag removed.
isaaclab_arena/relations/relation_loss_strategies.py	Minor cleanups; pre-existing flagged issues: _compute_mesh_loss ignores child orientation, anchor-case parent_pos_resolved unsqueeze causes IndexError for batch_size > 1.
isaaclab_arena/tests/test_mesh_collision.py	New test file: covers sphere decomposition, Warp mesh caching, AABB loss, and end-to-end mesh-mode solver. Tests are focused and correct.
isaaclab_arena/relations/collision_mode.py	New file: simple CollisionMode enum with BBOX and MESH variants. Clean and correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant OP as ObjectPlacer
    participant RS as RelationSolver
    participant WM as WarpMeshAndSphereCache
    participant K as Warp SDF Kernel

    OP->>RS: solve(objects, initial_positions, env_bboxes, orientations)
    RS->>WM: get_collision_mesh per object
    RS->>WM: get_query_spheres per mesh
    RS->>WM: get_warp_mesh per mesh
    WM-->>RS: MeshPairCache forward + reverse

    loop Adam iterations
        RS->>K: multi_mesh_sdf(active_centers, mesh_ids)
        K-->>RS: sdf_values with autograd gradients
        RS->>RS: relu(radii + clearance - sdf) loss
        RS->>RS: aabb loss for mesh-less pairs
        RS->>RS: backward + optimizer step
    end

    RS-->>OP: solved positions
    OP->>OP: validate AABB for mesh-less pairs
    OP->>OP: validate sphere-to-SDF for mesh pairs

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant OP as ObjectPlacer
    participant RS as RelationSolver
    participant WM as WarpMeshAndSphereCache
    participant K as Warp SDF Kernel

    OP->>RS: solve(objects, initial_positions, env_bboxes, orientations)
    RS->>WM: get_collision_mesh per object
    RS->>WM: get_query_spheres per mesh
    RS->>WM: get_warp_mesh per mesh
    WM-->>RS: MeshPairCache forward + reverse

    loop Adam iterations
        RS->>K: multi_mesh_sdf(active_centers, mesh_ids)
        K-->>RS: sdf_values with autograd gradients
        RS->>RS: relu(radii + clearance - sdf) loss
        RS->>RS: aabb loss for mesh-less pairs
        RS->>RS: backward + optimizer step
    end

    RS-->>OP: solved positions
    OP->>OP: validate AABB for mesh-less pairs
    OP->>OP: validate sphere-to-SDF for mesh pairs

Loading

_{Reviews (29): Last reviewed commit: "improve docstrings" | Re-trigger Greptile}

[alexmillane]

First partial review.

Looks good. I haven't got to the warp mesh based stuff.

[alexmillane]

Another partial review

[alexmillane]

Partial review

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

This adds CollisionMode.MESH (greedy sphere decomposition + differentiable Warp SDF queries) as an opt-in alternative to the AABB no-overlap path, with the validator mirroring the solver, broadphase gating, and yaw handling unified across both modes. The default stays BBOX, the structure (typed MeshPairCache/MeshPairEntry, forward/reverse directed pairs) is clean, and test coverage is genuinely strong. Two things are worth a closer look before merge: the mesh-only dependencies are now imported at the top of the pure relations/ domain, and the mesh-extraction and bbox paths compose scale differently (your own USD-scale test encodes the divergence).

Design, Boundaries & Scope

The architecture keeps the placement/relation-solving domain (relations/) sim-agnostic and import-light on purpose. This PR makes warp, trimesh, and isaaclab.utils.math unconditional top-level imports in that layer (relation_solver.py, and object_placer.py → warp_mesh_manager.py), so importing the solver/placer — and running the pure-Python placement tests — now hard-requires all three even when collision_mode=BBOX (the default). Since MESH is opt-in, could these imports be deferred into the MESH code paths (lazy import inside the mesh methods / WarpMeshAndSphereCache) so the default path and the domain layer stay lean? See the inline note on the isaaclab.utils.math import for a concrete way to drop at least that one.

Findings

See inline comments.

Test Coverage

Coverage is thorough: sphere-decomposition coverage, multi-mesh routing (good regression for the "all points hit mesh 0" failure), backward-gradient direction, sentinel-fails-validation, broadphase skip/no-skip, batch-size-2, target-only and anchor-initial_pose yaw, and end-to-end placement. The USD-scale tests correctly use the inner/outer run_simulation_app_function pattern. One caveat: test_usd_scale_helpers.py encodes the mesh-vs-bbox scale divergence as expected behavior — if the inline scale finding is correct, those assertions (and the test's "correct" framing) should be revisited.

Verdict

Minor fixes needed

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

This PR adds an opt-in mesh-based no-overlap mode (CollisionMode.MESH) built on greedy sphere decomposition + differentiable Warp SDF queries, unified with the existing AABB path, plus typed caches (MeshPairCache/MeshPairEntry) and yaw-aware transforms. The feature is well-factored and the test suite is genuinely thorough (sphere coverage, multi-mesh routing regression, backward-gradient, broadphase, target/anchor yaw, SDF sentinel). Two things are worth a look before merge: a default-path change in the env-build placement solve, and the solver layer picking up new top-level Warp/Isaac Lab imports.

Design, Boundaries & Scope

🟡 Lean-dependency regression in the solver layer. relations/relation_solver.py was previously torch-only; it now hard-imports warp and isaaclab.utils.math at module top level (and pulls in the warp mesh/SDF modules), so even the default BBOX path drags in Warp + Isaac Lab on import. The architecture deliberately keeps the relation-solving/placement domain lean. The two quat_apply/quat_apply_inverse uses only ever apply a pure-Z yaw — the exact rotation ObjectPlacer._centers_in_target_frame already does with cos/sin — so the Isaac Lab dependency looks avoidable, and the Warp/SDF imports could be deferred into the MESH code paths. See inline note.

Findings

🟡 Warning: environments/relation_solver_interface.py:38 — Drops the save_position_history=False, verbose=False overrides the old code applied; the env-build placement solve now runs with RelationSolverParams defaults (verbose=True, save_position_history=True) for everyone. (inline)
🟡 Warning: relations/relation_solver.py:15 — New top-level warp + isaaclab.utils.math imports in the previously-lean solver. (inline)
🔵 Improvement: relations/relation_solver.py:668 — Two adjacent identical if collision_mode == MESH: blocks can be merged. (inline)

Test Coverage

Strong. New unit + integration tests cover sphere decomposition coverage, Warp mesh caching, multi-mesh routing (the test_multi_mesh_sdf_distinct_meshes regression is a nice catch), backward gradients, broadphase culling, source/target/anchor yaw, and the SDF no-face sentinel path failing loudly. The USD-scale tests correctly use the inner/outer run_simulation_app_function pattern. Warp-dependent tests are gated behind @requires_warp; please confirm the non-@requires_warp tests still land in Phase 1 as intended.

One note unrelated to a diff line: the PR description says "CLI --collision_mode mesh is a fallback," but no --collision_mode arg is added to the CLI, and --random_yaw_init was removed from it. So there's now no CLI path to enable either mesh mode or random-yaw — both are env-level placer_params-only. If that's the intent, the description should be updated; if not, the CLI fallback is missing.

Verdict

Minor fixes needed

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

Adds an opt-in CollisionMode.MESH path (greedy sphere decomposition + differentiable Warp SDF queries) alongside the existing AABB no-overlap path, with AABB fallback for mesh-less pairs and a matching validator. The mesh code is correctly isolated as pure geometric compute in relations/ (no live env / sim stepping), defaults stay on BBOX, and the SDF/yaw/anchor logic is backed by a thorough test suite. The implementation looks solid; the notes below are about a bundled CLI change and a few clarity/efficiency points.

Design, Boundaries & Scope

• --random_yaw_init CLI flag removed (isaaclab_arena/cli/isaaclab_arena_cli.py) — this is a user-facing change to an existing entry point that isn't mentioned in the PR summary (which is scoped to mesh collision). After this PR, random_yaw_init is reachable only by constructing an environment-level placer_params; anyone who passed --random_yaw_init on the CLI will now hit an "unrecognized argument" error. The builder's default ObjectPlacerParams(...) only sets placement_seed/resolve_on_reset, so there's no CLI override path. Was dropping the flag (rather than keeping it as a CLI override into placer_params) intentional? Worth a call-out either way.
• PR description vs. code: the description states "CLI --collision_mode mesh is a fallback," but I don't see a --collision_mode argument anywhere — MESH mode appears reachable only by building placer_params in code. If the CLI fallback was intended it's missing; otherwise the description is slightly stale.

Findings

🔵 Improvement: relations/warp_sdf_kernels.py:122 — clamp_sdf_sentinel docstring describes the opposite of the behavior (see inline).
🔵 Improvement: relations/warp_sdf_kernels.py:132 — single-mesh and multi-mesh kernels/autograd functions are near-duplicates (see inline).
🔵 Improvement: relations/relation_solver.py:489 — per-env Python loop in the mesh loss runs every optimization iteration (see inline).

Test Coverage

Coverage is strong and well-targeted: sphere-decomposition coverage, SDF backward gradients, multi-mesh routing (with an explicit regression note), broadphase gating, batch-size>1, anchor/target yaw via both orientations and baked initial_pose, the sentinel-must-fail-validation case, and end-to-end placement. The new sim tests in test_usd_scale_helpers.py use the run_simulation_app_function inner/outer pattern with deferred imports, matching the existing test_usd_helpers.py convention; the Warp-only tests correctly gate on @requires_warp and need no sim app. Regression tests are included for the bundled marker-yaw fix. Nothing material missing.

Verdict

Minor fixes needed — confirm the --random_yaw_init CLI removal is intended; the rest are clarity/efficiency improvements.

[alexmillane]

It's looking a lot better.

I have a few comments.

I have approved so once you solve them, feel free to merge.