GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Improve lft functions and lft documentation by tomaantjes · Pull Request #1249 · python-control/python-control ConversationThis PR aims to solve #1156 by updating the current Summary of main changes:
AI Disclosure:
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions…tionality and update reference update iosys_test test_lft docstring …ter performing lft
slivingston
self-requested a review
|
There was an error while loading. Please reload this page.
|
coverage: 94.782% (+0.03%) from 94.757% — tomaantjes:improve-lft-functions into python-control:main |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for proposing this. Some tests fail when Slycot is not installed, e.g.,
FAILED control/tests/bdalg_test.py::TestLft::test_lft_tf_inputs[-1--1] - control.exception.ControlMIMONotImplemented: MIMO system conversion not supported without Slycot
FAILED control/tests/bdalg_test.py::TestLft::test_lft_tf_inputs[1-1] - control.exception.ControlMIMONotImplemented: MIMO system conversion not supported without Slycot
Can you fix that? After you do, I will review this pull request.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Of course, I marked it in the newest commit. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| edition, 2005. | ||
| .. [1] J. Doyle, A. Packard, and K. Zhou, "Review of LFTs, | ||
| LMIs, and mu," Proceedings of the 30th IEEE Conference on | ||
| Decision and Control, Brighton, England, 1991, pp. 1227-1232. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| Decision and Control, Brighton, England, 1991, pp. 1227-1232. | |
| Decision and Control, Brighton, England, 1991, pp. 1227-1232. | |
| https://authors.library.caltech.edu/records/8t2hq-w7318 |
Add a URL to help readers find it quickly. (This can also be https://doi.org/10.1109/CDC.1991.261572 or https://doyle.caltech.edu/images/doyle/7/70/CDC1991.pdf, but the Caltech library open access is most helpful, in my opinion.) This is not done universally in the documentation of control, but there are examples with arXiv and DOI URLs.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| def lft(self, other, nu=-1, ny=-1, **kwargs): | ||
| """Return the linear fractional transformation. | ||
|
|
||
| A definition of the LFT operator can be found in Appendix A.7, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I do not have a copy of this book and could not find a reliable free source on the Internet, so I agree that the CDC paper is easier to find.
We should keep a link to the Matlab lft definition because it shows some compatibility between that and this implementation. https://www.mathworks.com/help/control/ref/lft.html is broken. The up-to-date URL seems to be https://www.mathworks.com/help/control/ref/inputoutputmodel.lft.html
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I agree adding a link to the Caltech library would be most helpful and I added it in addition to a link to the Matlab documentation.
I also noticed the docstring missed some information regarding the way I currently implemented the fallback with duplicate signal names and added that as well.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I will need another 2 or 3 days to complete my review.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.