Rework Reports (new history) #9754

hoechenberger · 2021-09-16T19:57:41Z

Rework of #9722 after #9749 had been merged

larsoner

Great that there is a real diff now! I did not look at everything, but noticed a few minor things

mne/report/__init__.py

mne/report/report.py

mne/report/tests/test_report.py

mne/utils/docs.py

hoechenberger · 2021-09-17T08:11:44Z

Thanks for the initial suggestions, @larsoner! Some good catches. I'll try to address all of them and ping you.

…eport2

… types of channels were not written and bug where annotations were not written to correct scale (mne-tools#9694) * Fixing bug * Update doc. * Update docstring * Add diff * Add diff * Add diff * Add warning * Fix flake * Adding test at data level. * Dont mess with global random state * Adding note to edf export * Adding note to edf export * Adding updated unit tests and io reading for edf * Adding unit test for channel type * Adding change log * Add warning * Add additional parameter * Fix docstring * Fixing other test * Fix flake * Apply suggestions from code review Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> * Update mne/export/tests/test_export.py Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> * Fixing flake * Apply suggestions from code review Co-authored-by: Daniel McCloy <dan@mccloy.info> * Update mne/export/_edf.py Co-authored-by: Daniel McCloy <dan@mccloy.info> * Update mne/export/_edf.py Co-authored-by: Daniel McCloy <dan@mccloy.info> * Update mne/export/_edf.py Co-authored-by: Daniel McCloy <dan@mccloy.info> * Merge * Address comments * Check double export * Fix flake * Remove scruff * Fix codespell * Apply suggestions from code review Co-authored-by: Daniel McCloy <dan@mccloy.info> * update docstring * Commeent on stim * Fix annotations * Adding docstring * Fix number of annotations * Add check for duration * Fix issues * Fix docstring * Apply suggestions from code review Co-authored-by: Clemens Brunner <clemens.brunner@gmail.com> * Clarify warning and inline comment * Fix docstring * Fix doc warning and len(annotations). Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Daniel McCloy <dan@mccloy.info> Co-authored-by: Clemens Brunner <clemens.brunner@gmail.com>

* update info html template * add all channel types to the html repr * update html repr test * Update mne/io/meas_info.py Co-authored-by: Eric Larson <larson.eric.d@gmail.com> * Update mne/io/meas_info.py Co-authored-by: Eric Larson <larson.eric.d@gmail.com> * add repr_html smoke test * ENH: Titles * update changelog * fix name in the change log * FIX: Move down Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

…eport2

agramfort

looks great !

I even just tested the mne report command it produced a beautiful report on my laptop !

🎉 👏

larsoner · 2021-10-05T15:03:25Z

Test time is still a problem (maybe MRI rendering, maybe something else):

74.13s call     mne/report/tests/test_report.py::test_full_report

The only report-related ones that show up in a different PR's recent build are:

23.52s call     mne/report/tests/test_report.py::test_render_report[mayavi]
23.08s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]

hoechenberger · 2021-10-05T15:17:03Z

@larsoner Ouch, good catch. I managed to cut down the test time drastically on my local machine, and I'm surprised tests still take so long on GH Actions. Back to the profiler, I suppose!

larsoner · 2021-10-05T15:29:48Z

3D plotting is much slower there, my guess is it's the new alignment plotting function. One option is to make it so the view/size there is very small so there is less to render

larsoner · 2021-10-05T15:30:21Z

On Linux you can test this using xvfb-run because it will use software rendering like the CIs -- want me to try locally?

larsoner · 2021-10-05T15:38:23Z

Locally there is at least some slowdown from software rendering, so I think it probably is the 3D rendered plot_alignment slowing things down:

$ pytest mne/report
25.12s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
15.49s call     mne/report/tests/test_report.py::test_full_report
...
$ xvfb-run pytest mne/report
23.49s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
21.96s call     mne/report/tests/test_report.py::test_full_report

And with if get_3d_backend() is not None and False: in test_report.py:656 so that the 3D stuff is skipped:

23.22s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
9.85s call     mne/report/tests/test_report.py::test_full_report

So locally I would target the four calls in those to speed them up somehow. Making the render window smaller is one way.

larsoner · 2021-10-05T15:40:54Z

... actually hold on, I think we have a regression with MNE_3D_BACKEND_TESTING where we automagically made it so that these things were faster. Let me see if I can fix it quickly.

hoechenberger · 2021-10-05T15:49:09Z

Oh yes, I think you might be in the right track here!!

larsoner · 2021-10-05T15:52:53Z

Nope, no regression -- really the slowdown here is caused because the renderer fixture is not used. But We can easily get around it by using the same actor-invisible trick. I've pushed a commit that locally fixes things, timings for xvfb-run:

22.97s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
12.97s call     mne/report/tests/test_report.py::test_full_report

I'll push and we'll see how the CIs do. But really these are both still too slow, this is what I see on main using the same xvfb-run call:

9.07s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
2.49s call     mne/report/tests/test_report.py::test_render_mri[pyvistaqt]

larsoner

Tests too slow currently

hoechenberger · 2021-10-05T16:06:58Z

But really these are both still too slow, this is what I see on main using the same xvfb-run call:

But keep in mind that we're actually plotting much more now than we used to, so a bit of slowdown is to be expected. But I agree, the current slowdown is too much.

…eport2

hoechenberger · 2021-10-06T07:24:06Z

@larsoner Test runtime summary for Linux / pip-pre / py3.9:

46.28s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
3492
41.12s call     mne/tests/test_bem.py::test_io_head_bem
3493
35.44s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[stat_map-s-1-1-init_p3-want_p3-brain.mgz]
3494
33.58s call     mne/export/tests/test_export.py::test_export_raw_edf[misc-edf]
3495
31.82s call     mne/viz/tests/test_3d.py::test_plot_alignment_basic[testing_data-testing_data-pyvistaqt]
3496
29.56s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[stat_map-vec-1-1-None-want_p1-None]
3497
27.18s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-sample-0.0-0.1-float-True]
3498
25.61s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[fsaverage-fsaverage-0.0-0.1-float-False]
3499
24.36s call     mne/tests/test_morph.py::test_volume_source_morph_basic
3500
22.71s call     mne/tests/test_docstring_parameters.py::test_docstring_parameters
3501
22.42s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-fsaverage-7.0-7.4-float-True]
3502
22.39s call     mne/tests/test_source_space.py::test_setup_source_space_spacing[2]
3503
22.30s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-sample-0.0-0.1-complex-False]
3504
20.70s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-fsaverage-10.0-10.4-float-False]
3505
20.08s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[glass_brain-s-None-2-None-want_p0-None]
3506
19.97s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[glass_brain-vec-None-1-init_p2-want_p2-None]
3507
18.87s call     mne/inverse_sparse/tests/test_mxne_inverse.py::test_mxne_inverse_standard[_pytest_param]
3508
17.93s call     mne/report/tests/test_report.py::test_full_report
3509

And on Linux / conda / py3.9:

180.92s call     mne/viz/tests/test_3d.py::test_plot_alignment_basic[testing_data-testing_data-mayavi]
3510
148.80s teardown mne/viz/tests/test_3d.py::test_plot_alignment_basic[testing_data-testing_data-mayavi]
3511
72.20s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[stat_map-s-1-1-init_p3-want_p3-brain.mgz]
3512
65.88s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[stat_map-vec-1-1-None-want_p1-None]
3513
56.89s call     mne/report/tests/test_report.py::test_render_report[mayavi]
3514
55.53s call     mne/report/tests/test_report.py::test_render_report[pyvistaqt]
3515
44.53s call     mne/minimum_norm/tests/test_inverse.py::test_inverse_operator_channel_ordering[testing_data-testing_data]
3516
44.42s call     mne/report/tests/test_report.py::test_full_report
3517
39.13s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[glass_brain-s-None-2-None-want_p0-None]
3518
39.01s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates[glass_brain-vec-None-1-init_p2-want_p2-None]
3519
32.33s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-sample-0.0-0.1-float-True]
3520
31.93s call     mne/viz/tests/test_3d.py::test_plot_alignment_basic[testing_data-testing_data-pyvistaqt]
3521
30.38s call     mne/tests/test_docstring_parameters.py::test_docstring_parameters
3522
29.83s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[fsaverage-fsaverage-0.0-0.1-float-False]
3523
28.22s call     mne/tests/test_morph.py::test_volume_source_morph_basic
3524
26.42s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-fsaverage-7.0-7.4-float-True]
3525
26.20s call     mne/tests/test_source_space.py::test_setup_source_space_spacing[2]
3526
25.62s call     mne/viz/tests/test_3d_mpl.py::test_plot_volume_source_estimates_morph
3527
25.59s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-sample-0.0-0.1-complex-False]
3528
24.11s call     mne/tests/test_morph.py::test_volume_source_morph_round_trip[sample-fsaverage-10.0-10.4-float-False]
3529

You think this is good enough?

Otherwise I can try even harder to make the tests shorter and potentially also merge test_full_report and test_render_report, as I believe they partially overlap.

larsoner · 2021-10-06T15:22:31Z

A minute is a really long time for a test -- we have over 3000 tests and a single one-minute test takes roughly 1/50th of our allowed test time. Can I take a look and see if I can speed it up somehow?

hoechenberger · 2021-10-06T18:12:56Z

Yes if you want to give it a shot, I won't say no! But only if you can squeeze it into your schedule! Otherwise I'll just try my best to improve things once more :)

larsoner · 2021-10-07T00:04:58Z

Linux pip-pre 3.9:

 48.69s total  mne/report/tests/

Linux conda 3.9:

 48.69s total  mne/report/tests/

In an unrelated PR #9810 (should be same as main) this group's time for Linux conda 3.9 was:

123.41s total  mne/report/tests/

So we now improve on this PR by just testing PyVistaQt. We could make it test Mayavi as well, but we're hopefully removing that soon enough that it won't matter. Or even changing whichever test is fastest that uses renderer_pyvistaqt to be renderer might be enough... I might push a commit to do this.

agramfort · 2021-10-07T05:19:52Z

Thx @hoechenberger !

rob-luke · 2021-10-07T05:24:31Z

Very cool. Can't wait to try these out.

hoechenberger · 2021-10-07T06:17:51Z

Thank you so much, @larsoner!

* upstream/main: MAINT: Update broken link, fix rendering (mne-tools#9829) Ensure plot_ica_sources() always plots traces of rejected ICs on top (mne-tools#9823) Improve plot_ica_sources() docstring (mne-tools#9825) MRG: Fix docstring for plot_ica_components() (mne-tools#9826) unpin jsonschema and filter its warning instead (mne-tools#9822) Add warning for SNIRF files with != 2 wavelengths (mne-tools#9817) add show_scalebars param to epochs.plot() (mne-tools#9815) MRG: Allow _plot_mri_contours() to return arrays (mne-tools#9818) MRG: Expand ~ in _check_fname() (mne-tools#9613) Improve ICA.plot_overlay() docstrings (mne-tools#9820) WIP, MAINT: Fix CircleCI (again) (mne-tools#9814) MRG, ENH: Add options to fit_dipole (mne-tools#9810) Rework Reports (new history) (mne-tools#9754) MRG, CI: Use VTK pre (mne-tools#9803)

Pull in changes from old PR

ac7eae3

larsoner reviewed Sep 16, 2021

View reviewed changes

hoechenberger and others added 27 commits September 17, 2021 14:45

Merge branch 'main' of https://github.com/mne-tools/mne-python into r…

e6d1d6b

…eport2

Remove _BEM_VIEWS from __init__.py

6c5f049

Add back cropped MNE logo

9c236d9

Change default of raw_psd back to False again

7785ffd

Remove old JS libraries

7e09555

Don't call tight_layout() on figs passed to _fig_to_img()

1b7cc18

Reduce verbosity related to prjector handling

6f3c4cd

Don't produce empty drop figure if no epochs were dropped

a918003

Don't add legend to GFP plots

0a1f725

use viz.utils.tight_layout()

284a2ea

More tight_layout()

20de178

Fix warnings

73c037d

Finally tight_layout() issues 🙈

35d24c6

Add back time zone test

0d07801

Deprecation handling

e6f3af3

Better deprecation testing

21276ee

ALMOST finish work on tutorial

a1113d6

Disable carousel buttons

3c94403

Finalize tutorial

e1ed295

Maximize main content column width

58a30b4

Allow more than 1 BEM sliders in a row

202e092

Apply changes discussed with Alex

fe4ebe2

flake

1006f31

Merge branch 'main' of https://github.com/mne-tools/mne-python into r…

659d040

…eport2

Add options to decimate time points in STC and Evoked plots

5d05cef

agramfort approved these changes Oct 5, 2021

View reviewed changes

ENH: Speed up test

c625b7c

larsoner requested changes Oct 5, 2021

View reviewed changes

hoechenberger added 2 commits October 5, 2021 20:47

Merge branch 'main' of https://github.com/mne-tools/mne-python into r…

1cc8b1e

…eport2

Speed up evoked rendering in test

7809b3e

MAINT: Speed up tests

2f30351

FIX: Also mayavi

d4a32b8

larsoner approved these changes Oct 7, 2021

View reviewed changes

agramfort merged commit 6763cac into mne-tools:main Oct 7, 2021

This was referenced Oct 7, 2021

BUG: _plot_mri_contours makes too many figures #9812

Closed

WIP, MAINT: Fix CircleCI #9813

Closed

hoechenberger mentioned this pull request Oct 23, 2021

Use fixes._compare_version for version checks everywhere #9894

Closed

hoechenberger deleted the report2 branch December 7, 2021 16:16

Uh oh!

Rework Reports (new history) #9754

Rework Reports (new history) #9754

Uh oh!

Conversation

hoechenberger commented Sep 16, 2021

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hoechenberger commented Sep 17, 2021

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

hoechenberger commented Oct 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

hoechenberger commented Oct 5, 2021

Uh oh!

larsoner commented Oct 5, 2021

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

hoechenberger commented Oct 5, 2021

Uh oh!

hoechenberger commented Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larsoner commented Oct 6, 2021

Uh oh!

hoechenberger commented Oct 6, 2021

Uh oh!

larsoner commented Oct 7, 2021

Uh oh!

agramfort commented Oct 7, 2021

Uh oh!

rob-luke commented Oct 7, 2021

Uh oh!

hoechenberger commented Oct 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hoechenberger commented Oct 5, 2021 •

edited

Loading

hoechenberger commented Oct 6, 2021 •

edited

Loading