Skip to content

enhance step_info to MIMO and time series of response data #577

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Mar 20, 2021
Merged

enhance step_info to MIMO and time series of response data #577

merged 13 commits into from
Mar 20, 2021

Conversation

bnavigator
Copy link
Contributor

Enhance the step_info() function to return a 2D list of info dicts for MIMO systems.

Cleaned up the code a little.

Fixes #574

WIP: needs unit tests for MIMO

@bnavigator
Copy link
Contributor Author

Ping @juanodecc

@coveralls
Copy link

coveralls commented Mar 17, 2021 •
edited
Loading

Coverage Status

Coverage increased (+0.08%) to 89.028% when pulling a878846 on bnavigator:mimo-step-info into 22b9953 on python-control:master.

@bnavigator bnavigator marked this pull request as ready for review March 17, 2021 21:56
@juanodecc
Copy link
Contributor

sorry i dont understand ... i has make a new version of step_info which supports MIMO systems on my branch, but now i dont know what i should do. Git is so complicated for my

@bnavigator
Copy link
Contributor Author

Please just wait. I am working on this PR right now. Also, note that I am dumb. The current version of this PR does not do really do MIMO. I should know better.

@bnavigator
Copy link
Contributor Author

Had to make some adjustments to the automatic calculation of timevectors. Surely one wants to have tfinal included into the generated timevector.

murrayrm
murrayrm reviewed Mar 19, 2021
View reviewed changes
@bnavigator bnavigator changed the title enhance step_info to MIMO enhance step_info to MIMO and time series of response data Mar 19, 2021
@bnavigator
Copy link
Contributor Author

Now also supports a time series of reponse data as parameter, which could come from measurements or another simulation.

@bnavigator bnavigator force-pushed the mimo-step-info branch 4 times, most recently from 3660e15 to ab555dd Compare March 19, 2021 15:03
@bnavigator bnavigator mentioned this pull request Mar 19, 2021
Closed
@murrayrm
Copy link
Member

@bnavigator Is this now good to go?

@sawyerbfuller Do you want to have a final look before merge? This will go into 0.9.0.

@bnavigator
Copy link
Contributor Author

As far as I am concerned, it is good to go. The time response data part has not been reviewed yet.

After #579 is merged some of the .real treatment for dcgain() and InfValue is obsolete. So, if #579 is going to be merged before, I will have the change to clean this up a little.

@murrayrm
Copy link
Member

I'll merge #579 now and then we can rebase this one (and give @sawyerbfuller a chance to have a look at the time response data part). I can also dig into if he is busy.

@sawyerbfuller
Copy link
Contributor

Looks good to me

@sawyerbfuller sawyerbfuller self-requested a review March 20, 2021 03:03
@murrayrm murrayrm merged commit 76d51ff into python-control:master Mar 20, 2021
This was referenced Mar 20, 2021
Merged
Closed
@murrayrm murrayrm added this to the 0.9.0 milestone Mar 20, 2021
@bnavigator bnavigator deleted the mimo-step-info branch February 18, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@murrayrm murrayrm murrayrm left review comments

@sawyerbfuller sawyerbfuller sawyerbfuller approved these changes

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

Successfully merging this pull request may close these issues.

step_info error for MIMO system
5 participants
@bnavigator @coveralls @juanodecc @murrayrm @sawyerbfuller