Fix reversed argument order in H1JoystickGaitTracking action-rate penalty - #352
Open
rootkiller6788 wants to merge 1 commit into
Open
rootkiller6788 wants to merge 1 commit into
rootkiller6788 wants to merge 1 commit into
Conversation
…alty H1JoystickGaitTracking called _cost_action_rate with (info["last_act"], info["last_last_act"], action), but the signature is (act, last_act, last_last_act). The current action therefore never enters the first-derivative term c1=(act-last_act)^2, and the second-derivative term is computed about the wrong point. Every other locomotion env (g1, go1, t1, op3, berkeley_humanoid, ...) passes (action, info["last_act"], info["last_last_act"]).
rootkiller6788
marked this pull request as ready for review
September 3, 2026 05:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
In
mujoco_playground/_src/locomotion/h1/joystick_gait_tracking.py, theaction_ratereward for the registeredH1JoystickGaitTrackingenvironment was called with its three arguments in the wrong order:The method signature is
_cost_action_rate(self, act, last_act, last_last_act)and computes:So with the current call,
act=info["last_act"]andlast_act=info["last_last_act"]: the first-derivative penaltyc1measures the change between the two previous actions and completely ignores the action just taken, and the second-derivative termc2is computed about the wrong point.Every other locomotion environment with the same 3-argument helper passes the arguments in the intended order, e.g.
g1/joystick.py,go1/joystick.py,t1/joystick.py,op3/joystick.py,berkeley_humanoid/joystick.py:Impact
H1JoystickGaitTrackinghasaction_rate=-0.01in its reward config, so this penalty is active during training. Because the arguments are shifted, the penalty signal is wrong at every step: the current action is excluded from the first-derivative term, so the policy receives a mis-scoped action-rate cost.Fix
Reorder the arguments to
(action, info["last_act"], info["last_last_act"]), matching the method signature and every sibling environment.Verification
mujoco_playground/_src/locomotion/h1/joystick_gait_tracking.py:456-462) and that all ~7 sibling envs with the same 3-arg helper call it withactionfirst.H1JoystickGaitTrackingis a registered environment (_envs/_cfgsinmujoco_playground/_src/locomotion/__init__.py) andaction_rate=-0.01is non-zero in its default reward config.last_act/last_last_actare initialized inreset()and updated eachstep(), so the bug manifests on every transition.py_compilepasses on the edited file.