CPU Saturation test (prerequisite for #1175 PR)
This branch includes an additional test aimed at highlighting the CPU usage while running a high number of simulations. ...
Does this close any currently open issues?
This is an addition that aims to give the devs a way to test the fix that will be proposed in the next days for the simulate_for_sbi function
Any relevant code examples, logs, error output, etc?
You don't need to let the test finish, it is just supposed to provide the workload.
Any other comments?
Remember that the testing is actually done by looking at the process manager / htop while the test is running.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask. We're here to
help! This is simply a reminder of what we are going to look for before merging
your code.
- [x] I have read and understood the contribution guidelines
- [x] I agree with re-licensing my contribution from AGPLv3 to Apache-2.0.
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] I have reported how long the new tests run and potentially marked them
with
pytest.mark.slow. - [x] New and existing unit tests pass locally with my changes
- [x] I performed linting and formatting as described in the contribution guidelines
- [x] I rebased on
main(or there are no conflicts withmain) - [ ] For reviewer: The continuous deployment (CD) workflow are passing.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 75.61%. Comparing base (
ba19688) to head (4601b04).
Additional details and impacted files
@@ Coverage Diff @@
## main #1187 +/- ##
==========================================
- Coverage 84.55% 75.61% -8.95%
==========================================
Files 96 96
Lines 7603 7603
==========================================
- Hits 6429 5749 -680
- Misses 1174 1854 +680
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 75.61% <ø> (-8.95%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
I suggest, one the other PR #1188 is merged, we can merge main into this branch to then double check whether CPU saturation has improved with the fixes.
If yes, we can actually delete the cpu_saturation_test.py as it is not really a CI test. We only keep the changes to multiprocessing_test.py. Would you agree?
I suggest, one the other PR #1188 is merged, we can merge
maininto this branch to then double check whether CPU saturation has improved with the fixes. If yes, we can actually delete thecpu_saturation_test.pyas it is not really a CI test. We only keep the changes tomultiprocessing_test.py. Would you agree?
Sure makes sense and keeps main history less cluttered. I'd then post the runtime differences in this thread.
Here are the performances before and after the simulate_for_sbi refactoring:
- old implementation
- new implementation
Interestingly, the CPU usage is not fully saturated in either case:
- old implementation
- new implementation
This could be due to the fact that the test is not computationally intensive, as other, heavier, simulations have shown to fully saturate CPU with the new implementation.
At this point, this branch has served its proof-of-concept purpose and can be deleted without merging (to the best of my understanding).