Skip to content

Fix timebase bug in InterconnectedSystem (issue #754) #755

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 6, 2022

Conversation

murrayrm
Copy link
Member

@murrayrm murrayrm commented Aug 4, 2022

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.

@murrayrm
Copy link
Member Author

murrayrm commented Aug 4, 2022

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??

(None, 0.1, 0.1),
(None, None, None),
(0.2, None, 0.2),
(0.2, 0.1, ValueError),
Copy link
Contributor

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Great suggestion. Done.

@murrayrm murrayrm force-pushed the timebase_bug-03Aug2022 branch from 229bfe6 to cf30479 Compare August 6, 2022 04:37
@coveralls
Copy link

Coverage Status

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

@bnavigator bnavigator merged commit b9cf384 into python-control:main Aug 6, 2022
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants