MRG: Pyvista background plotter #6551

GuillaumeFavelier · 2019-07-10T13:24:08Z

Fixes the last part of #6518.

This PR adds the missing _set_3d_view() and _set_3d_title() functions to the pyvista 3d backend and switches to BackgroundPlotter.

In order to avoid regression, I need to investigate:

1) _Brain object features (my results are here)
2) interactivity inside a jupyter notebook (paused for now)

State of testing:

Catch DeprecationWarning coming from ipywidgets (see also FIX,VIZ: Fix DeprecationWarning in pyvista backend #6578)
test_plot_alignment() becomes a slow_test (see also VIZ, BUG: test_plot_alignment timeout #6566)
Enable snapshot_brain_montage (my results are here)
Use set_3d_backend to facilitate 3d backend transition
Camera settings are identical to Mayavi from now on
PyVista version upgraded to 0.21.3
Azure CONDA_VERSION upgraded (see also TST: Testing CIs for LinkError on Azure #6590)
Fix tutorials/misc/plot_ecog.py
Fix the bug causing the 'tube' glyph to have incorrect radius.

Build examples/tutorials with pyvista:

Test 1 failed with timeout
Test 2 failed with no output, memory issue?, related to #310
Test 3 failed with segfault
Test 4 failed with TypeError
Test 5, 6 and 7 failed during make test-doc (fixed by BUG: Restore conftest to doctests #6599)
Test 8 was killed by the system (related to #327 and #338, waiting for reviews)
Test 9 failed with RuntimeError but circle uploaded its results (fixed by 38390ff)
Test 10 is a success.

To do next:

Use the BackgroundPlotter for testing now that offscreen rendering is available.
Port the mne/report.py to use the 3d backend abstraction layer
Improve lighting (see this comment)

Known issues:

missing window title (fixed by #305)
visual artifact at the first frame fixed after interaction/refresh (example below, #315 didn't fix this)
hanging windows while building the doc with make html_dev (fixed by #293)
camera not updated initially, manually resizing the window can solve it (found here, fixed by #315)
offscreen rendering is still experimental (fixed by #309)

lgtm-com · 2019-07-10T14:01:39Z

This pull request introduces 2 alerts when merging e8d2d8e into 26231ff - view on LGTM.com

new alerts:

2 for Unused local variable

codecov · 2019-07-10T15:21:16Z

Codecov Report

Merging #6551 into master will increase coverage by <.01%.
The diff coverage is 87.17%.

@@            Coverage Diff            @@
##           master   #6551      +/-   ##
=========================================
+ Coverage    89.4%   89.4%   +<.01%     
=========================================
  Files         415     416       +1     
  Lines       74985   75140     +155     
  Branches    12330   12347      +17     
=========================================
+ Hits        67039   67182     +143     
- Misses       5118    5123       +5     
- Partials     2828    2835       +7

GuillaumeFavelier · 2019-07-11T12:21:14Z

Starting by 1), these are examples using _Brain and BackgroundPlotter:

Basic Visualization

Activation Data Visualization

Display ROI Values

I have to report 2 bugs for now:

missing window title
visual artifact at the first frame fixed after interaction/refresh (example below)

This reverts commit e4f2b74.

GuillaumeFavelier · 2019-07-12T21:57:39Z

These are the results of the now available snapshot_brain_montage() function used in examples/visualization/plot_3d_to_2d.py:

PyVista	Mayavi

I have inconsistency with Mayavi on the second figure and the camera's distance is not correct for PyVista but this can be fixed later.

I have to report one bug:

The camera was not updated initially, I had to manually resize the window to obtain a satifying result:

Note: I used the master branch of PyVista, that's why windows have a title now

larsoner · 2019-07-12T22:24:32Z

That's great!

Bugs like the no refresh are probably not "our problem" (they are an upstream library bug) but there are often workarounds. We can work around them later. In the meantime if you can file a bug report with some minimal example to pyvista that's great.

GuillaumeFavelier · 2019-07-16T08:52:07Z

After discussing IRC with @larsoner and @agramfort, it seems that building the doc with the experimental version of the offscreen BackgroundPlotter could have some issues cleaning the memory. I will investigate further.

GuillaumeFavelier · 2019-07-31T14:15:29Z

With the last test, tutorials/misc/plot_ecog.py has failed with RuntimeError so I still need to fix this. But Circle was able to upload its results successfully, we can already check the examples and tutorials.

doc/conf.py

larsoner · 2019-07-31T15:02:16Z

tutorials/simulation/plot_dics.py

+    fig = brain._figures[0][0]
+    mne.viz.set_3d_view(fig, azimuth=0, elevation=0, distance=550,
+                        focalpoint=[0, 0, 0])
+    mne.viz.set_3d_title(fig, 'MNE-dSPM inverse (RMS)')


Instead just do brain.show_view https://pysurfer.github.io/generated/surfer.Brain.html#surfer.Brain.show_view

And maybe in stc.plot you could use time_label='MNE-dSPM inverse (RMS)' to set the title instead?

I had some trouble to make it work with time_label locally. The window's title wasn't updated for some reason and I obtained the "Mayavi Scene" instead. Maybe I did something wrong, I'll push my changes.

larsoner · 2019-07-31T15:03:57Z

tutorials/simulation/plot_dics.py

+        fig = brain._figures[0][0]
+        mne.viz.set_3d_view(fig, azimuth=0, elevation=0, distance=550,
+                            focalpoint=[0, 0, 0])
+        mne.viz.set_3d_title(fig, 'DICS power map, approach %d' % approach)


same, hopefully can remove the use_3d_backend

larsoner · 2019-07-31T15:04:44Z

examples/inverse/plot_mne_point_spread_function.py

+    time_label = "MEG %d"
+    brain_meg = stc_psf_meg.plot(hemi='rh', subjects_dir=subjects_dir,
+                                 time_label=time_label,
+                                 figure=create_3d_figure(size=(500, 500)))


I don't understand why you need to use use_3d_backend in these examples. stc.plot should use PySurfer which will use mayavi anyway

It's for the create_3d_figure() function which relies on the _Renderer now. Before the port, it was mlab.figure().

You don't need create_3d_figure, use stc.plot(..., size=(500, 500)) instead

Oh wait, that could lead to a problem with it plotting into the same Mayavi figure because it's the same subject name.

In that case, perhaps do figure=1 in the first plot and figure=2 in the second one, and it should spawn separate plots for you.

(one of these two solutions should hopefully work and allow you to avoid the MNE context manager business)

GuillaumeFavelier · 2019-07-31T16:23:01Z

All CIs are happy with this version. The results are available here.

larsoner · 2019-07-31T16:41:55Z

mne/viz/_3d.py

        hemis = [hemi]

-    if title is None:
+    if title is None and isinstance(time_label, str):


Actually instead of using time_label you could just pass title. Sorry I forgot it was an argument at all. Then you don't need to add this conditional.

So for the code in the example you can do title = 'MNE...'; stc.plot(time_label=title, title=title, ...) and it will show up in the title bar and in the image itself. Is that what you want?

Yes, that's perfect!

larsoner · 2019-07-31T17:13:05Z

examples/inverse/plot_mne_crosstalk_function.py

+    time_label = "MNE %d"
+    brain_mne = stc_ctf_mne.plot(hemi='rh', subjects_dir=subjects_dir,
+                                 time_label=time_label,
+                                 figure=create_3d_figure(size=(500, 500)))


You can't use figure=1 here and figure=2 below?

I just missed this example, thank you

larsoner · 2019-07-31T17:20:46Z

@GuillaumeFavelier can you look through the rendered examples and set this to MRG if it's good to go? +1 for merge from me if so

agramfort · 2019-08-01T12:39:51Z

environment.yml

  - mne
  - vtk
-  - pyvista>=0.20.3
+  - https://github.com/GuillaumeFavelier/pyvista/zipball/memory_footprint


I would wait for pyvista to merge the PRs so we use master here. Maybe some API we'll change still. wdyt @GuillaumeFavelier ?

Yes, the discussion about memory optimization is still open. I would recommend waiting until it's merged then I can probably do one last pass to check that everything works as expected.

… Pysurfer)

massich · 2019-08-02T10:29:05Z

I'vee check the examples and apart the fact that the montage look like a "salami brain" everything looks fine. Maybe the lighting of this https://14519-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/source-modeling/plot_dipole_orientations.html#sphx-glr-auto-tutorials-source-modeling-plot-dipole-orientations-py could be made lighter but I'm +1 to merge like this.

maybe just add the lighting of the example above to the todo list next to reducing the diameter of the location markers.

agramfort · 2019-08-02T10:33:56Z

let's fix the montage plots before merging then.

…

massich · 2019-08-02T11:53:18Z

@GuillaumeFavelier the travis error is not related. only network.

GuillaumeFavelier · 2019-08-02T12:52:54Z

It should be fixed now (I like salami btw):

example	Mayavi	PyVista
`plot_eeg_on_scalp.py`
`plot_montage.py`

massich · 2019-08-02T13:19:19Z

NICE!! great job @GuillaumeFavelier, we are almost free of mayavi

massich · 2019-08-02T17:41:18Z

Great!!

…

On Fri, Aug 2, 2019, 16:59 Alexandre Gramfort ***@***.***> wrote: Merged #6551 <#6551> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6551?email_source=notifications&email_token=ABVX5YZ3BUVL6MT7DOZXTGDQCRDWBA5CNFSM4H7PKB7KYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOS3F5YSA#event-2529942600>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABVX5Y3QF33OWGQ5NE5VFULQCRDWBANCNFSM4H7PKB7A> .

* Switch to BackgroundPlotter * Add _set_3d_view() * Add _set_3d_title() * Fix unused variables * Fix offscreen support * Update pyvista package * Catch DeprecationWarning coming from ipywidget * Fix import ordering * TST: try with test_plot_alignment as slowtest * Enable snapshot_brain_montage * TST: Try to build the doc with pyvista [circle full] * Revert "TST: Try to build the doc with pyvista" This reverts commit e4f2b74. * Refactor _Figure management * Fix _set_3d_view and _set_3d_title * Fix set_camera * TST: Try to build doc [circle full] * TST: Fix the test [circle full] * TST: Fix the test again [circle full] * TST: Try to build doc [circle full] * TST: Try build doc again [circle full] * TST: build doc with pyvista [circle full] * Revert doc testing * Improve mayavi/pyvista backend switching * Update backend switching in examples/tutorials * Catch some deprecation warnings * Catch some more warnings * TST: Build doc with pyvista [circle full] * Revert "TST: Build doc with pyvista" This reverts commit cbeef23. * Update backend switching for plot_dics.py * Fix duplicate figure bug and drastically improve camera settings * TST: Build doc with pyvista [circle full] * Revert "TST: Build doc with pyvista" This reverts commit cac3487. * TST: Upgrade PyVista to master version * TST: Upgrade PyVista to version 0.21.2 * TST: Use conda version 4.5.10 * TST: Use conda version 4.7.5 * TST: Build doc with pyvista [circle full] * TST: Build doc normally [circle full] This reverts commit 66c6f3d. * revert last commit [circle full] * Revert "Disable building of doc for now" This reverts commit be7cd48. * Go back to the doc building configuration with pyvista This reverts commit 20570f8. * TST: Ignore doc/tutorials/report.rst in doctest [circle front] * TST: Render all the figures [circle full] * Revert "TST: Ignore doc/tutorials/report.rst in doctest" This reverts commit d968917. * TST: Build doc with pyvista [circle full] * Fix plot_ecog.py [circle full] * Clean doc scraper conf * Avoid use_3d_backend in examples/tutorials * Fix minor bugs * Fix figure title * Avoid use_3d_backend in plot_mne_crosstalk_function.py * TST: Update PyVista to 0.21.3 [circle full] * Switch back to mayavi to build the doc by default (compatibility with Pysurfer) * Fix tube radius

GuillaumeFavelier added 3 commits July 10, 2019 11:44

Switch to BackgroundPlotter

6acd819

Add _set_3d_view()

ae2bb20

Add _set_3d_title()

e8d2d8e

GuillaumeFavelier added the VIZ label Jul 10, 2019

GuillaumeFavelier self-assigned this Jul 10, 2019

GuillaumeFavelier added 2 commits July 10, 2019 16:28

Fix unused variables

b6f04d0

Fix offscreen support

aee1230

GuillaumeFavelier added 2 commits July 11, 2019 13:53

Update pyvista package

984d558

Merge branch 'master' into pyvista_background_plotter

39b1546

GuillaumeFavelier added 6 commits July 11, 2019 15:42

Catch DeprecationWarning coming from ipywidget

3b6c60e

Fix import ordering

1effa00

TST: try with test_plot_alignment as slowtest

97011e6

Enable snapshot_brain_montage

a9779f4

TST: Try to build the doc with pyvista [circle full]

e4f2b74

Revert "TST: Try to build the doc with pyvista"

abd8cdd

This reverts commit e4f2b74.

GuillaumeFavelier mentioned this pull request Jul 12, 2019

VIZ, BUG: test_plot_alignment timeout #6566

Closed

GuillaumeFavelier added 7 commits July 14, 2019 16:59

Refactor _Figure management

6143d25

Merge branch 'master' into pyvista_background_plotter

b9bf33d

Fix _set_3d_view and _set_3d_title

81f7c94

Fix set_camera

5c7e4c8

TST: Try to build doc [circle full]

1adf328

TST: Fix the test [circle full]

4c7d323

TST: Fix the test again [circle full]

1a65195

GuillaumeFavelier mentioned this pull request Jul 16, 2019

MRG, DOC: bad channels / bad spans tutorials #6575

Merged

Fix plot_ecog.py [circle full]

38390ff

larsoner reviewed Jul 31, 2019

View reviewed changes

Clean doc scraper conf

40f6319

Avoid use_3d_backend in examples/tutorials

804aba3

larsoner reviewed Jul 31, 2019

View reviewed changes

GuillaumeFavelier added 2 commits July 31, 2019 18:54

Fix minor bugs

05728a6

Fix figure title

cb44ce4

larsoner reviewed Jul 31, 2019

View reviewed changes

Avoid use_3d_backend in plot_mne_crosstalk_function.py

3df8d6d

larsoner approved these changes Jul 31, 2019

View reviewed changes

agramfort reviewed Aug 1, 2019

View reviewed changes

GuillaumeFavelier added 2 commits August 2, 2019 08:36

TST: Update PyVista to 0.21.3 [circle full]

73efd69

Switch back to mayavi to build the doc by default (compatibility with…

76516e6

… Pysurfer)

Fix tube radius

6babdf3

agramfort approved these changes Aug 2, 2019

View reviewed changes

GuillaumeFavelier changed the title ~~WIP: Pyvista background plotter~~ MRG: Pyvista background plotter Aug 2, 2019

agramfort merged commit f64d613 into mne-tools:master Aug 2, 2019

GuillaumeFavelier deleted the pyvista_background_plotter branch August 2, 2019 15:06

GuillaumeFavelier mentioned this pull request Oct 30, 2019

[MRG] Update 3d backend overview table #6998

Merged

larsoner mentioned this pull request Jun 17, 2022

[MAINT] Remove redundancy in Abtract Qt Classes #10779

Closed

Uh oh!

MRG: Pyvista background plotter #6551

MRG: Pyvista background plotter #6551

Uh oh!

Conversation

GuillaumeFavelier commented Jul 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

State of testing:

Build examples/tutorials with pyvista:

To do next:

Known issues:

Uh oh!

lgtm-com bot commented Jul 10, 2019

Uh oh!

codecov bot commented Jul 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GuillaumeFavelier commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Basic Visualization

Activation Data Visualization

Display ROI Values

Uh oh!

GuillaumeFavelier commented Jul 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Jul 12, 2019

Uh oh!

GuillaumeFavelier commented Jul 16, 2019

Uh oh!

GuillaumeFavelier commented Jul 31, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeFavelier commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

larsoner commented Jul 31, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

massich commented Aug 2, 2019

Uh oh!

agramfort commented Aug 2, 2019 via email

Uh oh!

massich commented Aug 2, 2019

Uh oh!

GuillaumeFavelier commented Aug 2, 2019

Uh oh!

massich commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

massich commented Aug 2, 2019 via email

Uh oh!

GuillaumeFavelier commented Jul 10, 2019 •

edited

Loading

codecov bot commented Jul 10, 2019 •

edited

Loading

GuillaumeFavelier commented Jul 11, 2019 •

edited

Loading

GuillaumeFavelier commented Jul 12, 2019 •

edited

Loading

GuillaumeFavelier commented Jul 31, 2019 •

edited

Loading

massich commented Aug 2, 2019 •

edited

Loading