Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAINT: Tests taking too long #8242

Closed
larsoner opened this issue Sep 11, 2020 · 7 comments · Fixed by #8246
Closed

MAINT: Tests taking too long #8242

larsoner opened this issue Sep 11, 2020 · 7 comments · Fixed by #8246
Milestone

Comments

@larsoner
Copy link
Member

larsoner commented Sep 11, 2020

From #8240 the conda job has:

=========================== slowest 20 test modules ============================
603.30s total  mne/viz/tests
548.90s total  mne/tests
234.70s total  mne/viz/_brain/tests
173.04s total  mne/preprocessing/tests
151.16s total  mne/beamformer/tests
113.52s total  mne/minimum_norm/tests

EDIT: And the pip job:

560.57s total  mne/viz/tests
513.19s total  mne/tests
210.83s total  mne/viz/_brain/tests
176.37s total  mne/preprocessing/tests
149.31s total  mne/beamformer/tests
123.94s total  mne/minimum_norm/tests

Looks like mne/viz and mne/viz/_brain are particularly painful.

At least for _Brain it would be good to figure out where the overhead comes from -- this is going to contribute at least somewhat to the mne/viz speed as well. @GuillaumeFavelier can you look into it? I know we've looked before but it's worth revisiting again probably.

For example I wonder if one little trick we could use is have a fixture that makes all objects in the scene SetVisible(False) -- if we don't actually look at the output, there's no real reason to burn software renderer cycles on it. For example we could have it in every test that uses pyvista except for one that does each type of rendering (vol, mixed, surf) so that it's still preserved one place. WDYT?

@larsoner larsoner added this to the 0.21 milestone Sep 11, 2020
@GuillaumeFavelier
Copy link
Contributor

I'll take a look as soon as I can @larsoner

@GuillaumeFavelier
Copy link
Contributor

It's an item of #7162

@GuillaumeFavelier
Copy link
Contributor

I wonder if one little trick we could use is have a fixture that makes all objects in the scene SetVisible(False) -- if we don't actually look at the output, there's no real reason to burn software renderer cycles on it.

In most of our tests, the plotters are actually closed just after rendering the scene so I'm not sure it's enough. I'll try it of course.

I was thinking about turning _add_mesh or add_actor into a no-op function.

@larsoner
Copy link
Member Author

I was thinking about turning _add_mesh or add_actor into a no-op function.

This would actually kill the testing of MNE->pyvista->VTK, so it would be better to avoid this if possible. SetVisible, on the other hand, just kills VTK->GPU (in theory at least; we trust VTK to work at this level I think)

@GuillaumeFavelier
Copy link
Contributor

But we set this actor property after everything is already mapped right? I think it's already too late for the GPU

@GuillaumeFavelier
Copy link
Contributor

And by "no-op" I mean returning an actor with a mapper associated to no dataset.

@larsoner
Copy link
Member Author

I think it's already too late for the GPU

I'd be surprised if a complex scene involving volume rendering for example doing SetVisible(False) had no impact on rendering speed. All computations involving the ray projection and stuff should be skipped completely by an efficient implementation. Maybe it's assuming too much that VTK has done these optimizations.

At the level of the GPU, even things like vertex shaders should have things in vertex and fragment shaders that just quit if they don't need to compute. At the level of the CPU, hopefully the volume rendering ray casting will only be polled by visible actors. In other words, hopefully the SetInputConnection stuff works smartly enough...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants