-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Improve parallel default selection on macOS #5383
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In Qiskit#5324 we changed the default for running with parallel map on macOS to default to false unconditionally, but that was likely too strict. While there are a myriad of issues related to Python multiproccessing and macOS it doesn't fail in every macOS environment. Additionally the performance benefits from running in parallel is signtificant. This commit attempts to make the default change a bit more nuanced. Instead of defaulting to false on all macOS systems it only does that for Python 3.8 and macOS. This is because in python 3.8 the use of spawn instead of fork means in the best cases if it works completely as expected it will be slower (see $5323) but also means there are unforseen consequences (see Qiskit#5376) because it's spawning a new interpreter instead of forking. But, often it just doesn't work. For other python versions there are still potential issues on newer versions of macOS (>=10.14) so to catch potential issues there a small parallel benchmark is run on import to ensure that it both works and is faster than serial. If both are true then we default it parallelism to True, otherwise it's False. Fixes Qiskit#5373
mtreinish
changed the title
Improve parallel default selection on macOS
[WIP] Improve parallel default selection on macOS
Nov 11, 2020
Heh, ok the first draft of the benchmark is hanging the on import in macOS. So this definitely needs work still. |
The use of the parallel benchmark for macOS to determine whether parallel execution is actually faster or not was hanging on import. I expect this was due to it being executed on import and not having sufficient context awareness in the subprocesses to handle that. To avoid the hang this just makes the default for macOS parallel processing to be True if Python < 3.8.
mtreinish
changed the title
[WIP] Improve parallel default selection on macOS
Improve parallel default selection on macOS
Jan 12, 2021
kdk
approved these changes
Mar 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In #5324 we changed the default for running with parallel map on macOS
to default to false unconditionally, but that was likely too strict.
While there are a myriad of issues related to Python multiproccessing
and macOS it doesn't fail in every macOS environment. Additionally the
performance benefits from running in parallel is signtificant. This
commit attempts to make the default change a bit more nuanced. Instead
of defaulting to false on all macOS systems it only does that for Python
3.8 and macOS. This is because in python 3.8 the use of spawn instead of
fork means in the best cases if it works completely as expected it will
be slower (see #5323) but also means there are unforseen consequences
(see #5376) because it's spawning a new interpreter instead of forking.
But, often it just doesn't work. For other python versions there are
still potential issues on newer versions of macOS (>=10.14) but in general
it still provides a speedup if it works.
Details and comments
Fixes #5373