Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Time response data class#649

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

Merged
bnavigator merged 14 commits intopython-control:masterfrommurrayrm:timeresponse_class
Nov 21, 2021

Conversation

murrayrm
Copy link
Member

This PR adds aTimeResponseData class, following the discussion in#645, which provides a more object-oriented approach to the representation of time response data. All time responses functions now return an instance ofTimeResponseData containing the results of simulations. The class implements an__iter__ method that allows backward compatibility with current call signatures.

Old approach (still works):

t, y = ct.step_response(siso_sys)plt.plot(t, y)t, y = ct.step_response(mimo_sys)plt.plot(t, y[0, 1], label="input 1 to output 0")plt.plot(t, y[1, 1], label="input 1 to output 1")

New approach:

response = ct.step_response(siso_sys)plt.plot(response.time, response.outputs)response = ct.step_response(mimo_sys)plt.plot(response.time, response.outputs[0, 1], label="input 1 to output 0")plt.plot(response.time, response.outputs[1, 1], label="input 1 to output 1")

The attributestime,outputs,states andinputs are actually class properties that carry out "squeeze processing" allowing use of 1D arrays for SISO systems. Time response data can also be accessed via the attributest,y,x, andu, which are always in "MIMO" format (indexed by output, input (trace), and time:

response = ct.step_response(siso_sys)plt.plot(response.t, response.y[0, 0])

Other notes:

  • I adopted the terminology of a "trace" to handle the time response data returned bystep_response andimpulse_response, where a MIMO system generates a full matrix of input/output responses. The output, state, and input responses for those functions are represented by a 3D array indexed by output/state/input, trace, and time.

  • The terminology fortime,outputs,states andinputs was chosen to be consistent with theOptimalControlResult class. This is implemented via a set of properties that perform the squeeze (and transpose) processing to provide compatibility with our existing functionality.

  • TheInputOutputData class is currently only used to store the output of simulations, but it is set up to serve as a pure data representation class (similar toFrequencyResponseData) and could later be useful for things like input to system identification routines.

  • No changes to existing unit tests were needed => should be fully backwards compatible.

At a future date, we may want to consider adding some additional functionality:

  • Plot methods that plot input/output simulations in a uniform way
  • Methods to convert data intoDataFrame format for use of pandas plotting capabilities

I'm still not completely sure if this is a useful PR, but I was motivated by the way a similar approach taken inJuliaControl and thought I would give it a try. Suggestions and comments welcome (will leave in draft form for a bit to allow for discussion).

@coveralls
Copy link

coveralls commentedAug 26, 2021
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 90.079% when pulling136d6f4 on murrayrm:timeresponse_class into18976c1 on python-control:master.

@sawyerbfuller
Copy link
Contributor

If you call me a skeptic about object-oriented software I'll answer "guilty as charged" but this seems pretty cool and would work well with#645. I also like the potential utility for future system identification routines, and for future plotting functionality. Presumably when the latter comes we can do things like add label text, which can come in handy.

It would be nice, in the interest of minimizing friction for quick analyses of SISO systems, to have the shortert,y attributes be the ones that are auto-squeezed, but if there is already a convention established inOptimalControlResult then maybe we should stick with that.

I'm not sure I understand the concept of a trace. is that different than the 3D array that encodes [output, input, time]?

@murrayrm
Copy link
MemberAuthor

The latest push fixes an inconsistency in the way that squeeze processing was handled for states versus input/outputs. In the original code, squeeze processing was never applied to the state vector. The consequence was that if you usedsqueeze=True orsqueeze=False then you got a different action on the state vector than the output vector.

To avoid breaking code, I set things up so that if you access the state vector through the old interface (by assigning one of the time response systems to a tuple withreturn_x=True) then you get the old behavior. But if you use the new interface (response.states) the squeeze processing matches what is done on the input/output vectors. Namely, settingsqueeze=True will get rid of all 1 dimensional entries and settingsqueeze=False will index everything by the state, trace (corresponding to the input number ofstep_response andimpulse_response), and time (in that order), even for SISO systems. If you leavesqueeze=None then the trace is removed for SISO systems but maintained for MIMO systems.

As in the original PR, all legacy code continues to run unchanged and there are new unit tests to validate the updated behavior for the new interface.

Alternatively, I would be OK making everything consistent across output, state, and input squeeze processing, but then we would break existing code. So my thought was to leave that in place and eventually deprecate it (perhaps after we have implemented all of the suggestions in#645).

@murrayrm
Copy link
MemberAuthor

murrayrm commentedAug 27, 2021
edited
Loading

I'm not sure I understand the concept of a trace. is that different than the 3D array that encodes [output, input, time]?

It's the same. I decided to use the term "trace" because in the case of step/impulse response it happens to be the case that each trace (simulation run) corresponds to choosing one of the inputs. But more generally, you could imagine having multiple input/output traces (eg, for the aforementioned system ID) and a given trace might just be come combination of inputs.

Note also thatTimeResponseData includes aninputs property of dimension (ninputs, ntraces, ntimes). So you can have a different number of traces than inputs, if you like.

sawyerbfuller reacted with thumbs up emoji

@murrayrm
Copy link
MemberAuthor

Presumably when the latter [plotting functionality] comes we can do things like add label text, which can come in handy.

I've added the ability to store labels in the latest commit and set things up so that the signals names for I/O systems, if present, are copied over as the default labels forinput_output_response.

sawyerbfuller reacted with thumbs up emoji

@sawyerbfuller
Copy link
Contributor

@billtubbs Would this PR help with what you were working on for#645 ? ie., should we merge this PR?

@murrayrmmurrayrm marked this pull request as ready for reviewNovember 21, 2021 16:50
@murrayrm
Copy link
MemberAuthor

I've been using the branch with this PR as my default branch for a while and everything seems OK. Ready to be merged, I think.

@billtubbs You mentioned in a separate note that you had made progress on#645. Does this conflict with that? If not, we can probably go ahead and merge.

@billtubbs
Copy link
Contributor

@billtubbs You mentioned in a separate note that you had made progress on#645. Does this conflict with that? If not, we can probably go ahead and merge.

I don't think so. I haven't looked at this in detail but I was only planning to fix the specific axes and figure problems with some of the plot functionsrlocus,pzmap,bode_plot,nyquist_plot,gangof4_plot,nichols_plot. My work is probably already behind the master so will need revising anyway if there are any impacts.

@bnavigatorbnavigator merged commit56cecc0 intopython-control:masterNov 21, 2021
@murrayrmmurrayrm deleted the timeresponse_class branchNovember 25, 2021 22:06
@murrayrmmurrayrm added this to the0.9.1 milestoneDec 30, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bnavigatorbnavigatorbnavigator approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.9.1
Development

Successfully merging this pull request may close these issues.

5 participants
@murrayrm@coveralls@sawyerbfuller@billtubbs@bnavigator

[8]ページ先頭

©2009-2025 Movatter.jp