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 processing in frd, zpk by murrayrm · Pull Request #1064 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Nov 23, 2024

Copy link
Copy Markdown
Member

This PR addresses issue #1062 by making sure that timebases are handled uniformly in all system creation factory functions:

  • For ss, tf, frd, and zpk, use either an additional argument specifying the sampling time or use the dt keyword (using both generates a warning).
  • For nlsys, use the dt keyword
  • If no timebase is specified, use the default timebase set by config.defaults['control.default_dt'], which is 0 (continuous time) unless you change it.
    It was the last of these that was the problem in zpk: it was defaulting to None and that was later interpreted as a discrete time system.

Also added unit tests to make sure everything is consistent across the different factory functions.

Sorry, something went wrong.

Uh oh!

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

All reactions
murrayrm added 2 commits November 20, 2024 20:45
@murrayrm
fix timebase processing in frd, zpk
f25e625
@murrayrm
make frd timebase duplication testing consistent + unit tests
8b29e45
@murrayrm murrayrm linked an issue Nov 23, 2024 that may be closed by this pull request
Different impulse responses for zpk vs tf #1062
Closed
@coveralls

coveralls commented Nov 23, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.696% (+0.004%) from 94.692%
when pulling 8b29e45 on murrayrm:fix_frd_timebase-20Nov2024
into d810d5a on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston
slivingston self-requested a review November 23, 2024 03:56
slivingston
slivingston approved these changes Nov 24, 2024
View reviewed changes
@bnavigator
bnavigator merged commit 57b5307 into python-control:main Nov 24, 2024
@murrayrm murrayrm mentioned this pull request Nov 27, 2024
Unexpected behavior for a system defined using zpk #1067
Closed
@murrayrm murrayrm mentioned this pull request Jan 19, 2025
unexpected behaviour of feedback combined with step_response #1102
Closed
@murrayrm murrayrm added this to the 0.10.2 milestone Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Development

Successfully merging this pull request may close these issues.

Different impulse responses for zpk vs tf

Uh oh!

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

4 participants

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