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 — Frequency plot improvements by murrayrm · Pull Request #1011 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Jun 16, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This PR adds a number of improvements for frequency plots (Bode, Nyquist, etc). The changes are motivated by my experience in using python-control for a course, where I found a few things that were harder to do than they should have been.

Changes in this PR:

  • Allow label keyword in frequency response commands to override default label generation.

  • Fix bug in processing indent_radius keyword when nyquist_plot is passed a system (keyword was not getting sent through to nyquist_response).

  • Restore functionality that allows omega to be specified as a list of 2 elements (indicating a range) in all frequency response/plotting routines. This used to work for nyquist but got removed at some point. It now works for all frequency response commands.

    • Note: this could break some code if someone meant to evaluate the frequency response at exactly two frequencies. This case seems rare, but it can be fixed by passing the two elements as an array.
  • Fix up the ax keyword processing to allow arrays or lists + uniform processing in all freqplot routines.

  • Fix up rcParams processing (uniformity + remove unnecessary code). (This needs more work in a future pass to eliminate the rcParams keywords that are part of frequency plotting functions and to pull separate time and frequency plot parameters into a single place. There are some TODOs in the code marking what needs to be done.)

  • Added new suptitle() function to add titles that are better centered (on axes instead of figure). Using this for frequency domain plots for now; will update for time domain plots as well.

  • Set up frd as factory function with keywords, including setting the signal/system names. Calls to ct.FRD should how be replaces with ct.frd, though the former will still work.

  • Allow Bode and Nyquist plots for FRD systems with different omega vectors (Fix nyquist plotting from FrequencyResponseData #996) as well as mixtures of FRD and other LTI systems.

  • Added a notebook examples/cds110-bode_nyquist.ipynb that shows a lot of the new functionality in the context of frequency domain analysis.

  • Minor: changed the way that freq_label (frequency axis label) is implemented so that it can be more easily overriden (use fstring with '{units}' instead of '%' => can put in custom units).

For the most part these changes just make it easier to do things that were already possible. Example of some new code that makes use of these features (from examples/cds110-bode_nyquist.ipynb):

import control as ct
P = ct.tf([1], [1, 0.1, -1])  # normalized inverted pendulum
C = ct.tf([2, 10], [1])       # PD controller
L = P * C                     # Loop transfer function

# Bode and Nyquist plots
plt.figure(figsize=[7, 4])
ax1 = plt.subplot(2, 2, 1)
plt.title("Bode plot for L", size='medium')
ax2 = plt.subplot(2, 2, 3)
ct.bode_plot(
    [P, C, L], label=['P', 'C', 'L'], ax=[ax1, ax2],
    initial_phase=0)

ax3 = plt.subplot(1, 2, 2)
ct.nyquist_plot(L, ax=ax3)
plt.title("Nyquist plot for L", size='medium')

ct.suptitle("Loop analysis for inverted pendulum")
plt.tight_layout()

bode_nyquist

For the mixed FRD functionality (motivated by comments I made in #996), here's an example showing how Nyquist plots can be combined in ways that weren't possible before:

s = ct.tf('s')
sys = ct.tf(
    (0.02 * s**3 - 0.1 * s) / (s**4 + s**3 + s**2 + 0.25 * s + 0.04),
    name='tf')
sys1 = ct.frd(sys, np.logspace(-1, 1, 15), name='frd1')
sys2 = ct.frd(sys, np.logspace(-2, 2, 20), name='frd2')

ct.nyquist_plot([sys, sys1, sys2])
ct.suptitle("Mixed FRD, tf data")

frd_nyquist

Sorry, something went wrong.

Uh oh!

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

All reactions
@coveralls

coveralls commented Jun 16, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.502% (+0.01%) from 94.49%
when pulling 4b3597f on murrayrm:freqresp_improvements-16Apr2024
into dbc998d on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

murrayrm added 6 commits June 26, 2024 08:50
@murrayrm
allow label keyword in bode_plot() to override default labels
b9acc99
@murrayrm
add label keyword to singular_value_plot and nyquist_plot
0b6348f
@murrayrm
add additional tests for freqplot label override + small fixes
606cf48
@murrayrm
don't strip common parts of labels if given explicitly
5e405cc
@murrayrm
BUG: indent_radius not handled properly when passed to nyquist_plot
eeebec3
@murrayrm
restore [wmin, wmax] functionality + documentation updates
402b45f
@murrayrm
murrayrm force-pushed the freqresp_improvements-16Apr2024 branch from 4b3597f to ffee41f Compare June 26, 2024 15:51
@coveralls

coveralls commented Jun 26, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.535% (+0.01%) from 94.522%
when pulling ffee41f on murrayrm:freqresp_improvements-16Apr2024
into ad6b49e 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 June 27, 2024 16:11
@slivingston slivingston self-assigned this Jun 27, 2024
slivingston
slivingston reviewed Jun 28, 2024
View reviewed changes
Comment thread control/frdata.py Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 28, 2024
View reviewed changes
Comment thread control/ctrlplot.py
fig.suptitle(title, **kwargs)

elif frame == 'axes':
# TODO: move common plotting params to 'ctrlplot'

@slivingston slivingston Jun 28, 2024

Copy link
Copy Markdown
Member

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.

This same TODO comment is already freqplot.py, and it is not clear what the comment means here (in ctrlplot). Should the TODO comment be deleted here in favor of the one in freqplot.py ?

Sorry, something went wrong.

Uh oh!

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

All reactions

@slivingston slivingston Jun 28, 2024

Copy link
Copy Markdown
Member

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.

Reading it again, I think this TODO comment is to be able to

rcParams = config._get_param('ctrlplot', 'rcParams', rcParams)

If so, then I understand the motivation, and OK to keep both TODO comments.

Sorry, something went wrong.

Uh oh!

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

All reactions

@murrayrm murrayrm Jun 29, 2024

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.

I've got another PR coming that will address both TODOs. Doing things in stages so that everything is in (slightly) smaller chunks.

Sorry, something went wrong.

Uh oh!

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

All reactions

@slivingston slivingston Jun 29, 2024

Copy link
Copy Markdown
Member

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.

Awesome. I am almost done reviewing this one.

Sorry, something went wrong.

Uh oh!

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

All reactions
slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread doc/plotting.rst Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread control/freqplot.py Resolved

Uh oh!

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

slivingston
slivingston reviewed Jun 29, 2024
View reviewed changes
Comment thread control/freqplot.py Resolved

Uh oh!

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

@coveralls

coveralls commented Jun 29, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.518% (-0.004%) from 94.522%
when pulling 29f3713 on murrayrm:freqresp_improvements-16Apr2024
into ad6b49e on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@slivingston

slivingston commented Jun 29, 2024

Copy link
Copy Markdown
Member

Excellent improvements! I did not find any technical errors, but here are a few misprints in examples/cds110_bode-nyquist.ipynb:

  • "the way that basic" -> "the way basic"
  • The sentence that begins with "The system parameters are given by" should have \epsilon = 0.01, because the sentence continues with "and we assume that" on the next line.
  • "loop dynamicsof" -> "loop dynamics of"
  • "For how we just" -> "For now we just"
  • To get TeX rendering in section "Stability margins": $|1 - L(j\\omega) -> $|1 - L(j\\omega)$
  • "interpreation" -> "interpretation"

OK to leave it as-is, but one of the commit messages has a misprint: "reectoring/regularization of ax keyword processing" should have "refactoring".

After you consider/apply the above, it is ready to merge.

All reactions

Sorry, something went wrong.

Uh oh!

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

murrayrm added 7 commits June 29, 2024 11:12
@murrayrm
refactoring/regularization of ax keyword processing
6f6c70d
@murrayrm
regularize processing of rcParams
404fbdf
@murrayrm
add suptitle() function for better centered titles
1a94f4e
@murrayrm
updated freqplot documentation + figures
28995f1
@murrayrm
move plotutil to ctrlplot
ffb0a0f
@murrayrm
update freq_label handling + add Bode/Nyquist notebook
4734912
@murrayrm
set up frd() as full factory function for FRD I/Osystems
aeb187f
murrayrm added 5 commits June 29, 2024 11:12
@murrayrm
update unexpected keyword check for GitHub action variation
ddb5b7c
@murrayrm
allowed Bode/Nyquist for mixed FRD systems
2fde963
@murrayrm
update CDS 110 example
b7c158b
@murrayrm
fix typos in doc, docstrings pointed out by @slivingston review
59a676e
@murrayrm
fix issues in examples/cds110_bode-nyquist.ipynb per @slivingston review
343df2c
@murrayrm
murrayrm force-pushed the freqresp_improvements-16Apr2024 branch from 29f3713 to 343df2c Compare June 29, 2024 18:27
@coveralls

coveralls commented Jun 29, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.518% (+0.01%) from 94.506%
when pulling 343df2c on murrayrm:freqresp_improvements-16Apr2024
into e5394c4 on python-control:main.

All reactions

Sorry, something went wrong.

Uh oh!

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

@murrayrm
murrayrm merged commit feeb56a into python-control:main Jun 29, 2024
@murrayrm
murrayrm deleted the freqresp_improvements-16Apr2024 branch June 29, 2024 18:40
@murrayrm murrayrm mentioned this pull request Jun 29, 2024
Time response plot improvements #1018
Merged
@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
@murrayrm murrayrm mentioned this pull request Jun 30, 2024
Version 0.10.1 release plans #1020
Closed
11 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston left review comments

Uh oh!

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

Assignees

@slivingston slivingston

Labels

None yet

Projects

None yet

Milestone

0.10.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

3 participants

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