NotificationsYou must be signed in to change notification settings
Fork446
Star1.9k

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.

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

Jump to bottom

Merged

murrayrm merged 13 commits intopython-control:masterfrombnavigator:mimo-step-info

Mar 20, 2021

Merged

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

murrayrm merged 13 commits intopython-control:masterfrombnavigator:mimo-step-info

Mar 20, 2021

Conversation

Copy link

Contributor

bnavigator commentedMar 17, 2021

Enhance thestep_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

enhance step_info to MIMO

ff960c3

Copy link

ContributorAuthor

bnavigator commentedMar 17, 2021

Ping@juanodecc

remove test function with duplicate name

b98008a

Copy link

coveralls commentedMar 17, 2021•
edited
Loading

Coverage increased (+0.08%) to 89.028% when pullinga878846 on bnavigator:mimo-step-info into22b9953 on python-control:master.

bnavigator added2 commits

March 17, 2021 22:01

add step_info mimo test (TransferFunction)

ed63358

fix timevector and list return population for MIMO step_info

35d8eba

bnavigator marked this pull request as ready for review

March 17, 2021 21:56

Copy link

Contributor

juanodecc commentedMar 18, 2021

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

Copy link

ContributorAuthor

bnavigator commentedMar 18, 2021

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 added5 commits

March 18, 2021 23:44

apply isort

468ffbf

step_info from MIMO step_response

a1fd47e

reenable masked timevector test, and really test if we get the tfinal

238482e

include tfinal in auto generated timevector

cc90d8d

mimo is slycot only

01ef666

Copy link

ContributorAuthor

bnavigator commentedMar 19, 2021

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

bnavigator requested a review fromsawyerbfuller

March 19, 2021 00:02

murrayrm reviewed

Mar 19, 2021

View reviewed changes

control/timeresp.py OutdatedShow resolvedHide resolved

sawyerbfuller reviewed

Mar 19, 2021

View reviewed changes

control/timeresp.pyShow resolvedHide resolved

sawyerbfuller approved these changes

Mar 19, 2021

View reviewed changes

bnavigator added2 commits

March 19, 2021 12:52

Describe MIMO step_info return and add doctest example

20ed368

support time series of response data in step_info

43d73f0

bnavigator changed the title~~enhance step_info to MIMO~~enhance step_info to MIMO and time series of response data

Mar 19, 2021

Copy link

ContributorAuthor

bnavigator commentedMar 19, 2021

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

add yfinal parameter to step_info

6587f69

bnavigator force-pushed themimo-step-info branch 4 times, most recently from3660e15 toab555ddCompare

March 19, 2021 15:03

include yfinal in matlab.stepinfo call signature

a878846

bnavigator force-pushed themimo-step-info branch fromab555dd toa878846Compare

March 19, 2021 15:05

bnavigator mentioned this pull request

Mar 19, 2021

sys.dcgain() should return a real value rather than a complex value (until/unless we support complex coefficients)#578

Closed

Copy link

Member

murrayrm commentedMar 19, 2021

@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.

Copy link

ContributorAuthor

bnavigator commentedMar 20, 2021

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 fordcgain() and InfValue is obsolete. So, if#579 is going to be merged before, I will have the change to clean this up a little.

Copy link

Member

murrayrm commentedMar 20, 2021

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.