Skip to content

doc inconsistency about return type of lqr - matrix vs array #418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
jerabaul29 opened this issue Jun 12, 2020 · 1 comment · Fixed by #476 or #477
Closed

doc inconsistency about return type of lqr - matrix vs array #418

jerabaul29 opened this issue Jun 12, 2020 · 1 comment · Fixed by #476 or #477
Assignees
@murrayrm
Labels
documentation
Milestone
0.8.4

Comments

@jerabaul29
Copy link

I think the doc about the return type of the lqr function looks inconsistent:

https://python-control.readthedocs.io/en/0.8.1/generated/control.lqr.html

states that K is a 2-d array. But it looks a lot like a matrix actually :)

>>> K, _, _ = control.lqr(np.array([1]), np.array([1]), np.array([1]), np.array([1]))
>>> K
matrix([[2.41421356]])

This is Python 3.6.9, control 0.8.3, Ubuntu 18.04 defaults.

This leads to possibly subtle bugs:

https://stackoverflow.com/questions/62342225/how-to-squeeze-when-numpy-squeeze-does-not-seem-to-squeeze?

Do you think it may be a good idea to 1) update the doc 2) maybe change from matrix to array output altogether? :)
@murrayrm
Copy link
Member

The plan is to shift away from the use of the np.matrix class at a future point. This functionality is already in place through the use of the use_numpy_matrix function. In version 0.8.3, the np.matrix class is still used for backward compatibility, but you can switch to using arrays through the function call

control.use_numpy_matrix(False)

When we release version 0.9.0, we will switch to have np.ndarray be the default class for all matrices in python-control.

I'll leave this issue open for a bit because I think we generally need to update the documentation to reflect that fact that the return type for functions depends on how you have configured python-control. I've been collecting up documentation updates, so will include those changes in that PR.

@murrayrm murrayrm self-assigned this Jun 12, 2020
@murrayrm murrayrm added this to the 0.8.4 milestone Jun 12, 2020
@bnavigator bnavigator mentioned this issue Aug 16, 2020
Closed
@murrayrm murrayrm mentioned this issue Oct 17, 2020
Closed
murrayrm added a commit to murrayrm/python-control that referenced this issue Dec 22, 2020
@murrayrm
update lqr() return type documentation (addresses python-control#418)
3e6347f
@bnavigator bnavigator linked a pull request Dec 22, 2020 that will close this issue
Documentation update #476
Merged
murrayrm added a commit that referenced this issue Dec 24, 2020
@murrayrm
Documentation update (#476)
120a926 
* fix typos, equation numbering in doc/flatsys.rst
* small updates (including numpydoc fixes) to statefbk.py, mateqn.py docstrings
* update setup.py contact info
* small fixes to iosys docstrings (from samlaf)
* update lqr() return type documentation (addresses #418)
* added 2D array/matrix note to all functions using _ssmatrix
* PEP8 formatting updates for statefbk.py, mateqn.py (while I was at it)
@murrayrm murrayrm mentioned this issue Dec 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@murrayrm murrayrm

Labels
documentation
Projects
None yet
Milestone
0.8.4
2 participants
@murrayrm @jerabaul29