GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — enhance step_info to MIMO and time series of response data by bnavigator · Pull Request #577 · python-control/python-control

Conversation

@bnavigator

@bnavigator bnavigator commented Mar 17, 2021

Copy link
Copy Markdown
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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bnavigator
enhance step_info to MIMO
ff960c3
@bnavigator

bnavigator commented Mar 17, 2021

Copy link
Copy Markdown
Contributor Author

Ping @juanodecc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
remove test function with duplicate name
b98008a
@coveralls

coveralls commented Mar 17, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bnavigator added 2 commits March 17, 2021 22:01
@bnavigator
add step_info mimo test (TransferFunction)
ed63358
@bnavigator
fix timevector and list return population for MIMO step_info
35d8eba
@bnavigator
bnavigator marked this pull request as ready for review March 17, 2021 21:56
@juanodecc

juanodecc commented Mar 18, 2021

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator

bnavigator commented Mar 18, 2021

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bnavigator added 5 commits March 18, 2021 23:44
@bnavigator
apply isort
468ffbf
@bnavigator
step_info from MIMO step_response
a1fd47e
@bnavigator
reenable masked timevector test, and really test if we get the tfinal
238482e
@bnavigator
include tfinal in auto generated timevector
cc90d8d
@bnavigator
mimo is slycot only
01ef666
@bnavigator

bnavigator commented Mar 19, 2021

Copy link
Copy Markdown
Contributor Author

Had to make some adjustments to the automatic calculation of timevectors. Surely one wants to have tfinal included into the generated timevector.
sawyerbfuller reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
bnavigator requested a review from sawyerbfuller March 19, 2021 00:02
murrayrm
murrayrm reviewed Mar 19, 2021
View reviewed changes
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sawyerbfuller
sawyerbfuller reviewed Mar 19, 2021
View reviewed changes
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sawyerbfuller
sawyerbfuller approved these changes Mar 19, 2021
View reviewed changes
bnavigator added 2 commits March 19, 2021 12:52
@bnavigator
Describe MIMO step_info return and add doctest example
20ed368
@bnavigator
support time series of response data in step_info
43d73f0
@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

bnavigator commented Mar 19, 2021

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
add yfinal parameter to step_info
6587f69
@bnavigator
bnavigator force-pushed the mimo-step-info branch 4 times, most recently from 3660e15 to ab555dd Compare March 19, 2021 15:03
@bnavigator
include yfinal in matlab.stepinfo call signature
a878846
@bnavigator
bnavigator force-pushed the mimo-step-info branch from ab555dd to a878846 Compare March 19, 2021 15:05
@bnavigator 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
@murrayrm

murrayrm commented Mar 19, 2021

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator

bnavigator commented Mar 20, 2021

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm

murrayrm commented Mar 20, 2021

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sawyerbfuller

sawyerbfuller commented Mar 20, 2021

Copy link
Copy Markdown
Contributor

Looks good to me
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sawyerbfuller
sawyerbfuller self-requested a review March 20, 2021 03:03
sawyerbfuller
sawyerbfuller approved these changes Mar 20, 2021
View reviewed changes
@murrayrm
murrayrm merged commit 76d51ff into python-control:master Mar 20, 2021
This was referenced Mar 20, 2021
dcgain and step_info post PR merge cleanup #582
Merged
Step info improve jpp #583
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

Uh oh!

There was an error while loading. Please reload this page.

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

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@bnavigator @coveralls @juanodecc @murrayrm @sawyerbfuller
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.