qiboteam / qibo-core

Core qibo elements
https://qibo.science
1 stars 0 forks source link

Repeated execution #34

Open alecandido opened 5 months ago

alecandido commented 5 months ago

We should reconsider how repeated execution is being triggered, and when it is possible to enable it automatically.

The extreme option is to have a single execute_circuit function with no flag at all, just decide whether to execute

just based on the content of the Circuit and the initial_state.

Cf.:

renatomello commented 5 months ago

Not being explicit may lead to user misunderstanding of what’s happening because results from repeated execution are only the same as density matrix simulation in the limit of infinite shots

On Tue, 18 Jun 2024 at 10:52, Alessandro Candido @.***> wrote:

We should reconsider how repeated execution is being triggered, and when it is possible to enable it automatically.

The extreme option is to have a single execute_circuit function with no flag at all, just decide whether to execute

  • state vector simulation
  • density matrix simulation
  • shots execution

just based on the content of the Circuit and the initial_state.

Cf.:

— Reply to this email directly, view it on GitHub https://github.com/qiboteam/qibo-core/issues/34, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABH5QVR77YEGUAVSJ6E3W7TZH7KKTAVCNFSM6AAAAABJPL74M2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGM2TSMBXG43TQNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

alecandido commented 5 months ago

What I mean is not avoiding being explicit, but just avoiding duplicated features (even now, execute_circuit might trigger execute_repeated_circuits) https://github.com/qiboteam/qibo/blob/70901d598ad77ca43abf1d9b3673608206cc6099/src/qibo/backends/numpy.py#L404-L406 and the need for manually removing portions of "phase space" (through error raising) https://github.com/qiboteam/qibo/blob/70901d598ad77ca43abf1d9b3673608206cc6099/src/qibo/backends/numpy.py#L407-L410

If there is an error, it means that there is an invalid combination of inputs. It'd be better if most of the inputs' combinations are valid, preserving at the same time the meaning of the parameters (avoiding the usage of weird combinations, leading to poor understanding by users), and if there were only one way of doing things (cf. PEP 20), and not many different combinations of invocations leading to the same result.

The proposal in the comments was along the line:

This should reproduce the behavior of the current Qibo (more or less, since it extracts more information from the initial_state parameter) but with a single public function, and just two parameters.

renatomello commented 5 months ago

What I mean is not avoiding being explicit, but just avoiding duplicated features (even now, execute_circuit might trigger execute_repeated_circuits) https://github.com/qiboteam/qibo/blob/70901d598ad77ca43abf1d9b3673608206cc6099/src/qibo/backends/numpy.py#L404-L406 and the need for manually removing portions of "phase space" (through error raising) https://github.com/qiboteam/qibo/blob/70901d598ad77ca43abf1d9b3673608206cc6099/src/qibo/backends/numpy.py#L407-L410

If there is an error, it means that there is an invalid combination of inputs. It'd be better if most of the inputs' combinations are valid, preserving at the same time the meaning of the parameters (avoiding the usage of weird combinations, leading to poor understanding by users), and if there were only one way of doing things (cf. PEP 20), and not many different combinations of invocations leading to the same result.

The proposal in the comments was along the line:

* to trigger a density matrix simulation, you need an explicit density matrix initial state (flagged by shape or type) - in that case, and only in that case, a density matrix simulation is always run

* if the circuit is _unitary_, a state vector simulation is run, being the initial state a state vector or `None`

* if the circuit is _non-unitary_ (because it contains channels, or collapsing measurements), repeated execution is triggered

This should reproduce the behavior of the current Qibo (more or less, since it extracts more information from the initial_state parameter) but with a single public function, and just two parameters.

Maybe we have a nomenclature conflict here.

Repeated execution, as I always understood and as is stated in the Qibo doc here, is about simulating noise models on statevectors probabilistically instead of exact noisy simulation on density matrices. In the limit of infinite samples, the former converges to the latter. A regular circuit with collapsing measurements do not require "repeated execution" under this definition of repeated execution.

Note that, in principle, "repeated execution" as defined above is independent of the presence of measurement gates in the circuit. However, its current implementation requires measurements. Moreover, repeated execution only makes sense for noise channels that are described by a linear combination of unitaries, which is not the case for e.g. a generic ReadoutErrorChannel or a generic KrausChannel.

In any case, I wouldn't advise on defaulting to "repeated execution" if the combination of noise channels and statevector simulation is true. That may lead to misunderstanding because the results wouldn't be exact but could be naively interpreted as such. On the other hand, the current requirements are noise channels + statevector simulation + measurements. But those are not necessary conditions, since measurements are not necessary as I stated above.

Maybe an extra boolean flag is needed, or the triggering of repeated execution also triggers a warning saying that the simulation won't be exact.

alecandido commented 5 months ago

Maybe an extra boolean flag is needed, or the triggering of repeated execution also triggers a warning saying that the simulation won't be exact.

Even better: instead of raising a warning, we could return an object explicitly telling that is coming from repeated execution (as a result object with a flag set to "not exact").