add reference gain design pattern to create_statefbk_iosystem#1071
add reference gain design pattern to create_statefbk_iosystem#1071murrayrm merged 5 commits intopython-control:mainfrom
Conversation
|
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.
| design patter (default), this system takes as inputs the desired | |
| design pattern (default), this system takes as inputs the desired |
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.
| the 'refgain' design patter, the system takes as inputs the | |
| the 'refgain' design pattern, the system takes as inputs the |
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.
I might have missed something, but some readers will not know the reference "FBS2e". One idea is to make FBS2e_ a link to the book website. This can be done with the following changes:
diff --git a/doc/conf.py b/doc/conf.py
index 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.rst
index 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 systemFeel 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.
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 like feedfwd_pattern; e.g., create_statefbk_trajgen(), which is implemented by calling _create_statefbk_iosystem() with feedfwd_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 when feedfwd_pattern has an unknown value? E.g.,
| sys, K, Kf, feedfwd_pattern='refgain') | |
| sys, K, Kf, feedfwd_pattern='refgain') | |
| with pytest.raises(NotImplementedError, match="unknown pattern"): | |
| ct.create_statefbk_iosystem(sys, K, Kf, feedfwd_pattern='refgai') |
There was a problem hiding this comment.
Related to the above, there may be existing code that calls create_statefbk_iosystem() with 3 positional arguments, thus giving integral_action without using the keyword. With this PR, that would effectively be a call to create_statefbk_iosystem() with a value to feedfwd_gain but feedfwd_pattern still at its default. I modified the test test_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 4
control/statefbk.py:831: ControlArgument
It is good that an exception is raised, but users may be confused about the cause.
I think that having feedfwd_gain as the third parameter is intuitive and worth keeping, so I propose that a ControlArgument exception is raised if feedfwd_gain is not None but feedfwd_pattern != 'refgain'.
There was a problem hiding this comment.
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 the create_estimator_iosystem and create_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.
| (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 to
96b7b73
Compare
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.)