-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixes joint out of position limits terminations #2442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
out_of_upper_limits = torch.any(asset.data.joint_pos[:, asset_cfg.joint_ids] > bounds[1], dim=1) | ||
out_of_lower_limits = torch.any(asset.data.joint_pos[:, asset_cfg.joint_ids] < bounds[0], dim=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the implementation is correct since it is only operating over subset of joints.
# Per‑env‑per‑joint mask of violations | ||
joint_pos = asset.data.joint_pos # [N_envs, N_joints] | ||
joint_lower = asset.data.soft_joint_pos_limits[..., 0] | ||
joint_upper = asset.data.soft_joint_pos_limits[..., 1] | ||
violations = (joint_pos < joint_lower) | (joint_pos > joint_upper) | ||
|
||
joint_ids = asset_cfg.joint_ids if asset_cfg.joint_ids is not None else slice(None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it simpler to do it this way?
if asset_cfg.joint_ids is None:
asset_cfg.joint_ids = slice(None)
limits = asset.data.soft_joint_pos_limits[:, asset_cfg.joint_ids]
out_of_upper_limits = torch.any(asset.data.joint_pos[:, asset_cfg.joint_ids] > limits[..., 1], dim=1)
out_of_lower_limits = torch.any(asset.data.joint_pos[:, asset_cfg.joint_ids] < limits[..., 0], dim=1)
return torch.logical_or(out_of_upper_limits, out_of_lower_limits)
I don't see why we need to make a masking first. As long as any of the joints are out of their limits, the returned value should be True. If that's not the case, then could you please also share a repro for the issue? |
Description
This PR fixes shape‑handling bugs in the termination helpers
joint_pos_out_of_limit
andjoint_pos_out_of_manual_limit
:Root cause – both functions reduced across joints with
torch.any(dim=1)
before slicing withasset_cfg.joint_ids
, leaving a 1‑D tensor and triggering anIndexError: too many indices
.Fix – construct a single
violations
mask, optionally slice joints, then reduce withtorch.any(dim=1)
.This preserves correct shapes (
[num_envs]
) and works for any joint‑selection type (None
,slice
, list, or tensor).Additional improvements
asset_cfg
immutable by avoiding in‑place modification ofjoint_ids
.No new dependencies were introduced.
Fixes #2441
Type of change
Screenshots
Not applicable – logic‑only change.
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there