-
Notifications
You must be signed in to change notification settings - Fork 131
[WIP] Rework experiment data callbacks for running analysis #398
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
[WIP] Rework experiment data callbacks for running analysis #398
Conversation
This doesn't fix the failing tests, but stops them from erroring due to change in `add_data` signature.
I'm confused by this. How does
There are a couple of caveats about using |
This crashes execution of analysis with no-data entry error because |
nkanazawa1989
left a comment
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 like add_processing_callback method. However, this allows one to call the method multiple times. What is the expected behavior if different callbacks are added? (Since there is no error handling, it is acceptable behavior?).
| self.backend, | ||
| ) | ||
| self._backend = data.backend() | ||
| self._backend = job.backend() |
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
| self._backend = job.backend() | |
| if not self._backend: | |
| self._backend = job.backend() |
?
| else: | ||
| raise TypeError(f"Invalid data type {type(data)}.") | ||
| # Get the last added future and add a done callback | ||
| _, future = self._job_futures[-1] |
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.
What happens in self._job_futures[:-1]? Submission of jobs and their completion are always in the same order? I guess this is not guaranteed because job is scheduled according to some formula and it prioritizes jobs with cost, i.e. number of circuits, shots, etc...
| job = backend.run(qobj) | ||
| # Run experiment jobs | ||
| max_experiments = getattr(backend.configuration(), "max_experiments", None) | ||
| if max_experiments and len(circuits) > max_experiments: |
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 should be taken also from run options. If we use pulse gate and scan a parameter of very long pulse, this job will quickly consume the waveform memory on hardware and raise the memory overflow error on a backend. To prevent this, we often split a job to keep total waveform data point number sufficiently small. For example, CR Hamiltonian tomography often runs into this situation.
|
@jyu00 @nkanazawa1989 This is now replaced by #407 which is refactored to use separate futures for analysis to avoid using the @jyu00 The job splitting is now in #402 and this was requested by @mtreinish so experiments can work for 3rd party providers, not just IBM backends. |
Summary
This PR attempts to rework the ExperimentData class so that data processing callbacks can be added to the job future objects using a separate method
add_processing_callbackthat uses theadd_done_callbackmethod of the Python Future API.It also combines multiple jobs added together in a single
add_datacall into a single future object, this is necessary to support job splitting for backends that have a maximum number of circuits per job that is less than the number of circuits generated by experiments.Splitting an experiment with large numbers of circuits into multiple jobs is also implemented in this PR.
Details and comments
The
run_analysismethod for BaseExperiment uses this new callback function. The goal of this is that callingrun_analysiswill not require blocking on results to work correctly so that the follwoing should be equivalent:These changes seem to work correctly for most experiments except the recently added CR Hamiltonian tomography one (which im not sure why only it seems to fail), and the result DB tests which explicitly use the old callback API which are all failling (and im not exactly sure how they should be updated at the moment).
I would like to get feedback on this approach before going any further with trying to fix the tests.