Add radial bounds to PositionLimits

[lgulich]

Summary

• Extend generic PositionLimits with optional world-XY disk and annulus bounds.
• Preserve positional relation_loss_weight compatibility and provide deterministic, tensorized optimization behavior at the exact center of a lower radial bound.
• Cover constructor validation, batching, weights, solver behavior, generic YAML relation attachment, and public Python/YAML usage.

Motivation

Rectangular limits cannot directly express a radial placement region. This adds composable disk/annulus constraints without changing legacy rectangular behavior.

Rendered example

Fifty cubes constrained to the intersection of a 30–60 cm annulus and an 80 cm × 80 cm axial box, both centered on the tabletop.

[arena-review-bot]

🤖 Isaac Lab-Arena Review Bot

Summary

Adds optional world-XY disk/annulus bounds to the generic PositionLimits relation, intersecting with the existing rectangular bounds. The change is well-scoped and stays entirely in the sim-agnostic relations layer: new params default to None (legacy behavior unchanged), positional relation_loss_weight compatibility is preserved and tested, and the radial loss cleanly reuses the existing band / single-boundary primitives. I verified the gradient math, including the straight-through estimator that handles the inner-disk singularity and the center-outside-box case — both produce correct, deterministic descent directions.

Findings

🔵 Improvement: relation_loss_strategies.py — the escape-direction estimator (_radius_with_lower_bound_escape_direction) is the subtlest logic here, but its escape branch (annulus center outside the axial box) is only exercised indirectly by the solver integration test. A focused unit test asserting the steered gradient direction would lock the behavior. See inline comment.

Test Coverage

Strong. Constructor validation (incl. the invalid-radial parametrized cases), batching, weight scaling, annulus / radius-min-only losses, the at-center gradient, axis+radial composition, generic YAML attachment, and two solver integration tests are all covered. These are pure-compute (no SimulationApp), so the inner/outer sim pattern correctly does not apply. Only gap noted above.

Verdict

Ship it

[greptile-apps]

Greptile Summary

This PR extends PositionLimits with optional world-XY disk and annulus bounds (center_x, center_y, radius_min, radius_max) composable with existing rectangular bounds, adds a custom gradient-steering helper to break symmetry when an object is trapped at the radial center, and covers the feature with construction-time validation, unit tests, gradient tests, solver integration tests, YAML round-trip tests, and updated documentation.

• relations.py: Four new keyword-only parameters are appended after relation_loss_weight, preserving positional-argument compatibility; five new assert statements enforce radial preconditions at construction time.
• relation_loss_strategies.py: compute_loss adds a radial loss block that composes additively with the per-axis losses, and _radius_with_lower_bound_escape_direction injects a well-defined gradient direction when the object is inside the excluded inner disk.
• Tests and docs: Test suite covers loss values, batch shapes, weight scaling, gradient direction at the exact center, mixed-batch correctness, composed axis+radial loss, and two Adam-solver integration tests.

Confidence Score: 4/5

Safe to merge; the radial path is off by default and leaves existing rectangular-only relations completely unaffected.

The gradient-steering helper is non-trivial but is validated by targeted gradient tests and two end-to-end solver tests. Two small gaps exist: center_x/center_y supplied without radial bounds are silently ignored rather than rejected, and the center_radius minus epsilon formula has a theoretical path to a negative intermediate value for non-float32 dtypes. Neither represents a present data-loss or correctness regression.

relation_loss_strategies.py contains the most complex new logic (_radius_with_lower_bound_escape_direction) and would benefit from a second read; relations.py has the silent no-op for center coordinates without radial bounds.

Important Files Changed

Filename	Overview
isaaclab_arena/relations/relations.py	Extends PositionLimits.init with center_x, center_y, radius_min, radius_max; adds correct construction-time assertions for radial bounds; new params appended after relation_loss_weight preserving positional-argument compatibility. Minor gap: no guard against providing center coordinates without any radial bound.
isaaclab_arena/relations/relation_loss_strategies.py	Adds radial loss computation to PositionLimitsLossStrategy.compute_loss and a static _radius_with_lower_bound_escape_direction helper. Logic and gradient routing are correct; one minor floating-point edge case with the center_radius minus epsilon formula for non-float32 dtypes.
isaaclab_arena/tests/test_position_limits.py	Adds thorough unit and integration tests covering constructor validation, batch shape, weight scaling, gradient correctness at the exact center, mixed-batch symmetry breaking, composed axis+radial loss, and two solver integration tests.
isaaclab_arena/tests/test_arena_env_graph_spec.py	Adds test verifying end-to-end YAML to Python conversion of radial params. No issues.
docs/pages/concepts/concept_object_placement.rst	Adds PositionLimits documentation with Python/YAML examples, parameter table, and loss-function entry. Accurately reflects the implementation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["compute_loss"] --> B{radius bound set?}
    B -- No --> C[Axis-only loss]
    B -- Yes --> D["Compute radial_delta and radius"]
    D --> E{Both bounds?}
    E -- Yes --> F["_radius_with_lower_bound_escape_direction"]
    E -- No --> G{radius_min only?}
    G -- Yes --> F
    G -- No --> H["single_boundary_linear_loss(radius, radius_max)"]
    F --> I{escape_components non-zero?}
    I -- Yes --> J["feasibility_steered_radius"]
    J --> K["torch.where(inside_lower_bound, steered, radius)"]
    I -- No --> L{at_center?}
    L -- Yes --> M["center_radius with +x gradient"]
    L -- No --> N["radius standard gradient"]
    M --> O["torch.where(at_center, center_radius, radius)"]
    N --> O
    K --> P["band or boundary loss"]
    O --> P
    H --> P
    C --> Q["total_loss x weight"]
    P --> Q

Loading

_{Reviews (1): Last reviewed commit: "Fix combined annulus and axial descent" | Re-trigger Greptile}

[alexmillane]

Couple of comments.

Both of them relate to the combined/separate treatment of box and cylindrical constraints.

Clearly some thinking has gone into how these two interact. I'm wondering if the complexity added by handling this due to realistic issues we're likely to face.

[alexmillane]

What do you think about separating this into two classes:

• PositionLimitsBox
• PositionLimitsCylindrical

That way the meaning of these (many) constructor arguments is separated and clear for the user. Right now they're mixed up and a bit difficult to determine what is used where.

[alexmillane]

Can we get a comment on each branch here.

• annulus
• keep-out of circle
• stay inside circle.

[alexmillane]

Is this a practical scenario? I.e. a scenario that we're likely to run into in situations that are not already degenerate?

Let's talk about this offline.