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 — improve forced_response and its documentation by bnavigator · Pull Request #588 · python-control/python-control

Conversation

@bnavigator

@bnavigator bnavigator commented Mar 24, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor
  • Fix doc error for return_x as revealed by control.forced_response argument T #586.
  • Remove redundant kwarg defaults. They are in the call signuature. Numpydoc compliant default indication
  • Enhance error message if ctime system without timevector is provided
  • Make sure the returned time vector keeps it's length

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
sawyerbfuller
sawyerbfuller reviewed Mar 24, 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 24, 2021
View reviewed changes
Comment thread control/timeresp.py Resolved

Uh oh!

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

bnavigator commented Mar 24, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Strange that the markov test fails here too.

https://github.com/python-control/python-control/pull/588/checks?check_run_id=2188093854#step:5:212

A rerun succeeded. Wonder what triggered the length mismatch of Y and U.
All reactions

Sorry, something went wrong.

Uh oh!

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

coveralls commented Mar 24, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 89.448% when pulling a8b72f5 on bnavigator:timeresp-doc into f1a9860 on python-control:master.
All reactions

Sorry, something went wrong.

Uh oh!

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

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread control/tests/timeresp_test.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread control/timeresp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
bnavigator force-pushed the timeresp-doc branch from a09e98c to 66b381f Compare March 25, 2021 12:24
@bnavigator bnavigator changed the title update parameter doc and error message for forced_response improve forced_response and its documentation Mar 26, 2021
@bnavigator

bnavigator commented Mar 26, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Even without the modifications of this PR, testMarkovResults started to fail sporadically (#588 (comment)), because scipy.signal.dlsim returns not enough samples when the last element of the time series is just below a multiple of sys_dt

Fixed with a6ea296

TODO: cover the new lines. They were only triggered by the testMarkovResults test depending on the random seed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
bnavigator force-pushed the timeresp-doc branch from a6ea296 to 77c3b40 Compare March 26, 2021 23:41
@bnavigator bnavigator mentioned this pull request Mar 28, 2021
a first implementation of the singular value plot as discussed in #592 #593
Merged
@bnavigator
bnavigator requested review from murrayrm and sawyerbfuller March 30, 2021 12:07
@bnavigator bnavigator linked an issue Mar 30, 2021 that may be closed by this pull request
control.forced_response argument T #586
Closed
@bnavigator bnavigator mentioned this pull request Mar 30, 2021
Include InfValue into settling min/max calculation for step_info #600
Merged
bnavigator added a commit to bnavigator/python-control that referenced this pull request Mar 31, 2021
@bnavigator
xfail testmarkovResults until python-control#588 is merged
b0179fb
bnavigator added a commit that referenced this pull request Mar 31, 2021
@bnavigator
Merge pull request #601 from bnavigator/master
9edc768 
xfail testmarkovResults until #588 is merged
bnavigator added 6 commits April 1, 2021 13:08
@bnavigator
update parameter doc and error message for forced_response
1f51526
@bnavigator
allow T=None with sys.dt=None in forced_response
2870432
@bnavigator
timeresp: add test system with dt=None
517b1eb
@bnavigator
more docstring fixups
52a3fd4
@bnavigator
test forced_response omitting parameters as documented
f0d764f
@bnavigator
ensure dlsim output length
9cac9fa
@bnavigator
bnavigator force-pushed the timeresp-doc branch from 7060eab to 3e1b615 Compare April 1, 2021 11:13
bnavigator added 6 commits April 1, 2021 13:16
@bnavigator
rework fixture setup for timeresp_test
7be508b
@bnavigator
specify tolerance and add comment for markov test
a72441b
@bnavigator
reactivate the the fast forced_response(U=0) algorithm and test it
177290c
@bnavigator
cover TF initial condition warning in forced_response
ce51d34
@bnavigator
reallow non zero timevector start
4404169
@bnavigator
avoid different realizations with and without slycot
a8b72f5
@bnavigator
bnavigator force-pushed the timeresp-doc branch from 3e1b615 to a8b72f5 Compare April 1, 2021 11:16
sawyerbfuller
sawyerbfuller approved these changes Apr 1, 2021
View reviewed changes

@sawyerbfuller sawyerbfuller left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Seems reasonable to allow a nonzero start time!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bnavigator
bnavigator merged commit 5434318 into python-control:master Apr 2, 2021
@murrayrm murrayrm added this to the 0.9.1 milestone Dec 30, 2021
@bnavigator
bnavigator deleted the timeresp-doc 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

@sawyerbfuller sawyerbfuller sawyerbfuller approved these changes
@murrayrm murrayrm Awaiting requested review from murrayrm

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

Development

Successfully merging this pull request may close these issues.

control.forced_response argument T

Uh oh!

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

4 participants

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