[fix] fix using bugs of the "get_aligned_pose" and "_prepare_warpping" function#217
Conversation
(1) fix the get_aligned_pose and get_changed_pose function (2) fix support for non-NumPy affordance_datas in the _prepare_warpping function.
There was a problem hiding this comment.
Pull request overview
This PR fixes pose-alignment logic in get_aligned_pose / get_changed_pose and makes action-bank “warpping” preparation more robust when env.affordance_datas contains non-NumPy values.
Changes:
- Fix
get_aligned_poseto respect the providedpose_axisinstead of always using the Z axis. - Fix
get_changed_pose(..., change_mode="align")to propagate the returned pose consistently with other change modes. - Prevent
_prepare_warppingfrom raising when an affordance value does not define.ndim.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
embodichain/lab/gym/utils/misc.py |
Corrects axis selection in pose alignment and updates align-mode pose propagation. |
embodichain/lab/gym/envs/action_bank/configurable_action.py |
Hardens _prepare_warpping to tolerate affordance values without .ndim. |
Comments suppressed due to low confidence (1)
embodichain/lab/gym/envs/action_bank/configurable_action.py:701
- This change is intended to support non-NumPy
affordance_datasvalues (by avoiding.ndimAttributeError), but there’s no regression test covering_prepare_warppingwith values that lackndim(e.g., Python scalars/objects) or with 3D affordance data that should be sliced to the first arena. Adding a unit test intests/gym/action_bank/test_configurable_action.pythat constructs a fake env withaffordance_datascontaining (1) a scalar and (2) a 3D tensor/ndarray would help prevent future breakage.
def _prepare_warpping(self, env):
if hasattr(env, "affordance_datas"):
for affordance_name, affordance_value in env.affordance_datas.items():
# NOTE: take only first arena's affordance data
if hasattr(affordance_value, "ndim") and affordance_value.ndim == 3:
affordance_value = affordance_value[0]
if isinstance(affordance_value, torch.Tensor):
affordance_value = np.asarray(affordance_value.cpu())
env.affordance_datas[affordance_name] = affordance_value
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rotation_axis = np.cross(pose_to_change[:3, pose_axis], align_vector) | ||
| rotation_axis_norm = np.linalg.norm(rotation_axis) | ||
| if rotation_axis_norm >= 1e-5: | ||
| rotation_axis = rotation_axis / rotation_axis_norm | ||
| rotation_angle = np.arccos(pose_to_change[:3, 2].dot(align_vector)) | ||
| rotation_angle = np.arccos(pose_to_change[:3, pose_axis].dot(align_vector)) | ||
| pose_to_change[:3, :3] = ( |
There was a problem hiding this comment.
rotation_angle is computed via np.arccos(pose_to_change[:3, pose_axis].dot(align_vector)) without normalizing align_vector (and without clipping the dot into [-1, 1]). If align_vector is not unit-length or there is minor floating-point drift, this can produce NaNs from arccos, breaking the alignment. Consider normalizing align_vector (and handling zero-norm) and clipping the dot product before calling arccos.
Description
Type of change
Checklist
black .command to format the code base.