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

Optimize 3D display#6085

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

Closed

Conversation

AlexandreAbraham
Copy link

While printing 3D objects, I noticed that the display was pretty slow. The reason is that matplotlib relies a lot on list of objects instead of using big numpy arrays to fasten 3D projections.

This PR aims at solving part of this problem. I replaced some airthmetic on list of vectors by artihmetic on big matrices. This impacts mainly 3D display.

I work on neuroimaging data. I noticed an approximate 25% speedup optimization on 3D display (depending on the complexity of the figure). Refreshment in interactive plotting is also faster. Example script:

frommatplotlibimportpyplotaspltfrommpl_toolkits.mplot3dimportAxes3Dfromnibabelimportgiftisurf_mesh='/path/to/spm12b/canonical/cortex_20484.surf.gii'coords,faces=gifti.read(surf_mesh).getArraysFromIntent(1008)[0].data, \gifti.read(surf_mesh).getArraysFromIntent(1009)[0].datalimits= [coords.min(),coords.max()]cmap='coolwarm'fig=plt.figure()ax=fig.add_subplot(111,projection='3d',xlim=limits,ylim=limits)ax.view_init(elev=0,azim=0)ax.set_axis_off()p3dcollec=ax.plot_trisurf(coords[:,0],coords[:,1],coords[:,2],triangles=faces,linewidth=0.,antialiased=False,color='white')fig.savefig('brain.png')plt.close(fig)

Possible other optimizations:

  • For display, matplotlib relies on a list of Path objects. This could be made faster with numpy arrays.
  • Poly3DCollection and others: a lot of unnecessary asarray and paddings of ones to obtain quaternions are done. This could be simplified by keeping a quaternion numpy array in the object and using appropriate numpy views but requires a deeper refactoring.
  • not plotting unseen triangle: For the moment, all polygons are drawn. if there are a lot, isn't it worth to compute which one are hidden by others and remove them?

@agramfort and@juhuntenburg, do you notice significant running time reduction with this PR?

@tacaswell
Copy link
Member

attn@WeatherGod

@WeatherGod
Copy link
Member

You have a lot of test failures:https://travis-ci.org/matplotlib/matplotlib/jobs/112810572

minz = 1e9
for (xs, ys, zs) in xyslist:
minz = min(minz, min(zs))
minz = np.min(xys[:, :, 2])
Copy link
Member

Choose a reason for hiding this comment

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

We are not guaranteed a non-empty array, hence the initialization of minz with 1e9.

@kingjr
Copy link

@AlexandreAbraham any progress on this front?

@choldgraf
Copy link
Contributor

+1 to this. I still haven't found a good method for plotting 3-D brains w/ points on top of it in python. Making a trisurf is super slow when I have lots of triangles.

@AlexandreAbraham
Copy link
Author

Hi,
Yes, I have understood why the tests were failing. I have unfortunately made some assumptions on the format of input data that turned out to be wrong. The optimization is still possible but requires more work. I had no time so far to fix it but I'll give it a try today.

@tacaswell
Copy link
Member

'power cycled' to restart the CI against current master.

@tacaswelltacaswell added this to the2.1 (next point release) milestoneJul 16, 2016
@jenshnielsen
Copy link
Member

Looks like there is a problem in one of the examples

Warning, treated as error:/home/travis/build/matplotlib/matplotlib/doc/examples/mplot3d/pathpatch3d_demo.rst:8: WARNING: Exception occurred in plotting pathpatch3d_demo from /home/travis/build/matplotlib/matplotlib/doc/mpl_examples/mplot3d/pathpatch3d_demo.py:Traceback (most recent call last):  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/sphinxext/plot_directive.py", line 654, in render_figures    figman.canvas.figure.savefig(img.filename(format), dpi=dpi)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1666, in savefig    self.canvas.print_figure(*args, **kwargs)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backend_bases.py", line 2227, in print_figure    **kwargs)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 517, in print_png    FigureCanvasAgg.draw(self)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 464, in draw    self.figure.draw(self.renderer)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 68, in draw_wrapper    return draw(artist, renderer, *args, **kwargs)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1262, in draw    renderer, self, dsu, self.suppressComposite)  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/image.py", line 139, in _draw_list_compositing_images    a.draw(renderer)  File "/home/travis/build/matplotlib/matplotlib/lib/mpl_toolkits/mplot3d/axes3d.py", line 279, in draw    for patch in self.patches]  File "/home/travis/build/matplotlib/matplotlib/lib/mpl_toolkits/mplot3d/axes3d.py", line 279, in <listcomp>    for patch in self.patches]  File "/home/travis/build/matplotlib/matplotlib/lib/mpl_toolkits/mplot3d/art3d.py", line 320, in do_3d_projection    s = np.vstack(self._segments3d, np.ones(self._segments3d.shape[1]))AttributeError: 'PathPatch3D' object has no attribute '_segments3d'

@anntzer
Copy link
Contributor

I guess my point is more "let's not add more public API".

NelleV reacted with thumbs up emoji

@NelleV
Copy link
Member

Hi@AlexandreAbraham
I think this is almost ready to be merged. Can you make the new public function private and rebase? The rebase might be quite painful…
If you are too busy with the new job, I can take over your branch :)

@NelleVNelleV self-assigned thisDec 18, 2016
@mwaskom
Copy link

Hi, I ended up here from this paper:https://riojournal.com/articles.php?id=12342

I'd be very interested in using the method in the paper, but I'm finding the speed that it can be accomplished with the current implementation of 3d meshses is a dealbreaker. Is there hope for this PR moving forward?

@choldgraf
Copy link
Contributor

+1 to this PR moving forward

@WeatherGod
Copy link
Member

WeatherGod commentedJul 13, 2017 via email

ok, I'll make it a priority this week during SciPy.
On Wed, Jul 12, 2017 at 10:11 AM, Chris Holdgraf ***@***.***> wrote: +1 to this PR moving forward — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6085 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-Lv2kePQIHFLTlgb98P4Wy33BQm1ks5sNNQCgaJpZM4HmSzk> .
mwaskom reacted with hooray emoji

@tacaswell
Copy link
Member

I think that this is the PR that@GaelVaroquaux mentioned to me earlier this week.

@choldgraf
Copy link
Contributor

lmk if you want someone to usertest this at the sprint. I can bring some brain surfaces :-)

@WeatherGod
Copy link
Member

There are several unaddressed comments from@Kojoley . I am currently working on rebasing this as well. I'll force-push a rebase soon (if github allows me).

@WeatherGod
Copy link
Member

doesn't look like github will let me, so, I'll do it the old way and submit a PR against your branch.

@WeatherGod
Copy link
Member

hmmph, that won't work, either.@AlexandreAbraham , do you see any option to allow maintainers to push changes to your PR on this page?

@tacaswelltacaswell modified the milestones:2.2 (next next feature release),2.1 (next point release)Jul 16, 2017
self._segments3d_data = np.vstack(segments)
# Add a fourth dimension for quaternions
self._segments3d_data = np.hstack([self._segments3d_data,
np.ones((n_segments, 1))])
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be marginally better as

self._segments3d_data  = np.empty((n_segments, 4))self._segments3d_data[:,:3] = segmentsself._segments3d_data[:,3] = 1

Also, are you sure you mean "quaternion" and not "homogenous coordinate"?

cum_s = 0
for s in self._seg_sizes:
segments_2d.append(xys[cum_s:cum_s + s, :2])
cum_s += s
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this loop isnp.split in some form

tis = vecw[1] < 1
vecw /= vecw[3]
# Integrating tis in the numpy array for optimization purposes
vecw[3, :] = tis
Copy link
Contributor

Choose a reason for hiding this comment

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

Get rid of this colon - it makes the function fail on 1d inputs

Copy link
Contributor

@eric-wiesereric-wieser left a comment

Choose a reason for hiding this comment

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

Storing coordinates in the first axis of the array is kind of unusual (see hownp.linalg andscipy.misc.imread behave).

Perhaps these should all bexyz[..., 0] instead ofxyz[0]?

@jklymakjklymak modified the milestones:needs sorting,v3.0May 2, 2018
@jklymak
Copy link
Member

This seems to have attracted lots of interest, but eventually never made it over the hump.@WeatherGod is there still some hope for this, or has its momment passed?

@WeatherGod
Copy link
Member

WeatherGod commentedMay 2, 2018 via email

Perhaps parts of this needs to be separated out into smaller PRs. There issome good stuff here, but there were also some questionable stuff, too,that never got addressed.
On Wed, May 2, 2018 at 1:10 PM, Jody Klymak ***@***.***> wrote: This seems to have attracted lots of interest, but eventually never made it over the hump.@WeatherGod <https://github.com/WeatherGod> is there still some hope for this, or has its momment passed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6085 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARy-ENj7x-jsaEbU7onzowiCV_Giuu8ks5tueiYgaJpZM4HmSzk> .

@jklymak
Copy link
Member

jklymak commentedMay 2, 2018
edited
Loading

Closing as inactive. The goodies are here for anyone who wants them!

@jklymakjklymak closed thisMay 2, 2018
@QuLogicQuLogic removed this from thev3.0 milestoneMay 2, 2018
@rth
Copy link
Contributor

rth commentedJul 5, 2018

This seems to have attracted lots of interest, but eventually never made it over the hump
Closing as inactive.

If it's closed, what's the mechanism then for contributors to know that this PR is stalled, needs help and can be continued ?

do you see any option to allow maintainers to push changes to your PR on this page?

Should be allowed by Github by default now.

I'll make it a priority this week during SciPy. [in 2017]

Maybe someone might be interested in looking into it during the scipy sprint next week?

@rth
Copy link
Contributor

rth commentedJul 5, 2018

If I understand correctly this PR rebased (as of July 2017) can be found in@WeatherGod 'shttps://github.com/WeatherGod/matplotlib/tree/pr/6085 branch.

rth pushed a commit to rth/matplotlib that referenced this pull requestJul 5, 2018
@rthrth mentioned this pull requestJul 5, 2018
10 tasks
@rth
Copy link
Contributor

rth commentedJul 15, 2018

This PR was continued in#11577

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@eric-wiesereric-wiesereric-wieser requested changes

@anntzeranntzeranntzer left review comments

@KojoleyKojoleyKojoley left review comments

Assignees

@NelleVNelleV

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

15 participants
@AlexandreAbraham@tacaswell@WeatherGod@kingjr@choldgraf@jenshnielsen@Kojoley@anntzer@NelleV@mwaskom@jklymak@rth@eric-wieser@mdboom@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp