anassinator / ilqr

Iterative Linear Quadratic Regulator with auto-differentiatiable dynamics models
GNU General Public License v3.0
371 stars 79 forks source link

alpha multiplication with k and K #10

Closed lgulich closed 5 years ago

lgulich commented 5 years ago

Hi anassinator

First off, thanks a lot for a beautifully written library!

I have noticed that in the following line you multiply alpha with (k[i] + K[i]...), whereas in the paper eq 12 (as well as in a comment of yours in the lines above) it is mentioned that alpha should only be multiplied with k[i].

https://github.com/anassinator/ilqr/blob/2184c4847fa363c0c67d6c203cf93452981109d8/ilqr/controller.py#L208

Is this done so on purpose?

anassinator commented 5 years ago

Yeah, I had issues converging with alpha being multiplied with k[i] only back then, but I might've inadvertently solved it since, so maybe it should be changed back. You can experiment with both and see what works better for your use case. Technically, multiplying K[i].dot(xs_new[i] - xs[i]) by alpha doesn't really make sense.

lgulich commented 5 years ago

Thanks for your response. I experimented with both and the results improved a lot when using the conventional way. Hence why I would recommend to change it back to this way, which is also how it is stated in the paper.

anassinator commented 5 years ago

Tested it again too, and it works well enough. Should be fixed in 677ce87