tofu icon indicating copy to clipboard operation
tofu copied to clipboard

test suite execution time is too long

Open flothesof opened this issue 2 years ago • 4 comments

Hi, as discussed in #609 and other comments, I strongly believe the test execution time should be kept low. I believe it has only recently increased to around 11 minutes, which is a 5x or 450% increase from the 2 minutes it took a few months ago. I’m not opposed to that sort of thing, but I am pretty sure this can be cut back to 2 minutes quite easily by changing some parameters in the longest tests.

After inspecting things just on the surface, I’ve also noticed that the tutorials have become part of the tests. I think this should not be the case: sphinx-gallery is designed to handle failing examples (see here https://sphinx-gallery.github.io/stable/configuration.html#dealing-with-failing-gallery-example-scripts). A better way to test this would be to include documentation generation in the CI instead of copying and pasting examples in the tests!

I really think we can keep the same coverage but keep durations low.

Cheers, Florian

flothesof avatar Jan 14 '22 15:01 flothesof

Below a list of slowest durations on my machine.

======================================================================================================== slowest 10 durations =========================================================================================================
195.23s call     tofu/tests/tests04_spectro/test_01_fit12d.py::Test01_DataCollection::test03_fit1d
41.28s call     tofu/tests/tests05_nist/test_03_core.py::Test01_openadas::test01_search_online_by_wavelengthA
40.62s call     tofu/tests/tests03_openadas/test_03_core.py::Test01_openadas::test01_search_online
17.29s call     tofu/tests/tests01_geom/test_04_core_optics.py::Test01_Crystal::test14_plot_focal_error_summed
12.96s setup    tofu/tests/tests01_geom/test_03_core.py::Test03_Rays::test00_set_move
12.63s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test19_plot_svd
11.33s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test18_spectrogram
10.60s call     tofu/tests/tests01_geom/test_04_sampling.py::test05_sa_integ_map
9.16s call     tofu/tests/tests02_data/test_03_core.py::Test01_DataCam12D::test16_compare
8.66s call     tofu/tests/tests01_geom/test_03_core.py::Test01_Struct::test16_plot

flothesof avatar Jan 14 '22 15:01 flothesof

Thanks @flothesof !

  • first line (195.23 s) : this can probably be reduced by tuning, this tests fits a large number of spectra and tests all combinations of options for the fitting parameters, but there is probably some redundancy, I'll look into it now

  • second and third line (41.28 s and 40.62 s) : this goes online, sends http requests to a website to check it successfully recovers a result, but again, I'm probably testing too many combinations

  • Others: would require more work, the others are testing core numerical routines, I'm not very tempted to risk reducing coverage

By looking closely into the first 3 lnes I should be able to save 1 or 2 minutes I guess

Didou09 avatar Jan 14 '22 15:01 Didou09

Great, thank you for your feedback!  That would be a nice start.

Another thing that surprised me while looking at the tests is: almost none of your test functions have a docstring. I’m not saying this to annoy you, but a single line docstring for tests does wonders sometimes (even though you tried to think hard about the name of the test). If you do a PR, it would be nice to include docstrings for the tests you change :D

flothesof avatar Jan 14 '22 15:01 flothesof

commits #5833fe8 and #68e8f9a in PR #611 (the last 2 commits) partially address this issue.

But the PR cannot be merged as unit tests are not fully passing, opened issue #612 to solve this.

Didou09 avatar Jan 14 '22 19:01 Didou09