-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fix parallel optimisation and increase coverage #299
Fix parallel optimisation and increase coverage #299
Conversation
…sing import and context set
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #299 +/- ##
===========================================
+ Coverage 95.49% 95.79% +0.30%
===========================================
Files 38 38
Lines 2040 2045 +5
===========================================
+ Hits 1948 1959 +11
+ Misses 92 86 -6 ☔ View full report in Codecov by Sentry. |
This PR is now ready for review. There is a larger term discussion on how we handle
At the moment, this adds multiprocessing for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks interesting, @BradyPlanden, nice! I wonder if something like Dask can be used to fix what has been happening on Windows here (and also, given the recent parallel discussion on PyBaMM). It could be too much work to add, though 😬
Actually, this won't entirely work and might break further sometime. For example, if one does import pybop
import sys and then something like this
gives us this: tap to view output
so, running tap to view output
Therefore, we should delete all of these entries (i.e., |
Thanks for the feedback @agriyakhetarpal. I've updated now, I believe this fully unloads the module. Let me know if you catch any other issues! |
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
Description
Bugfix optimisation.parallel, add coverage tests, initial multiprocessing import and context set.
Issue reference
Fixes #298
Review
Before you mark your PR as ready for review, please ensure that you've considered the following:
Type of change
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ nox -s tests
$ nox -s doctest
You can run integration tests, unit tests, and doctests together at once, using
$ nox -s quick
.Further checks:
Thank you for contributing to our project! Your efforts help us to deliver great software.