GeezerVison is showing github.com — standardize time response return values, return_x/squeeze keyword processing by murrayrm · Pull Request #511 · python-control/python-control
Conversation
This PR is a partial replacement for #507, focused on addressing the issues in #453 relating to time responses:
-
Update forced_response to only return the state trajectory if return_x is True, consistent with all other time response functions (including input_output_response). Prior to this, forced_response always returned the state vector. In addition to the inconsistency, this was also problematic if the system you were simulating was a transfer function, since the state space depended on the realization and this information is lost. Some other smaller changes related to this:
- Issue a warning if you ask for the state space trajectory corresponding to a transfer function
- Set the default value of
return_x using `config.defaults['forced_response.return_x'] (default = False)
- Reset the default value of
config.defaults['forced_response.return_x'] to True with use_legacy_defaults for versions < 0.9.0 for backward compatibility.
- Unit tests to make sure all of that is working.
-
Consolidate processing of time responses through a new internal function _process_time_response that provides uniform processing of the return_x and squeeze keywords. This is mainly in preparation for any future changes in how we handle squeeze, so that changes only need to be made at one location in the code. The functionality of the code is unchanged, but the processing is a big different:
- The default value for the
squeeze keyword for time responses is now set in config.defaults['control.squeeze_time_response']. This is currently set to True, which means that time responses for systems with a single output are squeezed to a 1D response.
- Added some unit tests to make sure that we get the right return values for all of the current time responses (
impulse_response, initial_response, step_response, forced_response, input_output_response).
- Added some commented out code for future implementation of a
squeeze='siso' capability that would allow time responses to mirror current frequency response behavior (squeeze=True only removes axes for SISO systems). I left this out for now pending resolution of the frequency response squeeze processing (drafted in Standardize squeeze processing in frequency response functions #507, but needs more thought and effort).
Other changes, not directly related to those above
- Added a warning if you try to specify a non-zero initial state when simulating a transfer function.
- Docstring cleanup to use numpydoc syntax in few places.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coveralls
commented
Jan 17, 2021
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|

Coverage increased (+0.06%) to 87.607% when pulling 47bbf16 on murrayrm:standardize_time_response into 6f674cf on python-control:master.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jan 17, 2021
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.