isaac-sim / IsaacLab

Unified framework for robot learning built on NVIDIA Isaac Sim
https://isaac-sim.github.io/IsaacLab
Other
1.62k stars 526 forks source link

[Bug Report] actuation_pd.py DCMotorCfg saturation allow a dictionary #608

Open Robokan opened 2 weeks ago

Robokan commented 2 weeks ago

If you are submitting a bug report, please fill in the following details and use the tag [bug].

Describe the bug

in the class DCMotorCfg in actuation_pd.py line 182

if self.cfg.saturation_effort is not None: self._saturation_effort = self.cfg.saturation_effort else: self._saturation_effort = torch.inf

should be:

    self._saturation_effort = self._parse_joint_parameter(self.cfg.saturation_effort, torch.inf)

A clear and concise description of what the bug is.

It crashed when using different saturations for different actuators.

Steps to reproduce

If the Go1 actuator is specified according to the spec sheet:

GO1_ACTUATOR_CFG = ActuatorNetMLPCfg( joint_names_expr=["._hip_joint", "._thigh_joint", "._calf_joint"], network_file=f"{ISAACLAB_NUCLEUS_DIR}/ActuatorNets/Unitree/unitree_go1.pt", pos_scale=-1.0, vel_scale=1.0, torque_scale=1.0, input_order="pos_vel", input_idx=[0, 1, 2], effort_limit={"._hip_joint": 23.7, "._thigh_joint": 23.7, "._calf_joint": 35.5}, # taken from spec sheet velocity_limit=30.0, # taken from spec sheet saturation_effort={"._hip_joint": 23.7, "._thigh_joint": 23.7, ".*_calf_joint": 35.5} # same as effort limit

It will crash complaining that dictionaries are not supported

Please try to provide a minimal example to reproduce the bug. Error messages and stack traces are also helpful.

System Info

Describe the characteristic of your environment:

Additional context

Add any other context about the problem here.

Checklist

Acceptance Criteria

Add the criteria for which this task is considered done. If not known at issue creation time, you can add this once the issue is assigned.

Mayankm96 commented 2 weeks ago

Although not a deal breaker for us to implement this functionality, is this something that conceptually makes sense?

To me, an actuator is a physical drive, and the limits are specific to that drive. If the robot has different drives, there should be different actuator models for them, which should be grouped into the actuators dictionary.

Robokan commented 2 days ago

In the case of the Go1 and Go2 they have the same actuators at each joint but at the knees they add a reduction gear. This effects the the torque and max speed of those actuators. I agree with your question. What is the best way to handle this. Should the knees have different actuator models in this case? Or just a way to make the adjustment on the actuator config itself?