search

google-deepmind / mujoco_mpc

Real-time behaviour synthesis with MuJoCo, using Predictive Control
https://github.com/deepmind/mujoco_mpc
Apache License 2.0
913 stars 135 forks source link

Bug in iLQS: dim_actions and dim_parameters not updated. #264

Closed nimrod-gileadi closed 5 months ago

nimrod-gileadi commented 6 months ago

On this line, local variables dim_actions and dim_parameters are declared, which means that the assignment doesn't affect the member variables of the same name, so they're always zero.

This has two effects:

  1. Later down that function, mju_mulMatTMat is called with matrix sizes of 0, so there's no mapping from the iLQG nominal plan to the sampling nominal plan.
  2. The spline mapping computation happens every time, instead of just once.

The fix is simple, removing the int from the two lines. I tried it and it looks fine to me, but I saw no obvious impact on the performance of iLQS, so I wanted to check with Taylor if there's some model where this could have a significant impact, and to validate that recomputing the spline mapping is not necessary.

thowell commented 6 months ago

Good catch!

This change should only improve tasks that use this planner. I tried the fix and things seem good to me.