[ENH]: New pyqtgraph-backend for 2D-Data-Browser

[marsipu]

About this PR

This PR adds the new pyqtgraph-backend I developed during 2021's Google Summe of Code.
It covers basic functionality and is further developed in mne-tools/mne-qt-browser.
This PR is WIP and remaining ToDo's as well as ideas for improvements and new features can be submitted as issues in mne-tools/mne-qt-browser.
I would be glad about feedback for the new backend. I provided example code for testing below.

Example code

import mne
import os
import numpy as np

from mne.viz import use_browser_backend

# Load Raw
sample_data_folder = mne.datasets.sample.data_path()
raw_file = os.path.join(sample_data_folder, 'MEG', 'sample',
                        'sample_audvis_raw.fif')
raw = mne.io.read_raw(raw_file)

# Load Events
events_path = os.path.join(sample_data_folder, 'MEG', 'sample',
                           'sample_audvis_raw-eve.fif')
events = mne.read_events(events_path)

# Add test-annotations
onsets = np.arange(2, 8, 2) + raw.first_time
durations = np.repeat(1, len(onsets))
descriptions = ['Test1', 'Test2', 'Test3']
for onset, duration, description in zip(onsets, durations, descriptions):
    raw.annotations.append(onset, duration, description)

with use_browser_backend('pyqtgraph'):
    raw.plot(events=events, block=True)

Benchmark pyqtgraph-specific parameters

If you would like to compare performance of pyqtgraph-specific parameters (use of OpenGL, antialiasing, downsampling), you can fork my gsoc2021-repository and run python -m prototypes from the main directory.

Showcase

MNE-Browser-Promo.mp4

[agramfort]

to test should I just pip install git+https://github.com/pyqtgraph/pyqtgraph@master?

@cbrnr @marsipu what do you think if putting this feature in a different repo for now?
my recent experience with other mne side projects is that it really speeds up development
as you don't have to deal with mne-python full test suite etc.

[cbrnr]

@cbrnr @marsipu what do you think if putting this feature in a different repo for now?
my recent experience with other mne side projects is that it really speeds up development
as you don't have to deal with mne-python full test suite etc.

@larsoner @drammock you were more involved in the backend changes, what do you think?

[marsipu]

to test should I just pip install git+https://github.com/pyqtgraph/pyqtgraph@master?

Yes, and I would also recommend pyopengl because it improves performance a lot.

@cbrnr @marsipu what do you think if putting this feature in a different repo for now?
my recent experience with other mne side projects is that it really speeds up development
as you don't have to deal with mne-python full test suite etc.

The goal for this PR would be to make the complete viz-test-suite pass for pyqtgtaph.
I couldn't finish that during GSoC, but after my 2 week holiday now I will try to find time to focus on that first.
If you still want this process outside I could continue in my gsoc2021-repository too.
What do you prefer?

[agramfort]

I think there is no rush here.

[drammock]

@cbrnr @marsipu what do you think if putting this feature in a different repo for now?
my recent experience with other mne side projects is that it really speeds up development

@larsoner @drammock you were more involved in the backend changes, what do you think?

We discussed this at our last meeting, and @larsoner and I agreed that:

1. we want lots of folks to test out with their own data (and their own particular usage habits)
2. testing is easier if all people need to do is pip install 2 packages, check out this fork+branch, and copy-paste the example script above

I'm not too worried about speeding up development... @marsipu has a really nice prototyping framework at his other repo that runs automated performance benchmarks, and has been good about testing locally and only pushing large batches of commits so that our CIs don't get overwhelmed / backlogged.

[marsipu]

Ok, when I get on with the test and the other ToDos for this PR I will commit them to the gsoc2021-repository.
Or would [ci skip] also suffice to save CI-resources until all tests are adapted? (That may bring the advantage of tracking back the single commits in this PR)

[marsipu]

How would you suggest gathering feedback and potential bugs? Can you edit my list in #9686? Or can I make it editable?

[larsoner]

Or would [ci skip] also suffice to save CI-resources until all tests are adapted?

I would just push commits here with [ci skip] until you expect them to be green

How would you suggest gathering feedback and potential bugs? Can you edit my list in #9686? Or can I make it editable?

Yes maintainers can edit issues. But in general it's better if you keep track most of the time @marsipu

[larsoner]

@marsipu why is pyqtgraph master required? Is it just for turning off antialiasing or something? It would be nice if it's not required and we only use master functionality if it's present (e.g., by using mne.fixes._get_args or mne.utils.check_version to decide).

Do you know the minimal version of pyqtgraph that's needed? At the very least we'll need to add that to README.md in the optional dependencies section.

[larsoner]

... If it's just for OpenGL to work, given that it doesn't work on all platforms I'd leave OpenGL disabled by default, and let people enable it who have usable OpenGL graphics on their machines and want to install PyQtGraph master. It would be best if the code snippet worked for people without OpenGL and without needing latest master.

[drammock]

... If it's just for OpenGL to work, given that it doesn't work on all platforms I'd leave OpenGL disabled by default

I think the reason to use master is that they recently fixed it so that OpenGL now works on Windows, at least I think that's what @marsipu mentioned at our last meeting. So it ought to work on all platforms now.

[larsoner]

I would disable OpenGL by default and have people opt-in to trying it. Based on the number of failures we see for 3D viz I think it's the safest option.

[marsipu]

@larsoner Yes, it is as @drammock says, there are two OpenGL-related changes in the current dev-version from pyqtgraph:

1. fix bug breaking OpenGL-Plots on Windows (PR 1897)
2. make antialiasing disableable (PR 1932)

I hope there will be another release of pyqtgraph soon covering these changes.

On the machines I tested I had no other problems with OpenGL, but it seems reasonable to disable OpenGL by default since fixing problems with it seems to be complicated.
We might set it as a config-value then since potential problems are probably often hardware-dependent. WDYT?

[cbrnr]

OpenGL is so much faster that I really like to have it turned on by default. We haven't seen any issues so far, so unless there are any known bugs there is no reason to disable it. I think it is also fine to require the master branch for now, because the new browser backend lives only within a PR and hasn't been integrated into MNE yet. I'm pretty confident that the PyQtGraph devs will release a new version soon anyway.

[marsipu]

I agree to both points @larsoner and @cbrnr.
Maybe setting the default on the final integrated backend would depend on the experience you testers have with the version from this PR? Should I maybe activate openGL just for this testing-period by default to see if anyone got problems with it?

[agramfort]

let me try to bring some arguments to start with a dedicated repo: - you could have a test suite that runs in 5 mins - you can more easily test different pyqtgraph version (stable, dev, with opengl etc.) - you can release a lot more often. and I am +100 to make any relevant change to mne-python upstream to keep the browser code simpler like you did with the major refactoring you did on matplotlib backend.

…

[hoechenberger]

Hello, I lost track of the conversation a little, is this the right place to report my experiences with the UI and point out where the UX could be improved? 😅🙈

[marsipu]

Hello, I lost track of the conversation a little, is this the right place to report my experiences with the UI and point out where the UX could be improved? 😅🙈

Yes, or at #9686. I will gather the feedback then in the first comment

[marsipu]

let me try to bring some arguments to start with a dedicated repo:

• you could have a test suite that runs in 5 mins
• you can more easily test different pyqtgraph version (stable, dev, with
  opengl etc.)
• you can release a lot more often.

and I am +100 to make any relevant change to mne-python upstream to keep
the browser
code simpler like you did with the major refactoring you did on matplotlib
backend.

@agramfort I understand your points, especially regarding the release-frequency (though I hope that until the next mne-release in october, the pyqtgraph-backend won't need that many changes anymore).
Would you like to have the browser-code separate in general or just for this transition-period?
Setting up a separate repo would mean configuring the CIs too, right? I don't have much experience with that

[agramfort]

@agramfort <https://github.com/agramfort> I understand your points, especially regarding the release-frequency (though I hope that until the next mne-release in october, the pyqtgraph-backend won't need that many changes anymore).

what software engineers tell you is that: a software stops needing maintenance when people stop using it. version changes, OS changes, Everything changes around and you'll need to adjust. But hopefully it won't be much work but you don't know yet. For example years ago maintaining mayavi in mne was really hard although a company was pushing the software. Matplotlib changes things constantly but i's never a big pain. Now we this new set of dependencies I have no experience and I also want to be cautious. Will you and @CBNR be actively maintaining this code in mne-python? I would say let's learn from our mistakes is smaller projects and once we "know better" we can consolidate. my 2c

[marsipu]

Will you and @cbrnr be actively maintaining this code in mne-python?

I want to be available to maintain the functionality of the browser with pyqtgraph and to adapt to possible changes in the pyqtgraph-API in the future. I hope, that there won't be major API changes in pyqtgraph in future which would require rewriting a lot of the code. But you're right, I don't know that yet. I am also looking forward to see how much work still has to be done after you developers tested the backend and found bugs and/or ideas for improvement.

[agramfort]

I just manage to give it a try after updating the packaging.

it works really fast and it's already pretty usable congrats !

I see a few UI glitches when playing with annotations (selecting what is visible, removing one annotation etc.)
I am also not sure all shortcuts are working. Should it already?

great job @marsipu

[agramfort]

also I see now a number of traceback errors in the terminal. Eg.

---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
~/work/src/mne-python/mne/viz/_pg_figure.py in _remove_description(self)
    948                     self.mne.inst.annotations.description == rm_description)
    949                 for idx in rm_idxs:
--> 950                     self.mne.inst.annotations.delete(idx)
    951                 for rm_region in [r for r in self.mne.regions
    952                                   if r.description == rm_description]:

~/work/src/mne-python/mne/annotations.py in delete(self, idx)
    378         self.duration = np.delete(self.duration, idx)
    379         self.description = np.delete(self.description, idx)
--> 380         self.ch_names = np.delete(self.ch_names, idx)
    381
    382     def to_data_frame(self):

<__array_function__ internals> in delete(*args, **kwargs)

~/miniconda3/lib/python3.8/site-packages/numpy/lib/function_base.py in delete(arr, obj, axis)
   4404         else:
   4405             keep = ones(N, dtype=bool)
-> 4406             keep[obj,] = False
   4407
   4408         slobj[axis] = keep

IndexError: index 15 is out of bounds for axis 0 with size 15

[marsipu]

Thank you @agramfort

I see a few UI glitches when playing with annotations (selecting what is visible, removing one annotation etc.)

I missed some bugs there which I fixed now. The cause for the traceback though came from the sample code I supplied (I forgot to set annotations.ch_names). I updated the sample code now above.

[agramfort]

a few comments but otherwise

it really really well for me !

many other devs from @mne-tools/mne-python-steering-committee can try too?

[wmvanvliet]

Holy cow, this thing flies! A huge improvement over the matplotlib based renderer. 👏

I get an error message when selecting different annotation types and changing their color:

<mne.viz._pg_figure.PyQtGraphBrowser at 0x7f535f9c2940>Traceback (most recent call last):
  File "/m/home/home4/45/vanvlm1/data/projects/mne-python/mne/viz/_pg_figure.py", line 1067, in _set_color
    self.main._update_regions_colors()
AttributeError: 'PyQtGraphBrowser' object has no attribute '_update_regions_colors'

When I then select the color again, it works properly.

The speed of this browser begs fore some UI features that weren't feasible before, such as click+drag in the main window to scroll around, using scrollwheel+modifier key to zoom.

[larsoner]

I snuck this in because we need it for release anyway

[drammock]

Left a few docstring improvement comments. More importantly: keyboard shortcuts are not consistent between the two backends. pageup/pagedown do opposite things in pyqtgraph backend as in matplotlib backend. Make them consistent. I didn't thoroughly check all the keyboard shortcuts so I'm hoping this is an isolated oversight and there aren't other cases; @marsipu please check them all again.

[larsoner]

@marsipu feel free to address these tomorrow. If we need to push the release until then or later in the week it's okay, let's get it right!

[marsipu]

Left a few docstring improvement comments. More importantly: keyboard shortcuts are not consistent between the two backends. pageup/pagedown do opposite things in pyqtgraph backend as in matplotlib backend. Make them consistent. I didn't thoroughly check all the keyboard shortcuts so I'm hoping this is an isolated oversight and there aren't other cases;

@drammock I checked all the keyboard-shortcuts and there shouldn't be any differences left now. PageUp/PageDown were confused.

[drammock]

looks like the CI failures are related / relevant:

=================================== FAILURES ===================================
_____________________ test_min_window_size[pyqtgraph-None] _____________________
mne/viz/tests/test_raw.py:844: in test_min_window_size
    assert_allclose(fig._get_size(), (8, 8), atol=atol)
E   AssertionError: 
E   Not equal to tolerance rtol=1e-07, atol=0
E   
E   Mismatched elements: 2 / 2 (100%)
E   Max absolute difference: 8.33333333
E   Max relative difference: 1.04166667
E    x: array([16.333333, 11.930556])
E    y: array([8, 8])
___________________ test_min_window_size[pyqtgraph-0.1,0.1] ____________________
mne/viz/tests/test_raw.py:844: in test_min_window_size
    assert_allclose(fig._get_size(), (8, 8), atol=atol)
E   AssertionError: 
E   Not equal to tolerance rtol=1e-07, atol=0
E   
E   Mismatched elements: 2 / 2 (100%)
E   Max absolute difference: 8.33333333
E   Max relative difference: 1.04166667
E    x: array([16.333333, 11.930556])
E    y: array([8, 8])
=============================== warnings summary ===============================

[larsoner]

Green! In it goes. Thanks for working through this @marsipu , this is amazing stuff!

[rob-luke]

🎉 🎉 🎉 🎉 🎉

[drammock]

👏👏👏👏👏👏👏👏👏

[hoechenberger]

🥳 ❤️

[sappelhoff]

awesome work! 🚀

[agramfort]

@marsipu what you have achieved is big ! Honestly I had almost no hope you could pull this off ! 👏 👏 👏

[larsoner]

Hopefully just the first of many times you get to prove @agramfort wrong :)

[marsipu]

Thank you all for your support and feedback on the way. I am really happy that the new backend can now be tested/used by the greater community.

Honestly I had almost no hope you could pull this off !

Me too 😄 (I am currently preparing for my biggest exam in medicine next spring)

Hopefully just the first of many times you get to prove @agramfort wrong :)

That's the plan (at least in a positive sense)

[agramfort]

Honestly I had almost no hope you could pull this off ! Me too 😄 (I am currently preparing for my biggest exam in medicine next spring)

a medical doctor who can code for real ! you'll never stop bluffing me :)

Hopefully just the first of many times you get to prove @agramfort <https://github.com/agramfort> wrong :) That's the plan (at least in a positive sense)

I am a very positive person don't hesitate ;)