System norms 2 #971
System norms 2 #971
Conversation
control/tests/sysnorm_test.py
Outdated
| assert np.allclose(ct.norm(G1, p='inf', tol=1e-9 ), 1.0) # Comparison to norm computed in MATLAB | ||
| assert np.allclose(ct.norm(G1, p=2), 0.707106781186547) # Comparison to norm computed in MATLAB |
There was a problem hiding this comment.
See #960 (comment)
Please don't restrict the tolerance to 1e-9. It will create problems like the one solved in #366.
There was a problem hiding this comment.
Is it OK to set the default to 1e-2? (For comparison: statesp.linfnorm uses default tolerance 1e-10, and MATLAB's hinfnorm uses 1e-2.)
There was a problem hiding this comment.
That's maybe a bit coarse. I'd say we start with 1e-6 and see if we get failures, e.g. in distribution packages like the openSUSE rpm or conda after the next release.
|
I have fixed the futurewarning test. Please rebase. |
* Added control/tests/sysnorm_test.py * Added additional tests and warning messages for systems with poles close to stability boundary * Added __all__ * Do not track changes in VS Code setup.
* type check when calling ct.norm * metod argument in ct.norm (slycot or scipy) using function _slycot_or_scipy from ct.mateqn Change in sysnorm_test.py * set print_warning=False
* In sysnorm_test: Use pytest.warns context manager
* sysnorm_test: Use default tolerance in the tests.
e10b798 to
bad229a
Compare
|
Thank you @henriks76! |
|
Thank you @henriks76. I also had plans to implement the norm() method. That's why I was interested in Slycot.ab13bd and Slycot.ab13dd in the first place. You might want to add the method https://github.com/python-control/python-control/blob/main/doc/control.rst A new PR would be the right way. |
|
Thanks, @KybernetikJo and @bnavigator, for all the excellent feedback! I'll look into the documentation as soon as possible. |
|
Is it correct that the sphinx documentation will be automatically generated from the function docstring, or would you like me to copy it somewhere? This is all very new to me... I have already tried to follow the conventions here, but let me know if something needs fixing. |
No need to copy anything for API documentation, you only have to add your method to The autosummary of sphinx should take care about your API documentation. You have to install all requirements (https://github.com/python-control/python-control/blob/main/doc/requirements.txt) for sphinx (e.g. with pip) pip install -r doc/requirements.txtand then you run the bash command in the doc folder make htmlYou can follow the instructions, point 6.
I only skimmed over it, but it looks good to me. |
I believe all earlier feedback on
sysnorm.pyhas been addressed now.