-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add should_run_in_parallel
to public API
#12412
base: main
Are you sure you want to change the base?
Conversation
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 9100279306Details
💛 - Coveralls |
The function was added in a stable manner to enable backport to the 1.1 series, but from 1.2 onwards, we want this to be part of the public interface so that others can rely on it too.
d6d1da9
to
0bdea9c
Compare
Now rebased over #12410. |
decision is dependent on how many CPUs are available to Qiskit, what the :mod:`multiprocessing` | ||
start method is, how many processes were requested. |
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.
I mean sort of, we don't explicitly check what multiprocessing is set to (ie we ignore if the user explicitly sets this to spawn), but the value for PARALLEL_DEFAULT
is based on what the OS default start method is.
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.
Yeah, I think I was being more aspirational than correct here. We should do the check based on the multiprocessing
start method if that's what we care about, but we don't. I can change the wording.
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.
Maybe we should just do this, it seems totally in scope to update the logic in the function to call multiprocessing.get_start_method()
and get rid of the OS based logic here for 1.2.
Summary
The function was added in a stable manner to enable backport to the 1.1 series, but from 1.2 onwards, we want this to be part of the public interface so that others can rely on it too.
Details and comments
This was elided from #12410 to make that PR backwards compatible. This PR exposes the feature as part of the public API, so will be new for 1.2.
Depends on #12410.