-
Notifications
You must be signed in to change notification settings - Fork 438
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Small improvements to nlsys, bdalg #1019
Conversation
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 in bdalg.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.
if kwargs.get('inputs', None): | |
if 'inputs' in kwargs: |
Easier to read?
dict.get has signature get(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 size is None, e.g., do val = 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 of listsa and/or tuples of mixed scalar and vector elements. | |
# use of lists and/or tuples of mixed scalar and vector elements. |
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 is issues.) | |
given vector is non-zero, a warning is issued.) |
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 two elemenst) and an estimator state ``0`` (for both states), | |
``x0`` (with two elements) 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 a sinsoidal signal for the first | |
To run an input/output simulation with a sinusoidal signal for the first |
This PR provides some small changes to nonlinear I/O systems process:
linearize
, even ifcopy_names
isFalse
.update_names
method for I/O systems that allows signal and system names to be updated.x0
,u0
keywords inlinearize
andinput_output_response
to provide common functionality in allowing concatenation of lists and zero padding ("vector element processing").x0
andu0
don't match the expected size.nlsys
, which provides full state output, the output signal names are set to match the state names.