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

Fixplot_wireframe with nonequalrstride,cstride, plus additional speedups#29435

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

scottshambaugh
Copy link
Contributor

@scottshambaughscottshambaugh commentedJan 8, 2025
edited
Loading

PR summary

I accidentally introduced a regression in#29399, which broke wireframe plots when there are an unequal number of rows and columns. That case forms ragged data which can not be concatenated into a single numpy array. This fixes that, and adds an image test. But the fix isn't ideal, and there are a few different paths we could take:

  1. The current solution of creating an array large enough to hold both differently-shaped row and column arrays introduces a potentially large number of NaN points in the returned Line3DCollection, which might be an issue for users that then postprocess this data.
  2. An alternative would be to generate and return separate Line3DCollection's for the rows and columns, but that changes the return signature.
  3. Potentially we could modify Line3DCollection to handle groups of several collections internally? Seems messy though.
  4. We could also partially revert the previous PR and return to ragged lists for this data, but lose the vectorization / draw speed improvements.

Note that this issue isn't present for other collections such as Poly3DCollection because those do not plot a higher density of data on the lines in-between grid points like Line3DCollection does.

There is also a performance improvement here. In the previous PR I identified the autoscaling of the axes as a large time sink, but didn't realize that there was lower hanging fruit. Instead of autoscaling based on all the X, Y, Z data passed in, we now only autoscale based on thevisible data. For my example test of a 4000x4000 wireframe at the default rstride, cstride = 50, this reduces the data parsed for autoscaling from 3x4000x4000 to 3x4000x100. There is a slight change in behavior in that axis limits will no longer bound data which are skipped over between lines, which changes one of the baseline image tests.

Before, majority of plot time (3.5 seconds) dedicated to autoscaling:
image

After, 20milliseconds dedicated to autoscaling:
image

PR checklist

@scottshambaughscottshambaugh added topic: mplot3d Performance PR: bugfixPull requests that fix identified bugs labelsJan 8, 2025
@scottshambaughscottshambaugh added this to thev3.11.0 milestoneJan 8, 2025
@scottshambaughscottshambaugh changed the titleFix plot_wireframe with nonequal rstride, cstride, plus additional speedupsFixplot_wireframe with nonequalrstride,cstride, plus additional speedupsJan 8, 2025
@scottshambaugh
Copy link
ContributorAuthor

scottshambaugh commentedJan 24, 2025
edited
Loading

Thanks to the dev team for talking through this on the call today! It turns out that the approach there works well - keep the original data held as a ragged list in the Line3DCollection, and then just autoscale twice with the vectorized column and row data.

Speed to draw a 8000x8000 wireframe plot:

  • v3.10: 6.82 sec
  • main: 4.00 sec
  • this PR: 0.80 sec

The autoscaling still takes up a decent chunk of time (170 ms) relative to the draw routines, but it's vastly improved.
image

timhoffm reacted with hooray emojircomer reacted with rocket emoji

@oscargusoscargus merged commitac77fea intomatplotlib:mainJan 24, 2025
38 of 41 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Labels
PerformancePR: bugfixPull requests that fix identified bugstopic: mplot3d
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

3 participants
@scottshambaugh@timhoffm@oscargus

[8]ページ先頭

©2009-2025 Movatter.jp