Overhaul Qiskit Experiments #47

nkanazawa1989 · 2023-08-22T18:07:41Z

This is the proposal for refactoring plan of Qiskit Experiments. See details for the main document.

Since Qiskit Experiments project is moved to Qiskit-Extensions, where we can rely more on the community, this document includes a long educatory introduction for active developers who newly join this project.

eliarbel · 2023-08-23T09:40:46Z

0000-overhaul-qiskit-experiments.md

+few hundreds or maybe thousands of qubits.
+This document spots current performance bottlenecks and describes how to overcome the performance issues.
+
+As described in the [project documentation](https://qiskit.org/ecosystem/experiments/tutorials/intro.html#what-is-qiskit-experiments), QE consists of


It seem that this RFC handles 2 big proposed changes:

Improving analysis (hence QE) scalability

Breaking ExperimentData to more manageable pieces (via executors).

Obviously 2 facilitates 1 but we can focus on 1 first without going through the steps suggested for 2, if it creates a reasonable overhead with implementing 1 without 2 and then transitioning to 2. So I wonder if this is the case or if doing 1 without 2 is just too much work and these changes should come together

Yes, you are correct. 1 and 2 can be done in parallel, and currently @to24toro is investigating the JAX fitter. We can also improve figure generation. However seems like refactoring of composite analysis is very important to improve the user experience. Since composite analysis is tied to the experiment data, other developer can push forward 2.

eliarbel · 2023-08-23T09:41:30Z

0000-overhaul-qiskit-experiments.md

+
+Because composite analysis needs to instantiate new data container to run component analyses,
+calling the constructor of this expensive class consumes significant time in heavily batched experiment.
+Especially, acquisition of a service object and qiskit version information is the expensive part.


Should we recommend working in a Python prompt for some use-cases, such that this overhead is paid once?

Can you elaborate? I am thinking to introduce simpler container that doesn't have connection to experiment service. I think current design of _run_analysis(self, experiment_data) gives too much freedom to experiment developers, since technically they can modify remote data during analysis. This could introduce some edge case that we don't want to support.

I was referring to caching service object and qiskit version in the a python session (say IPython). A container without experiments service is obviously useful for certain use-cases, but what about those use-cases that a user will require results DB access?

I meant analysis usually doesn't require results DB access, so we can introduce simplified internal data container that doesn't have experiment service. Caching service object is easy and effective, but do we really need service object to run experiment analysis?

eliarbel · 2023-08-23T09:42:16Z

0000-overhaul-qiskit-experiments.md

+If experiments are conducted in the dedicated queue mode or with a [runtime in a session](https://qiskit.org/ecosystem/ibm-runtime/how_to/run_session.html), 
+the significant overhead in the local analysis task may also deteriorate the machine efficiency of the remote quantum computing system.
+
+With the proposed overhaul plan, we aim to achieve the T1 analysis wall time of < 5 sec at nq=100 either with a high-end laptop or workstation (sorry we don't have proper benchmark suite for QE!).


I've started something here: https://github.com/eliarbel/qiskit-experiments-benchmarks. We should discuss if and how we want to leverage this or something like this for formalizing a benchmark framework for QE

I don't have access to the repo, but I remember you showed a handmade benchmark suite. If we want to continuously monitor the performance, I guess we need to setup some bare metal server. @mtreinish knows how to setup the benchmak.

Right, ideally we should have a bare metal server and something like ASV. Let's have a separate discussion about this aspect.

0000-overhaul-qiskit-experiments.md

eliarbel · 2023-08-23T09:43:15Z

0000-overhaul-qiskit-experiments.md

+
+If we have enough bandwidth to support end-user migration, alternative implementation is worth considering.
+
+


I think we need more discussion around the alternative approach (i.e. getting rid of BaseExperiment.run), which indeed brings us closer to the runtime way of running. I don't know how acute is the single line of execution capability to our users. Personally I prefer 3 vertical calls instead of 3 horizontal (chained) calls; this way the code better captures the relevant steps, it's easier to debug if needed, profile etc.. But in any case we can keep BaseExperiment.run for compatibility (even after the deprecation period) in parallel of having executor-base flow supported.

If I can design the pattern from scratch without considering the backward compatibility, I would propose something like this.

with Executor(backend, shots=1000, block=True) as exp_data: qe.run(MyExperiment((0,), **options)) # get executor object from the contextvar

Context is very powerful, for example, I don't need explicit composite experiment classes:

with Executor(backend, shots=1000, block=True) as exp_data: with qe.parallel(): for q in qubits: qe.run(MyExperiment((q,), **options))

That's nice but I would also consider a non-context based syntax, just not to enforce using with blocks for users who prefer the standard code layout for some reason

If runtime sessions will become the default for most users, I think the context based syntax as the Q-E default is fine. I like the idea of keeping BaseExperiment.run for compatibility but promoting this context-based syntax as the standard pattern.

I added more comment about extendability in a99c946. Let's separately discuss execution pattern and focus on internal rework in this RFC. We need more user feedback to decide the interface.

coruscating

This is really nice work!

coruscating · 2023-08-30T19:18:27Z

0000-overhaul-qiskit-experiments.md

+Another important situation that we may need to cover is the break of the executor run.
+Job queueing in a quantum system may take very long time (sometime more than a day) if you don't reserve the device, 
+and we often terminate the kernel where the circuit executor is actively running.
+In this situation, we may want to restart from the analysis, once after all jobs successfully complete.


I think we should also add an option to not run the analysis automatically after job completion, like @TsafrirA's use case.

Yes. We can continue to support Experiment.run(analysis=None). In this case the analysis executor will receive an empty task dependency graph and run nothing.

coruscating · 2023-08-30T19:18:43Z

0000-overhaul-qiskit-experiments.md

+```python
+exc = Executor()
+exc.circuit_executor.backend = some_backend
+exc.circuit_executor.add_circuits(transpiled_circuits)


I like this usage pattern. Where would the circuit validation hook be?

Could you please elaborate? This is something to validate the circuit metadata against analysis class?

Yes, I was thinking something to validate that the metadata and circuit output formats are what's expected by the analysis class. We don't necessarily have to implement it for every analysis class, but having a template hook would be useful.

Added description in b83b848. Validation for composite experiment circuit might become too complicated to implement. Probably we need to simplify the metadata structure.

coruscating · 2023-08-30T19:29:17Z

0000-overhaul-qiskit-experiments.md

+
+If we have enough bandwidth to support end-user migration, alternative implementation is worth considering.
+
+


If runtime sessions will become the default for most users, I think the context based syntax as the Q-E default is fine. I like the idea of keeping BaseExperiment.run for compatibility but promoting this context-based syntax as the standard pattern.

coruscating

LGTM! 🚀

nkanazawa1989 added 2 commits August 23, 2023 03:00

First proposal for QE overhaul

2044afe

Mention termination of the running executor.

bfca662

eliarbel reviewed Aug 23, 2023

View reviewed changes

Replace JobExecutor -> CircuitExecutor to avoid confusion

3382f99

1ucian0 added the RFC proposal New RFC is proposed label Aug 28, 2023

coruscating reviewed Aug 30, 2023

View reviewed changes

nkanazawa1989 added 2 commits September 6, 2023 09:27

Move discussion of execution pattern to future extension.

a99c946

Add description for metadata validation

b83b848

coruscating approved these changes Sep 11, 2023

View reviewed changes

nkanazawa1989 added 3 commits September 12, 2023 08:53

Assign RFC number 0014

49620dc

Fix figure paths

b28f740

Update readme table

97c509e

nkanazawa1989 merged commit 2f480ad into Qiskit:master Sep 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Overhaul Qiskit Experiments #47

Overhaul Qiskit Experiments #47

nkanazawa1989 commented Aug 22, 2023 •

edited

Loading

eliarbel Aug 23, 2023

nkanazawa1989 Aug 23, 2023 •

edited

Loading

eliarbel Aug 23, 2023

nkanazawa1989 Aug 23, 2023

eliarbel Aug 29, 2023

nkanazawa1989 Aug 30, 2023

eliarbel Aug 23, 2023

nkanazawa1989 Aug 23, 2023

eliarbel Aug 29, 2023 •

edited

Loading

eliarbel Aug 23, 2023

nkanazawa1989 Aug 23, 2023 •

edited

Loading

eliarbel Aug 29, 2023

coruscating Aug 30, 2023

nkanazawa1989 Sep 6, 2023

coruscating left a comment

coruscating Aug 30, 2023

nkanazawa1989 Sep 6, 2023

coruscating Aug 30, 2023

nkanazawa1989 Sep 6, 2023

coruscating Sep 6, 2023

nkanazawa1989 Sep 11, 2023

coruscating Aug 30, 2023

coruscating left a comment


		If we have enough bandwidth to support end-user migration, alternative implementation is worth considering.

Overhaul Qiskit Experiments #47

Overhaul Qiskit Experiments #47

Conversation

nkanazawa1989 commented Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

nkanazawa1989 Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eliarbel Aug 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkanazawa1989 Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coruscating left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coruscating left a comment

Choose a reason for hiding this comment

nkanazawa1989 commented Aug 22, 2023 •

edited

Loading

nkanazawa1989 Aug 23, 2023 •

edited

Loading

eliarbel Aug 29, 2023 •

edited

Loading

nkanazawa1989 Aug 23, 2023 •

edited

Loading