-
Notifications
You must be signed in to change notification settings - Fork 55
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
Faster unit tests/notebooks #380
Comments
Thanks for compiling this, @sserita! Regarding the contents of test packages, I anticipate a few of these being addressed as part of #373. The changes on that branch broke some of the unit tests in that module, so I decided that since I was in the code already fixing them to also streamline some of them at the same time. |
Great! Whenever #373 is ready, I'll go through and check off whichever ones you've already gotten to. |
Question regarding the runtimes above. What system and pyGSTi version were these timings collected on? I.e. was this done locally on your system (presumably an apple silicon macbook) or on the github runners? I ask because I noticed some of the runtimes were significantly longer than I had recalled seeing on my system (windows laptop with xeon W-10855M processor) following the updates in #372. For example, looking at the second test in the I am running this on the bugfix-stricter-line-label-enforcement branch, but as far as I recall haven't made any changes to the tests in question since #372. |
The Edit: I just ran this without |
I was running these on a single thread so didn't have any dist flags set. But we do use that flag on the runners (and probably should based on what I understand about it) so maybe I should? If I recall correctly I think I had previously decided that flag is probably more relevant when doing multi-threaded notebook regressions than the standard unit tests. The notebook regression tests really need to have all of the cells in a notebook run on a single worker, whereas typically the standard unit tests should be generally more robust in this regard. Edit: I just re-ran the tests on my machine with multiple threads and the |
Yea agreed something seems wonky - or maybe I was running something else at the same time in the background. Timings are updated - do those seem more reasonable across the board? This was no |
I just spot checked a few of the updated timings and these seem much more reasonable. |
One thing that might be worth looking into in the course of trying to speed up the testing process is our current list of dependencies installed using the testing flag. A good chunk of the overall time is setting up the runners and installing packages, so removing any unneeded dependencies would be helpful here. |
With #373 merged in I re-ran the unit tests to get updated timings and see what bottlenecks were still unresolved. Note that I cheated a bit and re-ran these on songthrush (dynobird and quail were in use), so these aren't a perfect 1-for-1 comparison, but should hopefully be close enough. Ran tests on a single thread (though some of the linear algebra stuff was clearly utilizing openMP based on htop). Everything in test/unit around 5s or above:
Everything in test/test_packages around/over 5s (note previous list was over a minute)
Everything in jupyter_notebooks around/over 45 seconds:
I have started taking a crack at some of these on a new branch called 'feature-faster-testing-stuff', and will check things off as I make progress. |
This is mostly completed with the merge of #403. |
Is your feature request related to a problem? Please describe.
The unit and notebook regression tests take too long.
Describe the solution you'd like
Faster unit tests. This could be achieved by using different inputs or options which don't affect coverage but do affect runtime. Simple examples for GST related tests include using lower germ lengths, smaller gate sets, etc. This was already done for the
test/test_packages/algorithms
tests in #372, but we should do this across the whole codebase.Task list
Below are tasklists made by running pytest with
--durations 0
and taking the most expensive tests.Everything in
test/unit
around 5s or above:Everything in
test/test_packages
around/over a minute:Everything in jupyter_notebooks around/over a minute:
The text was updated successfully, but these errors were encountered: