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 — Fix timebase bug in InterconnectedSystem (issue #754) by murrayrm · Pull Request #755 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Aug 4, 2022

Copy link
Copy Markdown
Member

This PR fixes the timebase bug identified in issue #754. The fix was to modify the processing of the timebase in InterconnectedSystem so that it looks at the systems for the default timebase.

Sorry, something went wrong.

Uh oh!

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

murrayrm commented Aug 4, 2022

Copy link
Copy Markdown
Member Author

Conda-based builds are failing. I checked and this is also happening on the current main branch. This may be something associated with Slycot 0.5.0??
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm murrayrm mentioned this pull request Aug 4, 2022
CI tests are failing (xvfb issue?) #756
Closed
sawyerbfuller
sawyerbfuller reviewed Aug 4, 2022
View reviewed changes
Comment thread control/tests/timebase_test.py
(None, 0.1, 0.1),
(None, None, None),
(0.2, None, 0.2),
(0.2, 0.1, ValueError),

@sawyerbfuller sawyerbfuller Aug 4, 2022

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.

Great PR. Would be good to add tests for dt=True, which are compatible with all other discrete-time timebases. https://python-control.readthedocs.io/en/0.9.2/conventions.html

Sorry, something went wrong.

Uh oh!

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

@murrayrm murrayrm Aug 4, 2022

Copy link
Copy Markdown
Member Author

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.

Great suggestion. Done.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@murrayrm murrayrm mentioned this pull request Aug 6, 2022
Switch CI to mambaforge and conda-forge channel #757
Merged
murrayrm added 3 commits August 5, 2022 21:37
@murrayrm
fix timebase bug in InterconnectedSystem (issue python-control#754)
f17c919
@murrayrm
add additional checks for overriding timebase
3c13f2c
@murrayrm
added tests for dt=True case per @sawyerbfuller suggestion
cf30479
@murrayrm
murrayrm force-pushed the timebase_bug-03Aug2022 branch from 229bfe6 to cf30479 Compare August 6, 2022 04:37
@coveralls

coveralls commented Aug 6, 2022

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.0008%) to 94.654% when pulling cf30479 on murrayrm:timebase_bug-03Aug2022 into a73a03d on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnavigator
bnavigator merged commit b9cf384 into python-control:main Aug 6, 2022
@bnavigator bnavigator mentioned this pull request Aug 6, 2022
Bug: Incorrect ValueError: incompatible timebases #754
Closed
@murrayrm murrayrm added this to the 0.9.3 milestone Dec 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sawyerbfuller sawyerbfuller sawyerbfuller left review comments

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

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

4 participants

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