Unnecessary Conditional Check in camera_action Handling Logic

[fenneishi]

https://github.com/openai/Video-Pre-Training/blob/aed46b90e8db2332801feabd8be2de01f92c0ad2/run_inverse_dynamics_model.py#L103C1-L112C33

Issue Description

In the json_action_to_env_action function, the else branch for handling camera_action contains redundant logic that checks whether abs(camera_action[0]) > 180 and abs(camera_action[1]) > 180. These checks are unnecessary because, under the else condition, both camera_action[0] and camera_action[1] are guaranteed to be 0.

Code in Question:

else:
        if abs(camera_action[0]) > 180:
            camera_action[0] = 0
        if abs(camera_action[1]) > 180:
            camera_action[1] = 0

Reason for Redundancy:

Before reaching this else branch, camera_action[0] and camera_action[1] are set as follows:

camera_action[0] = mouse["dy"] * CAMERA_SCALER
camera_action[1] = mouse["dx"] * CAMERA_SCALER

When mouse["dx"] == 0 and mouse["dy"] == 0 (the condition for entering the else branch), both camera_action[0] and camera_action[1] are already 0. Therefore, the additional abs(camera_action) > 180 checks and assignments are redundant.

Suggested Fix

The logic for clamping camera_action should be moved outside the if-else block so that it applies universally, regardless of whether there is mouse input or not. This simplifies the flow and ensures the clamping logic is consistently applied:

camera_action[0] = mouse["dy"] * CAMERA_SCALER
camera_action[1] = mouse["dx"] * CAMERA_SCALER

if mouse["dx"] != 0 or mouse["dy"] != 0:
    is_null_action = False

camera_action[0] = 0 if abs(camera_action[0]) > 180 else camera_action[0]
camera_action[1] = 0 if abs(camera_action[1]) > 180 else camera_action[1]

[Miffyli]

Hey, sorry for the long delay! This repo is on a very slow maintanance attention 😅 .

When mouse["dx"] == 0 and mouse["dy"] == 0 (the condition for entering the else branch), both camera_action[0] and camera_action[1] are already 0. Therefore, the additional abs(camera_action) > 180 checks and assignments are redundant.

Hmm this does not seem to be true, as the "else" condition is triggered if either x or y axis is non-zero (in which case they might fall outside 180).

Regardless, a small update to remove the nested if-elses would, at the very least, made code more readable. I am happy to approve a PR if it is clean enough and properly tested.