-
Notifications
You must be signed in to change notification settings - Fork 364
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
Cluster backend #1281
Cluster backend #1281
Conversation
3155e76
to
4a17d16
Compare
274320a
to
dee725b
Compare
dee725b
to
d715065
Compare
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.
Overall this looks good. I did a quick initial skim over the code and have a couple inline comments. Also this definitely needs a release note (see: https://github.com/Qiskit/qiskit-aer/blob/main/CONTRIBUTING.md#release-notes ) to document the new feature and how to use it.
Also I'm wondering do you think we want to ask dask as an optional extra dependency in the setup.py. The code relies on the futures interface so it's not required, but it might be nice to let users do something like pip install 'qiskit-aer[dask]'
. What do you think?
if "executor" in run_options: | ||
executor = run_options["executor"] | ||
del run_options["executor"] |
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.
Instead of doing this why not just add executor
to the default options? That will also enable users to something like:
sim_backend = AerSimulator(executor=dask_magic())
sim_backend.run([qc]*10**5)
sim.backend.run([new_qc]*10**4)
and have it all run on dask.
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.
At first, I added the executor option as your code but DASK client variable is needed to be a local variable for the serialization. So I moved the executor option as a run option.
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 don't really understand this comment on why it can't be a a config option? Is the probably re-using an executor between calls to run
? If so can executors be copied to avoid this?
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.
class AerBackend(Backend, ABC):
def __init__():
self.executor = None:
/* ... */
def run():
self.executor = getattr(self._options, 'executor')
class AerBackend(Backend, ABC):
/*...*/
def run():
executor = None
executor = self._options_configuration["executor"]
These codes occur a cloudpickle error because the executor is not a local variable. The same error is mentioned in this website
So I changed the executor
option as the run option because executor
is passed every calling run
function. Are there any good idea to realize the executor
options as a config option?
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 found that the following code can be executed. I don't know why this code is ok but I can move executor
option to default options.
class AerBackend(Backend, ABC):
def __init__():
self.executor = None:
/* ... */
def run():
executor = None
if self._executor:
executor = self._executor
else:
executor = getattr(self._options, 'executor')
delattr(self._options, 'executor')
/* ....*/
aer_job_set = AerJobSet(self, job_id, self._run, experiments, executor)
aer_job_set.submit()
self._executor = executor
return aer_job_set
if backend_options and "executor" in backend_options: | ||
executor = backend_options["executor"] | ||
del backend_options["executor"] |
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.
Do we really want to support doing this in backend_options
even though it's deprecated.
Thank you for the comment. I will add a release note soon.
I agree. I will update setup.py to install dask if users add the option. |
Separate logic for a custom executor and job splitting so both can be changed separately. * Adds `executor` option to simulators for specifying a custom executor * Adds `max_job_size` option to simulators for specifying the max number of circuits per job when submitting to executor. If less than the number of circuits this will result in a job set. * Add max_size options to splitting code to control job sizes * Clean up splitting code in aerbackend so there are not so many conditionals
Change executor tests to use AerSimulator and reduce CI load on number of run tests, since we don't need to test too many circuits.
This reverts commit 10c8bb8.
This reverts commit c6edd4c.
Separate logic for a custom executor and job splitting so both can be changed separately. * Adds `executor` option to simulators for specifying a custom executor * Adds `max_job_size` option to simulators for specifying the max number of circuits per job when submitting to executor. If less than the number of circuits this will result in a job set. * Add max_size options to splitting code to control job sizes * Clean up splitting code in aerbackend so there are not so many conditionals
Change executor tests to use AerSimulator and reduce CI load on number of run tests, since we don't need to test too many circuits.
@hitomitak I accidently pushed directly to your branch when making some changes so I reverted those commits and then added them to a PR to your branch: hitomitak#2 |
Updates to executor and job size options
@chriseclectic Thank you for the refactoring. I changed the code to get |
b6b2706
to
3910f0e
Compare
3910f0e
to
86d7981
Compare
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.
Hiding the Dask (and running MPI) documentation in the contributing guide, which is intended for documenting additional details for developers building from source, seems wrong. We should make a follow up PR to move this documentation to its own page in the API docs. Other than that LGTM
@@ -738,6 +738,45 @@ meta = dict['metadata'] | |||
myrank = meta['mpi_rank'] | |||
``` | |||
|
|||
### Running with Threadpool and DASK |
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.
We should move this and the MPI docs in contributing to a page in the API docs, this can be done as a follow up though
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.
Changes for renamed file
Getting serialization errors while trying to submit Qiskit Circuit List to Qiskit Aer Simulator With respect to project: qiskit-advocate/qamp-fall-21#39, I am trying to simulate few simple Qiskit circuit lists on a Qiskit Aer Simulator, with compute engine as Dask on Kubernetes Cluster, but continuously facing serialization (pickling) error(s). I am even getting these errors while testing a DASK code example available on Qiskit Documentation Portal : https://qiskit.org/documentation/apidoc/parallel.html ** Execution Environment**
Error Messages Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): |
Summary
Take over #1084
Adds a new option of the backend to provide the user's executor. When user gives dask client as executor, Aer can execute a simulation on the distributed machines like HPC clusters.
When the executor is set, AerJobSet object is returned instead of a normal AerJob object.
AerJobSet divides multiple experiments in one qobj into each experiment and submits each qobj to the executor as AerJob.
After simulations, AerJobSet collects each result and combines them into one result object.
Example Usage:
Threadpool execution:
Dask execution:
Details and comments
I executed QV (10 depth, 64 circuits)and measured the execution time on 3VMs with Dask executor.
Env: 3VMs, OS: CentOS8.0, Memory 64 GB, CPU: 32 x 2.0 GHz or higher Cores.
1 Node : a normal Aer simulator
1 Node (DASK) : only one node with dask executor
2 Node (DASK) : two nodes with dask executor
3 Node (DASK) : three nodes with dask executor