Skip to content

3D plotting performance improvements #29397

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jan 28, 2025
Merged

3D plotting performance improvements #29397

merged 14 commits into from
Jan 28, 2025

Conversation

scottshambaugh
Copy link
Contributor

@scottshambaugh scottshambaugh commented Jan 3, 2025 •
edited
Loading

PR summary

This builds off of #16688, to rebase off of main, extend as applicable to all the 3D object types, and allow for compatibility with the new 3D axlim clipping. With the test code below, I am seeing substantial speedups in draw times for some types of plots.

Closes #16659

import time
import numpy as np
import matplotlib.pyplot as plt

fig, ax = plt.subplots(subplot_kw={"projection": "3d"})

x = y = np.linspace(-1, 1, 100)
X, Y = np.meshgrid(x, y)
Z = X ** 2 + Y ** 2

print("Timing...")

start_time = time.perf_counter()
#ax.plot_surface(X, Y, Z, rstride=1, cstride=1, axlim_clip=False)
#ax.scatter(x, y, np.ones_like(x), axlim_clip=False)
ax.plot_wireframe(X, Y, Z, axlim_clip=False)
#ax.bar3d(X.ravel(), Y.ravel(), 0, 0.01, 0.01, 1, axlim_clip=False)
ax.set_xlim(0, 1)
fig.canvas.draw()
end_time = time.perf_counter()

plt.close()
print(f"Time taken: {end_time - start_time:.4f} seconds")
function object draw time main this PR improvement
plot_surface (400x400) Poly3DCollection 16.55 sec 1.86 sec 8.9x
scatter (100000) Path3DCollection 3.65 sec 3.22 sec 1.1x
plot wireframe (8000x8000) Line3DCollection 2.72 sec 1.39 sec 2.0x
bard3d (100x100) Patch3DCollection 7.17 sec 2.02 sec 3.5x

PR checklist

@scottshambaugh scottshambaugh added this to the v3.11.0 milestone Jan 3, 2025
@scottshambaugh scottshambaugh force-pushed the masked_array_performance branch from 4527293 to 2f2e65c Compare January 3, 2025 05:07
@scottshambaugh
Copy link
Contributor Author

scottshambaugh commented Jan 5, 2025 •
edited
Loading

Thoughts on if the 3D speedups warrant a what's new?

@timhoffm
Copy link
Member

timhoffm commented Jan 6, 2025

Thoughts on if the 3D speedups warrant a what's new?

Since these are substantial, I‘d say yes.

@scottshambaugh scottshambaugh changed the title 3D masked array performance improvements 3D plotting performance improvements Jan 6, 2025
@scottshambaugh scottshambaugh force-pushed the masked_array_performance branch from d51b947 to 1516928 Compare January 6, 2025 00:32
greglucas
greglucas reviewed Jan 7, 2025
View reviewed changes
lib/mpl_toolkits/mplot3d/art3d.py
# Some faces might contain masked vertices, so we want to ignore any
# errors that those might cause
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these errors be handled within the _proj_transform_vectors case directly closer to where these errors would actually arise? I know there has been some work in numpy to not raise invalid/division warnings on masked elements, so is this still an issue now?

Copy link
Contributor Author

@scottshambaugh scottshambaugh Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I removed the error ignore line and did not see any issues. The tests cover partially masked Poly3DCollections, so that case is exercised, and it looks like the improved masked handling covers any previous errors here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like the error does crop up in the AppVeyor build, so adding the original error suppression back in. I don't want to put it in _proj_transform_vectors since it might not always be the case that we know that we have masked out the problematic vertices outside of that function.

@scottshambaugh scottshambaugh force-pushed the masked_array_performance branch from 927e500 to 645dfc8 Compare January 7, 2025 18:53
@scottshambaugh scottshambaugh mentioned this pull request Jan 7, 2025
Closed
6 tasks
greglucas
greglucas reviewed Jan 7, 2025
View reviewed changes
Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

I think this is a really great performance improvement and shows how much of an impact numpy vectorization can have. Just a few minor nits that you can take or leave.

@scottshambaugh scottshambaugh force-pushed the masked_array_performance branch 2 times, most recently from f6356c0 to 14e26f9 Compare January 8, 2025 18:12
@scottshambaugh scottshambaugh force-pushed the masked_array_performance branch from 14e26f9 to 68b8a6c Compare January 24, 2025 16:40
timhoffm
timhoffm reviewed Jan 24, 2025
View reviewed changes
@timhoffm timhoffm merged commit 29f3a5c into matplotlib:main Jan 28, 2025
40 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timhoffm timhoffm timhoffm approved these changes

@greglucas greglucas greglucas approved these changes

Assignees
No one assigned
Labels
Performance topic: mplot3d
Projects
None yet
Milestone
v3.11.0
Development

Successfully merging this pull request may close these issues.

Speeding up Axes3D.plot_surface 4-8x
3 participants
@scottshambaugh @timhoffm @greglucas