Conversation
zhx06
changed the title
zhx06
force-pushed
the
zxiao/bbox_batch_support
branch
2 times, most recently
from
9aa9df0 to
46af265
Compare
isaaclab_arena/utils/bounding_box.py
Outdated
| min_point: tuple[float, float, float] | ||
| """Local minimum extent (x, y, z) relative to object origin.""" | ||
| Stores min/max extents as (N, 3) tensors where N is the number of environments. | ||
| Properties return tuples/floats when N=1 and tensors when N>1. |
There was a problem hiding this comment.
I feel like the dual-mode return type is a reliability issue. Properties return tuple/float when N=1 and tensor when N>1. Code that works in single-env silently changes behavior when N increases. Recommend to always returning tensors
There was a problem hiding this comment.
Additionally we could simplify below some parts like self._min_point = self._to_batched_tensor(min_point),as its always a tensor (the input as well).
There was a problem hiding this comment.
This would require some updates downstream, but it would be worth it in my opinion
isaaclab_arena/utils/bounding_box.py
Outdated
| return value.unsqueeze(0).float() | ||
| return value.float() | ||
|
|
||
| def _format_output(self, tensor: torch.Tensor): |
There was a problem hiding this comment.
Suggestion to rename this function to what is formatted
There was a problem hiding this comment.
However can remove this if the class only handles tensors
isaaclab_arena/utils/bounding_box.py
Outdated
There was a problem hiding this comment.
I think this would break silently for N>1? When N>1, min_point returns a tensor and the subsequent torch.rand(3) arithmetic produces wrong-shaped results ((N,3) instead of (3,)).
I would suggest to move to single input and output type (tensors)
zhx06
force-pushed
the
zxiao/bbox_batch_support
branch
from
46af265 to
db21d7f
Compare
2 participants
Summary
Add batch dimension support to AxisAlignedBoundingBox
Detailed description
_validate_on_relationsZ boundary check.test_bounding_box.pyfor single-env and multi-env coverage.