- Notifications
You must be signed in to change notification settings - Fork445
add reference gain design pattern to create_statefbk_iosystem#1071
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 commentedDec 2, 2024 • 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.
slivingston commentedDec 4, 2024
I will review this today. |
control/statefbk.py Outdated
| (proportional and integral) are evaluated using the scheduling | ||
| variables specified by `gainsched_indices`. | ||
| Input/output system representing the controller. For the 'trajgen' | ||
| design patter (default), this system takes as inputs the desired |
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.
| designpatter (default),thissystemtakesasinputsthedesired | |
| designpattern (default),thissystemtakesasinputsthedesired |
control/statefbk.py Outdated
| state `x_d`, the desired input `u_d`, and either the system state | ||
| `x` or the estimated state `xhat`. It outputs the controller | ||
| action `u` according to the formula `u = u_d - K(x - x_d)`. For | ||
| the 'refgain' design patter, the system takes as inputs the |
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.
| the'refgain'designpatter,thesystemtakesasinputsthe | |
| the'refgain'designpattern,thesystemtakesasinputsthe |
doc/iosys.rst Outdated
| ctrl, clsys = ct.create_statefbk_iosystem(sys, K, kf, feedfwd_pattern='refgain') | ||
| This reference gain design pattern is described in more detail in Section | ||
| 7.2 of FBS2e (Stabilization by State Feedback) and the trajectory |
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 might have missed something, but some readers will not know the reference "FBS2e". One idea is to makeFBS2e_ a link to the book website. This can be done with the following changes:
diff --git a/doc/conf.py b/doc/conf.pyindex 75981d6..a523713 100644--- a/doc/conf.py+++ b/doc/conf.py@@ -285,3 +285,7 @@ import control.flatsys as fs import control.phaseplot as pp ct.reset_defaults() """++rst_prolog = """+.. _FBS2e: https://fbswiki.org/wiki/index.php/Feedback_Systems:_An_Introduction_for_Scientists_and_Engineers+"""diff --git a/doc/iosys.rst b/doc/iosys.rstindex e67f4dc..413f4d4 100644--- a/doc/iosys.rst+++ b/doc/iosys.rst@@ -57,7 +57,7 @@ Example To illustrate the use of the input/output systems module, we create a model for a predator/prey system, following the notation and parameter-values in FBS2e.+values in FBS2e_. We begin by defining the dynamics of the system
Feel free to apply this change to your PR, or I can open another PR that adds this and possibly other docs enhancements.
| else: | ||
| ctrl,clsys=ct.create_statefbk_iosystem( | ||
| sys,K,Kf,feedfwd_pattern='refgain') |
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.
create_statefbk_iosystem() is a big function now with several different ways of being called and a docstring of 180 lines. One design pattern to handle this is changing the function to be internal and then creating user-facing functions that have names implying values of arguments likefeedfwd_pattern; e.g., create_statefbk_trajgen(), which is implemented by calling _create_statefbk_iosystem() withfeedfwd_pattern='trajgen'. However, this alternative may not help users in practice.
In the scope of this PR, it suffices to ensure that mistakes when calling create_statefbk_iosystem() are caught. To that end, can you add a case for whenfeedfwd_pattern has an unknown value? E.g.,
| sys,K,Kf,feedfwd_pattern='refgain') | |
| sys,K,Kf,feedfwd_pattern='refgain') | |
| withpytest.raises(NotImplementedError,match="unknown pattern"): | |
| ct.create_statefbk_iosystem(sys,K,Kf,feedfwd_pattern='refgai') |
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.
Related to the above, there may be existing code that calls create_statefbk_iosystem() with 3 positional arguments, thus givingintegral_action without using the keyword. With this PR, that would effectively be a call to create_statefbk_iosystem() with a value tofeedfwd_gain butfeedfwd_pattern still at its default. I modified the testtest_lqr_integral_continuous to demonstrate this case, and the resulting error is
if isinstance(gain, np.ndarray): K = gain if K.shape != (sys_ninputs, estimator.noutputs + nintegrators):> raise ControlArgument( f'control gain must be an array of size {sys_ninputs}' f' x {sys_nstates}' + (f'+{nintegrators}' if nintegrators > 0 else ''))E control.exception.ControlArgument: control gain must be an array of size 2 x 4control/statefbk.py:831: ControlArgumentIt is good that an exception is raised, but users may be confused about the cause.
I think that havingfeedfwd_gain as the third parameter is intuitive and worth keeping, so I propose that aControlArgument exception is raised iffeedfwd_gain is notNone butfeedfwd_pattern != 'refgain'.
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've left the single function for now, though I agree this function is getting big and complicated. I'm not sure that splitting it in two simplifies things much, and the current naming matches thecreate_estimator_iosystem andcreate_mpc_iosystem naming pattern (and functionality).
I created the exception suggested above + unit test.
control/tests/statefbk_test.py Outdated
| (1,1), (1,None), | ||
| (2,np.diag([1,1])), (2,None), |
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.
| (1,1), (1,None), | |
| (2,np.diag([1,1])), (2,None), | |
| (1,1), | |
| (1,None), | |
| (2,np.diag([1,1])), | |
| (2,None), |
Easier to read if we have one case (i.e., one assignment of parameters) per line
604a3df to96b7b73Compared700ad7 intopython-control:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This PR adds a second design pattern to the
create_statefbk_iosystemfunction, allowing controllers that use a static gain on the reference input instead of the desired state and input. As described in the user documentation:In addition, this PR includes some changes that make the error messages for setting up a state space system more informative, but giving the expected size of matrices that do not have the right dimensions. (This could go in a separate PR, but it is here because I got frustrated with the error messages while debugging some examples.)