-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix reset functions #1416
base: main
Are you sure you want to change the base?
Fix reset functions #1416
Conversation
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.
This looks good. Thanks for the added functionality
Signed-off-by: Jean Tampon <[email protected]>
@@ -87,6 +87,12 @@ class EventCfg: | |||
"""Configuration for events.""" | |||
|
|||
# reset | |||
reset_to_default = EventTerm( |
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.
would this change affect other environments that use reset_joints_by_offset? such as ant, humanoid, cabinet
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.
This environment file isn't a place to have this feature as a unit test IMO. We're changing the way the environment operates.
|
||
# set into the physics simulation | ||
asset.write_joint_state_to_sim(joint_pos, joint_vel, env_ids=env_ids) | ||
asset.write_joint_state_to_sim(joint_pos_all, joint_vel_all, env_ids=env_ids) |
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 this a slower operation since you are copying over the data twice (first here by setting into joint_pos_all
and then it is again done inside the method). Why not keep it cleaner and just operate over the asset_cfg.joint_indices
in the whole thing?
Something like:
# extract the used quantities (to enable type-hinting)
asset: Articulation = env.scene[asset_cfg.name]
command_term: PhaseCommand = env.command_manager.get_term(command_name)
# broadcast env_ids if needed to allow double indexing
if env_ids != slice(None) and asset_cfg.joint_ids != slice(None):
env_ids = env_ids[:, None]
# read state
....
# clamp joint pos to limits
joint_pos_limits = asset.data.soft_joint_pos_limits[env_ids, asset_cfg.joint_ids]
joint_pos = joint_pos.clamp_(joint_pos_limits[..., 0], joint_pos_limits[..., 1])
# clamp joint vel to limits
joint_vel_limits = asset.data.soft_joint_vel_limits[env_ids, asset_cfg.joint_ids]
joint_vel = joint_vel.clamp_(-joint_vel_limits, joint_vel_limits)
# make sure env ids dimensions are compatible
if env_ids != slice(None) and asset_cfg.joint_ids != slice(None):
env_ids = env_ids.squeeze(1)
# set into the physics simulation
asset.write_joint_state_to_sim(joint_pos, joint_vel, env_ids=env_ids, joint_ids=asset_cfg.joint_ids)
Description
This PR introduces a
reset_joints_to_default
function that reset all joints of a robot to their default states.It also introduces the possibility to target specific joints in the
reset_joints_by_scale
andreset_joints_by_offset
.Fixes #980
Type of change
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there