search

RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.29k stars 1.26k forks source link

Confusing name for LinearQuadraticRegulator #11724

Open guzhaoyuan opened 5 years ago

guzhaoyuan commented 5 years ago

I was confused by builder.AddSystem<LinearQuadraticRegulator>(...) not working.

The LinearQuadraticRegulator actually creates an AffineSystem and returns a unique_ptr of the AffineSystem. LinearQuadraticRegulator is not the constructor for the AffineSystem.

It would be better to call the function MakeLinearQuadraticRegulator.

image

jwnimmer-tri commented 5 years ago

@RussTedrake If I'm reading it correctly, you added LQR in #4041. Could you weigh in on the rename suggestion?

Note that the overload set for LinearQuadraticRegulator includes some that return a System and some that return a LQRResult. Possibly we should use different names for those two cases?

RussTedrake commented 5 years ago

It's an interesting case -- in controls "LQR" is used frequently to refer to the mathematical problem/formulation; though I agree that technically that should be "LQR problem" and "LQR" should refer to the controller/gains that are the result of solving the problem.

I'm ok with changing the methods that return a system to "MakeLinearQuadraticRegulator". I think leaving the ones that return LQRResult as simply "LinearQuadraticRegulator" is probably right, too.