- Notifications
You must be signed in to change notification settings - Fork441
Small improvements to nlsys, bdalg#1019
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
Small improvements to nlsys, bdalg#1019
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedJun 30, 2024
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.
Functionality looks good. Minor suggestions and remarks.
| from .iosys import InputOutputSystem | ||
| __all__ = ['series', 'parallel', 'negate', 'feedback', 'append', 'connect'] | ||
| def series(sys1, *sysn): | ||
| def series(sys1, *sysn, **kwargs): |
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.
kwargs not documented in docstring, here or the other functions inbdalg.py.
control/iosys.py Outdated
| Parameters | ||
| ---------- | ||
| inputs : int, list of str, or 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.
name kwarg not documented
control/iosys.py Outdated
| @@ -825,6 +865,7 @@ def _process_labels(labels, name, default): | |||
| # | |||
| import re | |||
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 can go - imported at top (not changed in this PR, tho...)
control/iosys.py Outdated
| Parameters | ||
| ---------- | ||
| inputs : int, list of str, or 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.
explanation (or pointer to such - think this convention used in a few places?) of what integer and None do.
Possibly out-of-scope: what about a dict, of{oldname1:newname1, oldname2:newname2,...} - in this case don't have to specify all inputs (or outputs, or states...)
control/iosys.py Outdated
| """ | ||
| self.name = kwargs.pop('name', self.name) | ||
| if kwargs.get('inputs', 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.
| ifkwargs.get('inputs',None): | |
| if'inputs'inkwargs: |
Easier to read?
dict.get has signatureget(key, default=None), so if you want to use get you don't need None.
| v = np.array(v).reshape(-1) # convert to 1D array | ||
| val_list += v.tolist() # add elements to list | ||
| val = np.array(val_list) | ||
| elif np.isscalar(arg) and size is not None: # extend scalars |
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.
does this funciton need to handle the case when np.isscalar(arg) and sizeis None, e.g., doval = np.array([arg]) ?
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.
Added this case.
control/nlsys.py Outdated
| # Process input argument | ||
| # | ||
| # The input argument is interpreted very flexibly, allowing the | ||
| # use of listsa and/or tuples of mixed scalar and vector elements. |
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.
| # use oflistsa and/or tuples of mixed scalar and vector elements. | |
| # use oflists and/or tuples of mixed scalar and vector elements. |
coveralls commentedJul 9, 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.
doc/iosys.rst Outdated
| * Vector elements are zero padded to the required length. If you | ||
| specify only a portion of the values for states or inputs, the | ||
| remaining values are taken as zero. (If the final element in the | ||
| given vector is non-zero, a warning is issues.) |
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.
| given vector is non-zero, a warning isissues.) | |
| given vector is non-zero, a warning isissued.) |
doc/iosys.rst Outdated
| state and input vectors, respectively. | ||
| If we want to linearize the closed loop system around a process state | ||
| ``x0`` (with two elemenst) and an estimator state ``0`` (for both states), |
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.
| ``x0`` (with twoelemenst) and an estimator state ``0`` (for both states), | |
| ``x0`` (with twoelements) and an estimator state ``0`` (for both states), |
doc/iosys.rst Outdated
| ``x0`` (with two elemenst) and an estimator state ``0`` (for both states), | ||
| we can use the list processing feature:: | ||
| H = clsys.liniearize([x0, 0], 0) |
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.
| H = clsys.liniearize([x0, 0], 0) | |
| H = clsys.linearize([x0, 0], 0) |
doc/iosys.rst Outdated
| second argument in the list ``[x0, 0]`` is a scalar and so the vector | ||
| ``[x0, 0]`` only has three elements instead of the required four. | ||
| To run an input/output simulation with a sinsoidal signal for the first |
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.
| To run an input/output simulation with asinsoidal signal for the first | |
| To run an input/output simulation with asinusoidal signal for the first |
6406868 intopython-control:mainUh oh!
There was an error while loading.Please reload this page.
This PR provides some small changes to nonlinear I/O systems process:
linearize, even ifcopy_namesisFalse.update_namesmethod for I/O systems that allows signal and system names to be updated.x0,u0keywords inlinearizeandinput_output_responseto provide common functionality in allowing concatenation of lists and zero padding ("vector element processing").x0andu0don't match the expected size.nlsys, which provides full state output, the output signal names are set to match the state names.