MRG, ENH: Fix memory on CircleCI #8379

larsoner · 2020-10-16T13:05:02Z

larsoner · 2020-10-16T13:41:32Z

CircleCI now gives memory outputs which for the mixed source space example is:

I'll try to get this down somehow

larsoner · 2020-10-16T18:44:00Z

Looks like the culprit was our volume source space rr/nn/interpolator redundancy (and interpolator even when add_interpolator=False!). I wrote some code that makes it not copy these and not create them (the "empty" ones took 70 MB apiece, which for 9 regions leads to > 600 MB used to store "nothing"), looks much better now:

Ready for review/merge from my end.

larsoner · 2020-10-16T23:57:24Z

Okay we seem to have some ever-increasing memory problem:

Memory should of course increase as examples run and Sphinx / Sphinx-gallery accumulate objects, by not by so much. This makes me think that plot_source_space_morphing.py is just a symptom and not the underlying problem.

larsoner · 2020-10-20T13:41:22Z

I don't expect this [circle full] to succeed, but we should still merge this. It should help us see the memory increases / failures in master better. I also added a fix for latest matplotlib.

@GuillaumeFavelier can you take a look and approve if you're happy? Then I'll merge once the latest-matplotlib fix is merged upstream (nilearn).

GuillaumeFavelier · 2020-10-20T13:41:59Z

I'll review right now

GuillaumeFavelier

+1 for tools/circleci_download.sh

Just one question but except from that LGTM

doc/Makefile

larsoner · 2020-10-20T16:49:44Z

Okay looks like #8352 was probably the culprit as it added a reference cycle due to bound methods. Locally before 8abd6ff getting partway through a make html_dev-memory I got:

Now after 20 minutes of running I get a baseline around ~2GB instead of the ~5GB above:

I've added a brain_gc fixture that yells at us when we don't end up with properly GC'ed brains -- for example if brain.close isn't called we see a list of the type and repr(...)[:100] of the objects that are keeping the n brain instances alive:

_________________________________________________________ ERROR at teardown of test_plot_sparse_source_estimates[pyvista] __________________________________________________________
mne/conftest.py:490: in brain_gc
    assert n == 0, f'{n} after:\n{new.join(ref)}'
E   AssertionError: 1 after:
E     list: [[], {'__repr__': <slot wrapper '__repr__' of 'object' objects>, '__hash__': <slot wrapper '__hash__
E     frame: <frame at 0x7f5db684aa40, file '/home/larsoner/python/mne-python/mne/conftest.py', line 489, code br
E     method: <bound method Brain._on_mouse_move of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     method: <bound method Brain._on_button_press of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     method: <bound method Brain._on_button_release of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     method: <bound method Brain._on_pick of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     dict: {'fig': <Figure size 800x267 with 1 Axes>, 'canvas': <matplotlib.backends.backend_qt5agg.FigureCanva
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     dict: {'plotter': <pyvistaqt.plotting.BackgroundPlotter object at 0x7f5dadba1d30>, 'brain': <mne.viz._brai
E     method: <bound method Brain.set_data_smoothing of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     method: <bound method Brain.plot_time_line of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     method: <bound method Brain.set_playback_speed of <mne.viz._brain._brain.Brain object at 0x7f5da999af70>>
E     cell: <cell at 0x7f5da0dd0fa0: Brain object at 0x7f5da999af70>
E     cell: <cell at 0x7f5da10e10a0: Brain object at 0x7f5da999af70>
E     cell: <cell at 0x7f5da10e1220: Brain object at 0x7f5da999af70>
E   assert 1 == 0

@GuillaumeFavelier can you take another look? It looks like what really needed to happen was cleaning up the bound methods of Brain that were attached to the iren.

If the circle full passes, I'm going to add this GC reference check for Brain so that hopefully these get flagged in the future. I tried it already to see if we could fix "plot_montage.py", but it turns out that there are no references to pyvista.Plotter at all at the end of the example, indicating that it really is a problem at the VTK end with some objects not being cleaned up properly somehow.

GuillaumeFavelier · 2020-10-20T17:02:03Z

Very good catch on 8abd6ff @larsoner !

larsoner · 2020-10-20T18:18:22Z

Getting closer:

Need to figure out why there is a ~600 MB loss around 800 sec next

GuillaumeFavelier · 2020-10-22T11:13:53Z

How can I make sure that close() is called even in this situation?

Nevermind, all of this can be closed afterwards with renderer.backend._close_all().

larsoner · 2020-10-22T11:24:52Z

13ce561 passed with [circle full]!

Clearly there are some examples that could be improved, but at least it got us back to green.

I pushed an empty commit with another [circle full] to see if it at least builds again, maybe even with lower memory usage after your commits @GuillaumeFavelier. Okay to merge from your end @GuillaumeFavelier ? If so I'll look at your latest commits and merge once everything comes back green.

GuillaumeFavelier · 2020-10-22T11:27:05Z

Travis is far from happy, I am working on plot_source_estimates to improve the situation and there is the failure with notebook azure job. I would not say it's ready for merge yet.

GuillaumeFavelier · 2020-10-22T11:28:20Z

Worst case scenario, we can remove brain_gc from test_plot_source_estimates and test_plot_sparse_source_estimates and do that later?

larsoner · 2020-10-22T11:29:45Z

Worst case scenario, we can remove brain_gc from test_plot_source_estimates and test_plot_sparse_source_estimates and do that later?

Let's do that for now with a TODO or XXX comment. Feel free to push a commit with [circle full]. It would be good to get this in so that master CircleCI builds work again

GuillaumeFavelier · 2020-10-22T11:37:44Z

That's it for the TODO, should I push now or wait that Circle is done?

larsoner · 2020-10-22T11:52:40Z

Feel free to push now, don't bother with [circle full] because I have something to push as well

larsoner · 2020-10-22T11:59:45Z

mne/minimum_norm/spatial_resolution.py

-    resmat = np.absolute(resmat)  # only use absolute values
+    # we want to use absolute values, but doing abs() mases a copy and this
+    # can be quite expensive in memory. So let's just use abs() in place below.


FYI @olafhauk these matrices can be quite large (~8000x8000), so instead of doing this operation here, I've moved it to be done column-wise. It shouldn't be an appreciable performance difference but it should lower memory usage by ~800 MB in the plot_resolution_metrics.py example.

larsoner · 2020-10-22T12:01:28Z

@GuillaumeFavelier I worry that the FakeIren stuff is fragile, locally I get this:

Traceback (most recent call last):
  File "/home/larsoner/.local/lib/python3.8/site-packages/vtkmodules/qt/QVTKRenderWindowInteractor.py", line 463, in enterEvent
    self._Iren.EnterEvent()
AttributeError: 'FakeIren' object has no attribute 'EnterEvent'

I think we should prioritize fixing PyVista, getting a PR merged there, and then hopefully soon-ish get rid of our workarounds here.

GuillaumeFavelier · 2020-10-22T12:50:00Z

I worry that the FakeIren stuff is fragile

This is homemade hack basically 😓

we should prioritize fixing PyVista

Agreed 👍

GuillaumeFavelier · 2020-10-22T12:58:08Z

I can reproduce the failures of test_interactor_style locally:

E       AssertionError: Not all objects GCed:
E         vtkLightKit
E       assert 1 == 0
E        +  where 1 = len([(vtkmodules.vtkRenderingCore.vtkLightKit)0x7fa68995b6a0])

larsoner · 2020-10-22T12:59:20Z

This sounds like a comment for pyvista/pyvista#958 :)

GuillaumeFavelier · 2020-10-22T12:59:33Z

Whoops

GuillaumeFavelier · 2020-10-22T14:58:20Z

It seems that the brain has been off-road in plot_3d_to_2d.py 😅

Reference: https://22865-1301584-gh.circle-artifacts.com/0/dev/auto_examples/visualization/plot_3d_to_2d.html#sphx-glr-auto-examples-visualization-plot-3d-to-2d-py

larsoner · 2020-10-22T14:59:27Z

Yes probably in the change from focalpoint=None to focalpoint=(0., 0., 0.) default that happened at some point. Let's fix that in another PR and get this in so master is green!

larsoner · 2020-10-22T15:00:58Z

@GuillaumeFavelier maybe we should add focalpoint='auto' which means "use the center of the bounding box"? There are some other examples where this could be used instead of custom focalpoint=(...) calls (git grep focalpoint should expose them)

GuillaumeFavelier · 2020-10-22T15:02:22Z

I think that's the default when focalpoint=None

GuillaumeFavelier · 2020-10-22T15:03:10Z

mne-python/mne/viz/backends/_pyvista.py

Lines 771 to 774 in 278608f

    
           if focalpoint is not None: 
        
               focalpoint = np.asarray(focalpoint) 
        
           else: 
        
               focalpoint = (bounds[1::2] + bounds[::2]) * 0.5

larsoner · 2020-10-22T15:05:08Z

Okay then let's just update the broken example and the ones that manually set focalpoint=(...) to use focalpoint=None instead (assuming it works as well for those)

* upstream/master: Fix separate canvas (mne-tools#8408) FIX: focalpoint (mne-tools#8405) WIP: Refs (mne-tools#8406) tiny cosmetic improvements to BEM code (mne-tools#8404) MRG, ENH: Fix memory on CircleCI (mne-tools#8379) MRG: Update backend parameter in stc.plot() (mne-tools#8395)

larsoner force-pushed the mem branch from e5c06f5 to 1af8cc1 Compare October 16, 2020 13:22

larsoner changed the title ~~WIP: Fix memory on CircleCI~~ MRG, ENH: Fix memory on CircleCI Oct 16, 2020

larsoner force-pushed the mem branch 2 times, most recently from e4134e3 to e64e200 Compare October 16, 2020 19:55

larsoner mentioned this pull request Oct 16, 2020

Reduce memory usage of GLM analysis mne-tools/mne-nirs#191

Open

larsoner force-pushed the mem branch from e64e200 to b9e6c0f Compare October 20, 2020 13:40

GuillaumeFavelier approved these changes Oct 20, 2020

View reviewed changes

doc/Makefile Show resolved Hide resolved

larsoner force-pushed the mem branch 2 times, most recently from d8a80c5 to 37da9e1 Compare October 20, 2020 15:15

larsoner added 10 commits October 20, 2020 12:44

ENH: Add memory to circle [skip travis] [skip github]

5a9c418

ENH: Better memory

ca85fdf

FIX: No second param [circle full]

e7c6b9f

FIX: Download [circle full]

3230043

BUG: Fix for latest mpl [circle full]

81836fb

FIX: Order [circle full]

5738bb4

FIX: is_first_col dep

2624270

FIX: Old mpl

6dcc394

FIX: Fix problematic bound methods [circle full]

2c2302f

ENH: Tighter doc build [circle full]

3d5373d

FIX: Mem usage [circle full]

7251a62

larsoner force-pushed the mem branch from 8abd6ff to 7251a62 Compare October 20, 2020 19:10

larsoner mentioned this pull request Oct 20, 2020

BUG: VTK/PyVista not freeing memory with multiple figures #8241

Closed

TST: Circle [circle full]

2ce07ac

Remove some brain_gcs [circle full]

73be960

GuillaumeFavelier mentioned this pull request Oct 22, 2020

Use brain_gc in remaining tests #8403

Closed

ENH: Lower memory [circle full]

3ab92c9

larsoner commented Oct 22, 2020

View reviewed changes

BUG: Fixes [circle full]

278608f

GuillaumeFavelier mentioned this pull request Oct 22, 2020

TST: Test for GC pyvista/pyvista#958

Merged

larsoner merged commit 0629871 into mne-tools:master Oct 22, 2020

larsoner deleted the mem branch October 22, 2020 14:59

GuillaumeFavelier mentioned this pull request Oct 22, 2020

FIX: focalpoint #8405

Merged

GuillaumeFavelier mentioned this pull request Nov 16, 2020

ENH: New interface for Brain #7162

Closed

86 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRG, ENH: Fix memory on CircleCI #8379

MRG, ENH: Fix memory on CircleCI #8379

larsoner commented Oct 16, 2020 •

edited

Loading

larsoner commented Oct 16, 2020

larsoner commented Oct 16, 2020

larsoner commented Oct 16, 2020

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 20, 2020

GuillaumeFavelier left a comment

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 20, 2020

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020 •

edited

Loading

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

larsoner Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

MRG, ENH: Fix memory on CircleCI #8379

MRG, ENH: Fix memory on CircleCI #8379

Conversation

larsoner commented Oct 16, 2020 • edited Loading

larsoner commented Oct 16, 2020

larsoner commented Oct 16, 2020

larsoner commented Oct 16, 2020

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 20, 2020

GuillaumeFavelier left a comment

Choose a reason for hiding this comment

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 20, 2020

larsoner commented Oct 20, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020 • edited Loading

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

larsoner Oct 22, 2020

Choose a reason for hiding this comment

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

larsoner commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

GuillaumeFavelier commented Oct 22, 2020

larsoner commented Oct 22, 2020

larsoner commented Oct 16, 2020 •

edited

Loading

larsoner commented Oct 22, 2020 •

edited

Loading