Take gains for `InverseDynamicsDriver` as parameters

siddancha commented 1 month ago

Problem

Currently, the PID gains of the InverseDynamicsController are fixed to a certain default (kp=100, ki=1, kd=20).
Taking the PID gains as parameters is marked as a TODO item.

Solution

This PR implements taking parameters of the InverseDynamicsDriver as parameters.

This is specified by the user as a mapping from the names of the actuator joins to the PID gains, like so:

model_drivers:
spot: !InverseDynamicsDriver
  gains:
    arm_sh0: {kp: 1000, ki: 10, kd: 50}
    arm_sh1: {kp: 1000, ki: 10, kd: 50}
    arm_el0: {kp: 1000, ki: 10, kd: 50}
    arm_el1: {kp: 1000, ki: 10, kd: 50}
    arm_wr0: {kp: 1000, ki: 10, kd: 50}
    arm_wr1: {kp: 1000, ki: 10, kd: 50}
    arm_f1x: {kp: 3000, ki: 20, kd: 50}

Backward compatibility

The user is allowed to set the gains of any subset of actuators. Any unspecified actuators will use the default gains. Therefore, existing scenario files that use the InverseDynamicsDriver without specifying gains will continue using the defaults for all actuators.

This change is

siddancha commented 1 month ago

If you believe those gains are better for the spot inverse dynamics driver, can you please go ahead and add them to spot_with_arm_and_floating_base_actuators.scenario.yaml? Both to improve Spot, but because I want some example in CI to provide some level of coverage on this new code.

Added David's gains to the scenario file.

The linter also failed. https://github.com/RussTedrake/manipulation/blob/master/Developers.md#please-install-the-pre-commit-hooks

I completely missed the developer instructions! Fixed lint errors. I added a pointer to the developer instructions in the main README to help other contributors see it. Let me know if you'd like me to undo this.

RussTedrake commented 1 month ago

fyi.. just pushed the updated pip wheels, too.

RussTedrake / manipulation