Proposal for deprecation of flatten_result=False

[nkanazawa1989]

Suggested feature

Performance of the composite analysis is of vital importance to support 100+ qubit calibration with Qiskit Experiments. As we have previously investigated, the most time consuming part of analysis is the figure generation, but this overhead can be easily alleviated by selectively generating the figures, e.g. generating figure for bad quality results.

Apart from the figure generation, we can further reduce computational time cost by introducing better fitting subroutine #1192. Lastly (in this proposal), we can completely remove nested experiment data container structure for more speedup.

%load_ext snakeviz

import numpy as np
from qiskit_experiments.framework import ParallelExperiment
from qiskit_experiments.library import T1

from qiskit.providers.fake_provider import FakePerth
from qiskit_aer import AerSimulator
from qiskit_aer.noise import NoiseModel

noise_model = NoiseModel.from_backend(
    FakePerth(), thermal_relaxation=True, gate_error=False, readout_error=False
)
backend = AerSimulator.from_backend(FakePerth(), noise_model=noise_model)
delays = np.linspace(0, 500e-6, 20)

parallels = []
for q in range(backend.configuration().num_qubits):
    exp = T1((q,), delays=delays)
    exp.analysis.set_options(plot=False)
    parallels.append(exp)
exp = ParallelExperiment(parallels, flatten_results=True, backend=backend)
exp_data = exp.run(analysis=None).block_for_results()

%snakeviz exp._analysis._run_analysis(exp_data)

Above benchmark is a typical pattern in system maintenance where we run same kind of experiment in parallel (Figure generation is disabled to focus on the rest of steps). In this situation, CompositeAnalysis does

When flatten_results=False

• Initialize sub containers
• Marginalize count data and add to sub containers
• Run component analysis on corresponding sub container

When flatten_results=True

• Initialize sub containers
• Marginalize count data and add to sub containers
• Run component analysis on corresponding sub container
• Extract results and figures from sub container and add them to the outermost container
• Discard sub containers

As you can see, flatten_results option is just a difference of how to deliver the outcomes, and sub containers are initialized anyways. According to the benchmark, sub container initialization consumes 30% of analysis time in total (wait block is mainly time for fitting, and this is not analyzed with this profiler because the process is in another thread).

Proposal

In #1133 , we support table representation of analysis results. So far nested container structure might be preferred to efficiently analyze the results. In above parallel experiment example, if we want T1 value of Qubit3,

print(exp_data.child_data(3).analysis_results("T1").value)

With table, user can get nice html overview of all analysis results.

res = exp_data.analysis_results("T1")

This makes me think we no longer have motivation to keep complicated nested data structure. Given we agree with this, we don't need to make sub containers at all. Note that we still need to marginalize the count data, but this does NOT need to be an expensive ExperimentData container.

With this modification, BaseAnalysis would look like

    def run(experiment_data: ExperimentData, ...) -> ExperimentData:

        def run_callback(exp_data):
            results = self._process_data(exp_data.data())
            analysis_results, figures = self._run_analysis(results)
            ...
        
       experiment_data.add_analysis_callback(run_callback)
       return experiment_data

    @abstractmethod
    def _process_data(data: List[Dict]) -> ExperimentResults:  # Can be replaced by primitive

    @abstractmethod
    def _run_analysis(results: ExperimentResults) -> Tuple[List[AnalysisResultData], List[Figure]]:

and CompositeAnalysis would become

    @abstractmethod
    def _run_analysis(results: ExperimentResults) -> Tuple[List[AnalysisResultData], List[Figure]]:
        for component_analysis, component_results in zip(self._analyses, self._marginalize(results)):
            component_analysis_results, component_figures = component_analysis._run_analysis(component_results)
        ...        

The CompositeAnalysis._margianlize is a function that consumes a composite ExperimentResults and generates Itertor[ExperimentResults] for each component data. The marginal operation is implemented with Rust, so this subroutine must be performant.

Note that _run_analysis is an abstract method and this is API break for existing (including community) experiment libraries. However, we can safely introduce deprecation warning with

    try:
        self._run_analysis(results)
    except AttributeError:
        warnings.warn(..., DeprecationWarning)
        self._run_analysis(experiment_data)

because the class must access experiment_data.data() to perform analysis (assuming ExperimentResults doesn't have this attribute).

[yiiyama]

If I may chime in, since I've looked into why ExperimentData initialization takes so long in the past - the biggest time consumer seemed to be these lines where network communications happen, and they are triggered when service=None in the constructor, which is what is done now in CompositeAnalysis. It's actually trivial to add service=experiment_data.service to this line, and with that tweak the creation of nested ExperimentData is not that slow, actually.

[nkanazawa1989]

Thanks @yiiyama . This makes sense. Could you please open a PR to change the initialization of the experiment data container? Apart from performance, another intention of the proposed change here is the separation of data container and analysis subroutine. ._run_analysis(experiment_data: ExperimentData) allows too much freedom for the analysis subroutine. Indeed this subroutine is run as a part of analysis callback, but a developer can also add another analysis callback from the running callback. This could cause issues like deadlock, memory-leak, etc...

[gadial]

Thanks @yiiyama. Please note that setting service=experiment_data.service is tricky, since it only makes sense in the context of the old qiskit-ibmq-provider, not for the new qiskit-ibm-provider (this is why this "set service from provider" mechanism was added in the first place). I didn't expect such a performance hit, but I think we can solve this by setting the service once in the parent, and making the sub-experiments use that service.

[yiiyama]

@nkanazawa1989 Right, I read the motivation for this proposal and agree that this is a good idea. I just stumbled upon this issue while browsing the Issues page and wanted to share what I found before.
So @gadial Naoki asked me to open a PR but I'm afraid I don't know what exactly I should do if service=experiment_data.service isn't the solution. Would you take care of making the sub-experiments reuse the service (or not - I guess it won't matter once this proposal is implemented)?

[nkanazawa1989]

Closed and merged into #1268