MAINT: Make PyOpenGL optional

[larsoner]

The conda-forge build is failing because PyOpenGL isn't in the required env. Building with PyOpenGL installed will be a pain, and in general we shouldn't absolutely require that users have this anyway. This is true both because OpenGL can be tricky to get working, and because the performance benefits are unclear.

[larsoner]

@marsipu if this is okay with you, feel free to merge and push out 0.1.5 or I can do it

[marsipu]

@larsoner Yes I think too that OpenGL can be optional to avoid having all these additional problems.
Does pyopengl stay in requirements.txt?
Does the user now need to install pyopengl manually?

Maybe we could see if we collect the data from the speed-test from multiple users with different systems (CPU&GPU-combinations) in the future to see when OpenGL is an advantage.

[larsoner]

Does pyopengl stay in requirements.txt?

I'll remove it

Does the user now need to install pyopengl manually?

Now they can do pip install mne-qt-browser[opengl] but it's easy enough just to tell them to install PyOpenGL however they normally install stuff (conda or pip)

[marsipu]

Maybe you could also change the lines 2162-2169 in _pg_figure.py to something like:

if self.mne.use_opengl:
    try:
        import OpenGL
        logger.info(f'Using pyopengl with version {OpenGL.__version__}')
    except ImportError:
        logger.warning('pyopengl was not found and OpenGL can\'t be used!\n'
                                'Install pyopengl with "pip install pyopengl".')
        self.mne.use_opengl = False

[hoechenberger]

Building with PyOpenGL installed will be a pain

Why is that? PyOpenGL is available on conda-forge, no? So we could just include it as a dependency for the conda-forge package?

[marsipu]

So we could just include it as a dependency for the conda-forge package?

If thats an option I would +1 that.

Otherwise we also probably need to change the default for use_opengl in plot_raw of mne to False.

If you both got time you could do a quick run of test_speed.py (in mne_qt_browser/tests). Then we would habe a bit more data to decide on wether OpenGL should be default/installed&ready or not

[larsoner]

Building with PyOpenGL installed will be a pain

Basically all the reasons it's a pain to deal with OpenGL on CIs. They have to have xvfb on Linux, or you have to use shims on Windows (gl-ci-helpers). So now instead of just having a single meta.yml, you have os-specific build instructions. Then you have to maintain those.

It's not impossible to do this stuff, but regardless I don't think we should require OpenGL unless absolutely necessary. It's a pain for maintenance, and causes problems for users. I would feel this way even if OpenGL was way more performant, but given that in reality sometimes it's actually slower I don't see the point in putting in all this effort to deal with OpenGL when we can get away with making it an optional extension.

[larsoner]

Otherwise we also probably need to change the default for use_opengl in plot_raw of mne to False.

I'm also okay with just using it if it's present, and not using it if it's not. Or maybe better, add a MNE_BROWSE_USE_OPENGL=false config var that people can set to true. If it's False, don't try. If it's True, try, and warn if PyOpenGL isn't installed

[larsoner]

If you both got time you could do a quick run of test_speed.py (in mne_qt_browser/tests).

I tried on my macOS laptop and:

1. The raw traces were not actually displaying when it was running the OpenGL version of the test
2. It never actually returned from pytest mne_qt_browser
3. Subjectively I could tell the OpenGL case was way slower

When I pip uninstall pyopengl and run the test, I get

5.69s call     mne_qt_browser/tests/test_speed.py::test_scroll_speed[benchmark_param0]

and a bunch of errors related to not having PyOpenGL (should probably decorate the tests properly so it doesn't bother trying to run if import opengl doesn't work).

[hoechenberger]

@larsoner I think you're somehow mistaken, we can trivially add PyOpenGL as a dep without setting up xvfb etc. just add it as a dep, but avoid relying on it during tests. I don't think this would add any pain for conda-forge users.

[larsoner]

It's indeed possible #15 plus the conda forge default of trying to do mne-qt-browser --help was the problem with the conda forge build failing early on due to opengl (which it was BTW). But even if that's the case or I'm wrong about the challenges of conda forge setup, I think OpenGL being optional and opt-in is better than required or opt-out here unless the performance benefits are compelling enough to outweigh the support and maintenance costs associated with required/opt-out.

[larsoner]

In addition to things not working properly macOS with OpenGL (and the OpenGL parts that do manage to work being subjectively slower than the CPU-based variants), on my Linux machine which has a good graphics card I get:

use_opengl=False:
    Horizontal:  59.99
    Vertical:    47.14
use_opengl=True:
    Horizontal:  29.97
    Vertical:    30.03
precompute=False:
    Horizontal:  29.93
    Vertical:    29.94
precompute=True:
    Horizontal:  29.91
    Vertical:    30.01
defaults:
    Horizontal:  30.01
    Vertical:    29.98

So CPU is faster than GPU, at least for me. This to me is very far from meeting the bar for maintenance / support cost versus performance benefit.

Okay to make OpenGL opt-in rather than opt-out? In other words, I think we should make it so that the default is not to use OpenGL even if it's installed, and have users set MNE_BROWSE_USE_OPENGL=true on their system / in their config if they want to use it, then we try to do so and only warn if we can't (and still fall back to CPU in that case).

[hoechenberger]

Yes all good for me! Thanks for running these tests, @larsoner!