Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[tests] Speed up test_chunkteacher #3606

Merged
merged 3 commits into from
Apr 19, 2021
Merged

[tests] Speed up test_chunkteacher #3606

merged 3 commits into from
Apr 19, 2021

Conversation

stephenroller
Copy link
Contributor

Patch description
Speed up some slower basic unit tests, mostly by removing a combinatorial explosion of options that isn't really necessary. These tests are also somewhat flaky but hopefully this also helps with that.

Testing steps
Before:

29.17s call     tests/test_chunkteacher.py::TestNumExamples::test_slow_batchsort
26.23s call     tests/test_chunkteacher.py::TestNumExamples::test_slow_dynb
25.44s call     tests/test_chunkteacher.py::TestNumExamples::test_mp_slow_batchsort
22.49s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_mp_slow_dynb
22.28s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_mp_slow_batchsort
18.69s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_slow_dynb
18.63s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_slow_batchsort
12.64s call     tests/test_chunkteacher.py::TestNumExamples::test_slow_bs3
12.55s call     tests/test_chunkteacher.py::TestNumExamples::test_slow_bs2
12.53s call     tests/test_chunkteacher.py::TestNumExamples::test_slow_bs1
======= 60 passed, 113 warnings in 477.21s (0:07:57) =======

After:

$ pytest tests/test_chunkteacher.py
15.95s call     tests/test_chunkteacher.py::TestSlowChunk::test_slow_dynb
10.15s call     tests/test_chunkteacher.py::TestSmallBuffer::test_mp_small_buffer_batchsort
9.12s call     tests/test_chunkteacher.py::TestNumExamples::test_mp_normal_dynb
8.19s call     tests/test_chunkteacher.py::TestNumExamples::test_normal_dynb
8.04s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_mp_normal_dynb
7.80s call     tests/test_chunkteacher.py::TestSmallBuffer::test_mp_small_buffer_dynb
5.24s call     tests/test_chunkteacher.py::TestNumExamples::test_mp_normal_bs3
5.10s call     tests/test_chunkteacher.py::TestNumExamples::test_normal_batchsort
4.96s call     tests/test_chunkteacher.py::TestBackgroundPreprocessorNumExamples::test_mp_normal_bs3
4.77s call     tests/test_chunkteacher.py::TestSmallBuffer::test_mp_small_buffer_bs3
======= 22 passed, 113 warnings in 104.89s (0:01:44) =======

@@ -66,14 +65,6 @@ def test_normal_batchsort(self):
task='integration_tests:chunky', batchsize=2, dynamic_batching='batchsort'
)

@testing_utils.skipUnlessGPU
def test_mp_normal_bs1(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's redundant with the others. We have a non-multprocessing batchsize 1, and we have multiprocessing with other batchsizes. The explicit combination seems unnecessary.

Same goes for some of the other things I filtered out. I decided to test some things in isolation, rather than the combinatorial explosion of options.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think the remaining test would catch sth like distributed_world_size > 1 while batchsize = 1 (is this sth that is necessary to test?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think they will. I can bring that one back if you'd like.

Copy link
Contributor

@jxmsML jxmsML left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!! Thanks for the fix! 🚀

@stephenroller stephenroller merged commit d83cd22 into master Apr 19, 2021
@stephenroller stephenroller deleted the test_chunky branch April 19, 2021 20:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants