- Notifications
You must be signed in to change notification settings - Fork441
H-infinity mixed-synthesis#151
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedJul 1, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@roryyorke There's a small conflict with master that it would be good to fix. Will try to do a more thorough review in the coming days... |
Will have a look at the conflict. |
New functions augw and mixsyn modelled after Matlab and Octave'sfunction of the same names.python-control doesn't currently support input group naming, so thatis not implemented.
Both examples taken from Skogestad and Postlethwaite's MultivariableFeedback Control.
coveralls commentedJan 2, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
control/robust.py Outdated
| def _size_as_needed(w,wname,n): | ||
| """_size_as_needed(w,wname,n) -> w2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nonstandard docstring. I don't think we use this type of docstring (fcn(args) -> output) anyplace else. In addition, parameters, returns, etc are documented in a different format. Would prefer to use a consistent style, as done inhinfsyn in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Will fix here and elsewhere (augw, _matrix, ...). I see I also used this in xferfcn.py in310f580 in_dc_gain_cont.
control/robust.py Outdated
| def augw(g,w1=None,w2=None,w3=None): | ||
| """augw(g,w1=None,w2=None,w3=None) -> p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nonstandard docstring.
control/robust.py Outdated
| cl: closed system mapping evaluation inputs to evaluation outputs; if p is the augmented plant, with | ||
| [z] = [p11 p12] [w], then cl is the system from w->z with u=-k*y. StateSpace object. | ||
| [y] [p21 g] [u] | ||
| info: namedtuple with fields, in order, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is the first use of anamedtuple inpython-control. Is there a more standard way to return this sort of information innumpy orscipy? If so, perhaps we use that format?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
namedtuple has been around since Python 2.6, so it's hardly non-standard ;). I couldn't find namedtuple use in numpy, but it is used in scipy (stats/stats.py, for example).
I can change this to a plain tuple; if we do keep the namedtuple, I'd like to remove the leading underscore from the name.
control/statesp.py Outdated
| @@ -54,7 +54,7 @@ | |||
| import math | |||
| import numpy as np | |||
| from numpy import all, angle, any, array, asarray, concatenate, cos, delete, \ | |||
| dot, empty, exp, eye, matrix, ones, poly, poly1d, roots, shape, sin, \ | |||
| dot, empty, exp, eye, matrix, ones,pi,poly, poly1d, roots, shape, sin, \ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should avoid usingnumpy.pi since this can generate errors whennumpy is incorporated as amock function. SeePR 171, commit b881b7. Replace withmath.pi.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
unqualifiedpi no longer used following youraa6e0df anyway. Will remove.
control/statesp.py Outdated
| def _matrix(a): | ||
| """_matrix(a) -> numpy.matrix | ||
| a - passed to numpy.matrix | ||
| Wrapper around numpy.matrix; unlike that function, _matrix([]) will be 0x0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not clear to my why this is useful? Can you say a bit here (or in a comment in the code)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll expand the commentary; it's just a helper to make it easier to get 0x0 matrices, which are needed for empty state space systems.
control/robust.py: - doc-strings conform to python-control conventions - use plain tuple instead of namedtuple for auxiliary output in mixsyncontrol/statesp.py: - don't import pi from numpy - doc-string for _matrix expanded, conforms to conventionscontrol/tests/robust_test.py - change test to accept plain tuple
I thought I'd check the docstrings via sphinx, but when I run I get the same error on master. I see This is my conda environment (2.7-based, and AFAIK up-to-date; don't know why I don't have Scipy 1.0): |
coveralls commentedJan 5, 2018
1 similar comment
coveralls commentedJan 5, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@roryyorke I had the same problem with sphinx on my local machine. For some reason it seems to work on readthedocs.org. I think the issue may be with the way that version string is being reported (somehow comes in as a float). We should probably chase this down, but perhaps via a separate issue. I'll try to make sure things look OK on readthedocs when I get a chance (probably tonight or this weekend). |
murrayrm commentedJan 6, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sent@roryyorke a PR that creates the documentation on readthedocs. |
coveralls commentedJan 6, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This adds methods "augw" and "mixsyn", with interfaces similar to the Matlab and Octave functions of the same name. They are for so-called mixed-sensitivity design, in which the sensitivity, complementary sensitivity, and "input sensitivity" (K*S, or reference to input mapping) can be weighted and stacked for H-infinity synthesis.
As part of this, I've fixed what I think is a bug, namely that empty StateSpace objects could not be created. Such empty objects are convenient in the augw implementation.
I've added two examples for mixsyn, one SISO and one MIMO.
augw is the major part of the new code. It may be that there is some better way to implement it, but I have tried to write it in a way that no uncontrollable or unobservable modes are introduced, and so that is still (somewhat) clear.