Add safety limits on relative action target #373

alexander-soare · 2024-08-22T08:59:04Z

What this does

At the script level, add a mechanism and option for capping the size of the relative joint space target provided by the policy. This prevents a wild policy from burning the motors.

For a future PR

We decided to leave the default value of the safety limit to None (meaning no limit) in order to avoid friction for the user who doesn't know about this parameter. Here @Cadene suggests a follow up if we want to default the safety limit to a reasonable value:

How it was tested / how to check out and try

Run a policy rollout like:

python lerobot/scripts/control_robot.py record \
  --robot-path lerobot/configs/robot/koch_.yaml \
  --fps 30 \
  --warmup-time-s 5 \
  --episode-time-s 600 \
  --reset-time-s 60 \
  --num-episodes 1 \
  --pretrained-policy-name-or-path pretrained_models/act_bluecube_pinkcylinder_x10_chunk30_step200000 \
  --push-to-hub 0 \
  --force-override 1

Then observe the output. You might see lines like:

WARNING 2024-08-22 09:58:34 ol_robot.py:506 Relative action magnitude had to be clamped to be safe.
  requested relative action target: tensor([[-10.2270,   2.7441,  -4.4867,  -1.0570,   8.0041,  -1.0996]])
    clamped relative action target: tensor([[-10.0000,   2.7441,  -4.4867,  -1.0570,   8.0041,  -1.0996]])

especially if you go OOD.

aliberts

Thanks @alexander-soare, I just tried it and it works on my arm.

I was wondering however if we shouldn't put that action capping logic in the robot's config & class rather than in the record script. My reasons for wanting this are:

I'd like to have that safety feature in general when operating the robot, not just when running a policy. For instance if I start the teleop script when my follower arm is in resting position while the leader is at a 180° position, that follower is going to do a sudden jump which is not very safe (both for the user and the motors/arm). Another example I can think of is if I (human) do a sudden move during teleop, for any reason.
The values you defined in policy_action_safety_cap seem to be fundamentally associated with the motors/robot rather than the policy.

What do you think?

Cadene

Same comment as simon. We should move it to dynamixel.py no?

alexander-soare · 2024-08-23T11:00:23Z

lerobot/common/robot_devices/robots/koch.py

+            this_action = action[from_idx:to_idx]
+
+            if self.config.max_relative_target is not None:
+                max_relative_target = torch.tensor(self.config.max_relative_target)


FYI @Cadene @aliberts. This is not related to this PR, but see how I need to juggle tensors and numpy arrays here? Another reason we should make this class only interface with numpy arrays.

alexander-soare · 2024-08-23T11:00:49Z

lerobot/common/robot_devices/robots/koch.py

@@ -164,6 +165,7 @@ class KochRobotConfig:
    leader_arms: dict[str, MotorsBus] = field(default_factory=lambda: {})
    follower_arms: dict[str, MotorsBus] = field(default_factory=lambda: {})
    cameras: dict[str, Camera] = field(default_factory=lambda: {})
+    max_relative_target: list[float] | None = None


Should we choose a sensible default or default to None? @aliberts @Cadene

alexander-soare · 2024-08-23T11:02:00Z

lerobot/common/robot_devices/robots/koch.py

@@ -474,21 +498,53 @@ def capture_observation(self):
            obs_dict[f"observation.images.{name}"] = torch.from_numpy(images[name])
        return obs_dict

-    def send_action(self, action: torch.Tensor):
-        """The provided action is expected to be a vector."""
+    def send_action(self, action: torch.Tensor, follower_names: list[str] | None = None):


@aliberts @Cadene I had to make this change (adding follower_arms kwarg) as teleop_step wants to set the follower arms one by one and get a dt for each. We could have also just not gotten a dt for each arm.

alexander-soare · 2024-08-23T11:02:52Z

lerobot/common/robot_devices/robots/koch.py

+            if self.config.max_relative_target is not None:
+                max_relative_target = torch.tensor(self.config.max_relative_target)
+                # Cap relative action target magnitude for safety.
+                current_pos = torch.tensor(self.follower_arms[name].read("Present_Position"))


@aliberts @Cadene I'm not a fan of having to read the present position here. It adds time to the loop. This was one advantage of having this logic in the outer loop (not having to read the present position as we already knew what it was).

alexander-soare · 2024-08-23T11:03:26Z

lerobot/common/robot_devices/robots/koch.py

+
+        Args:
+            action: tensor containing the concatenated joint positions for the follower arms.
+            follower_names: Pass follower arm names to only control a subset of all the follower arms.


@Cadene @aliberts pleasee read the logic for this carefully and test if you can, as I don't have multiple arms.

Cadene

Could we allow to have one value? max_relative_target: int | list[int]

max_relative_target=[10, 10, 10, 10, 10, 15],

That way it's a bit more automated.

alexander-soare · 2024-08-23T11:23:54Z

Could we allow to have one value? max_relative_target: int | list[int]
max_relative_target=[10, 10, 10, 10, 10, 15],
That way it's a bit more automated.

@Cadene I found that the gripper needs a higher limit, and it's also okay because there's little load. Also, I found that in general I sometimes want to tweak the other motors. Mostly because of the gripper though, I usually don't want to use a single value. Anyway. I'll put it in, but won't suggest it in the docs.

alexander-soare · 2024-08-23T11:30:21Z

lerobot/common/robot_devices/robots/koch.py

-                follower_goal_pos[name] = action[from_idx:to_idx].numpy()
-                from_idx = to_idx
+        for name in follower_names:
+            to_idx += len(self.follower_arms[name].motor_names)


@Cadene @aliberts do we really expect this to be anything other than 6? The problem is that max_relative_target has a fixed length and refers to the motors of one robot.

I have resolved this by adding a post_init validation to the config class.

Cadene · 2024-08-23T12:03:07Z

lerobot/configs/robot/koch.yaml

@@ -37,3 +37,4 @@ cameras:
    fps: 30
    width: 640
    height: 480
+max_relative_target: [10.0, 10.0, 10.0, 10.0, 10.0, 15.0]


I am sorry I was not specific enough. One value should be the default. What do you think?

Cadene

LGTM

Ideally we can add a little test. But not a blocker.

alexander-soare added 2 commits August 22, 2024 09:53

Add safety limits on relative action target

103ac92

slightly increase gripper limit

1868169

alexander-soare added ✨ Enhancement New feature or request 🌍 Real world Real-world robotics & controls labels Aug 22, 2024

alexander-soare added 3 commits August 22, 2024 11:53

fix negative capping

4b5da7a

actually use the safe_action

d8c49e6

squeeze

eebc218

aliberts reviewed Aug 22, 2024

View reviewed changes

Cadene reviewed Aug 22, 2024

View reviewed changes

alexander-soare added 4 commits August 23, 2024 08:34

remove clone

aeb176b

Merge remote-tracking branch 'upstream/main' into safe_action

fe04d4d

add documentation

52fb242

add documentation

94386b7

alexander-soare commented Aug 23, 2024

View reviewed changes

Cadene reviewed Aug 23, 2024

View reviewed changes

alexander-soare added 3 commits August 23, 2024 12:27

Merge remote-tracking branch 'upstream/main' into safe_action

c8942ab

make it possible to specify a catch-all integer

f83e6cc

add limits to koch.yaml

bee40cd

alexander-soare commented Aug 23, 2024

View reviewed changes

Cadene reviewed Aug 23, 2024

View reviewed changes

alexander-soare added 4 commits August 26, 2024 09:16

scalar max_relative_target in koch.yaml

e726a29

Merge remote-tracking branch 'upstream/main' into safe_action

2f59632

revision

3560f97

use __setattr__ instead of __post_init__ for validation

104e195

Cadene approved these changes Aug 26, 2024

View reviewed changes

alexander-soare merged commit 9ce98bb into huggingface:main Aug 26, 2024
6 checks passed

alexander-soare deleted the safe_action branch August 26, 2024 13:30

amandip7 pushed a commit to amandip7/lerobot that referenced this pull request Oct 10, 2024

Add safety limits on relative action target (huggingface#373)

85e35bf

menhguin pushed a commit to menhguin/lerobot that referenced this pull request Feb 9, 2025

Add safety limits on relative action target (huggingface#373)

55682f9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add safety limits on relative action target #373

Add safety limits on relative action target #373

alexander-soare commented Aug 22, 2024 •

edited

Loading

aliberts left a comment •

edited

Loading

Cadene left a comment

alexander-soare Aug 23, 2024

alexander-soare Aug 23, 2024

alexander-soare Aug 23, 2024

alexander-soare Aug 23, 2024

alexander-soare Aug 23, 2024

Cadene left a comment

alexander-soare commented Aug 23, 2024

alexander-soare Aug 23, 2024

alexander-soare Aug 26, 2024

Cadene Aug 23, 2024

Cadene left a comment

Add safety limits on relative action target #373

Add safety limits on relative action target #373

Conversation

alexander-soare commented Aug 22, 2024 • edited Loading

What this does

For a future PR

How it was tested / how to check out and try

aliberts left a comment • edited Loading

Choose a reason for hiding this comment

Cadene left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadene left a comment

Choose a reason for hiding this comment

alexander-soare commented Aug 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cadene left a comment

Choose a reason for hiding this comment

alexander-soare commented Aug 22, 2024 •

edited

Loading

aliberts left a comment •

edited

Loading