Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improvements to Nichols chart plotting#723

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

roryyorke
Copy link
Contributor

These are relatively minor updates to the Nichols chart (AKA grid) plotting.

It's far from ideal; further possible improvements:

  • more tightly fitting the chart to the view (or data) xlim and ylim (the "extent"); the pain here is figuring out which contours to plot, and where to place the labels
    • depending on the limits, some contours will end up with a two visible segments
  • recreating the axis on zoom and pan events
  • a deluxe version of this would be too add (secondary?) axes, and show the closed-loop gain and phase in the "data value" text (the "x= , y = " on the Matplotlib toolbar)
  • I wonder if we could use a customContourSets, and plug into however that labelling is done?

Here are some before-and-after pictures (left is before) of a few example systems.

The new code gives a better result in almost all respects. One visible problem is that the closed-loop phase label for "0°" clashes with the "-40dB" (and similar) closed-loop gain label. The gains become crowded when the gain span is large (final figure), but that is the same in both versions.

I haven't added tests; I'll see how much time I have to do that. Easyish tests:

  • check for "tight" phase limits (see first example below)
  • check that the axes child text elements are have clip_on (I'd have to not check title & tick labels, presume this is possible)
  • check for existence of phase labels, contingent on label_cl_phases flag
  • test both branches of theif in_inner_extents, and in fact test the behaviour in general by setting xlim & ylim before calling nichols_grid.
    Please suggest other tests.

example1
example2
example3
example4

Details (commit message)

Clip closed-loop contour labels.

Add labels for constant closed-loop phase contours.

Use smaller of data or view extents when deciding on how big, in
phase, a chart to create.

Use more uniformly spaced closed-loop phase contours, and use more
widely-spaced contours when phase extent is large.

Add optionalax argument, for axes to add grid to.

Clip closed-loop contour labels.Add labels for constant closed-loop phase contours.Use smaller of data or view extents when deciding on how big, inphase, a chart to create.Use more uniformly spaced closed-loop phase contours, and use morewidely-spaced contours when phase extent is large.Add optional `ax` argument, for axes to add grid to.
@coveralls
Copy link

coveralls commentedApr 16, 2022
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 94.31% when pulling59cd872 on roryyorke:rory/nichols-improvements into2102181 on python-control:master.

@murrayrm
Copy link
Member

Changes look good. It might be worth adding some unit tests to cover the new features (eg, use ofax keyword).

@roryyorke
Copy link
ContributorAuthor

OK, I'll look at some tests, including testing theax argument.

I'll probably wrap up the linfnorm PR first, and I'm not sure when I'll get this PR done.

@roryyorke
Copy link
ContributorAuthor

I changed nichols_grid to return the artists it adds; this made testing much easier, but also makes it possible to customize the grid appearance and labels. Below is a not-especially-compelling example; more realistically one might change the intensity or alpha of the grid and labels.

plt.clf()mc,nc,ml,nl=ct.nichols_grid()fori,(c,l)inenumerate(zip(nc,nl)):nc.set_color(f'C{i}')nl.set_color(f'C{i}')fori,(c,l)inenumerate(zip(mc,ml)):c.set_color(f'C{i}')l.set_color(f'C{i}')

color-nichols

@murrayrm
Copy link
Member

This looks good to go to me.@roryyorke: any additional changes that you plan to make?

@roryyorke
Copy link
ContributorAuthor

roryyorke commentedApr 30, 2022 via email

No, all done.
On Sat, 30 Apr 2022, 07:05 Richard Murray, ***@***.***> wrote: This looks good to go to me.@roryyorke <https://github.com/roryyorke>: any additional changes that you plan to make? — Reply to this email directly, view it on GitHub <#723 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3C7TMVNROIIOFAL7I2EDVHS5QRANCNFSM5TSN3NBA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

@bnavigatorbnavigator merged commitae8d586 intopython-control:masterApr 30, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@bnavigatorbnavigatorbnavigator left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@roryyorke@coveralls@murrayrm@bnavigator

[8]ページ先頭

©2009-2025 Movatter.jp