qiboteam / qibo

A full-stack framework for quantum computing.
https://qibo.science
Apache License 2.0
294 stars 60 forks source link

Serialization of measurements #1431

Closed BrunoLiegiBastonLiegi closed 2 months ago

BrunoLiegiBastonLiegi commented 2 months ago

This addresses the bug #1426 by implementing a proper serialization of measurement gates and their result. Furthermore, the possibility to specify the basis of measurement gates with strings (for instance "Z" or ["X", "Z", "Y"]) has been added. I also made some minor modifications to MeasurementResult, and I would also like to remove the circuit attribute from it. At the moment it is only used to generate the samples when they are not available. However, as I see it, the MeasurementResult should always be associated with the samples.

Checklist:

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.94%. Comparing base (21732ad) to head (7864590). Report is 123 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1431 +/- ## ======================================= Coverage 99.94% 99.94% ======================================= Files 78 78 Lines 11317 11324 +7 ======================================= + Hits 11311 11318 +7 Misses 6 6 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibo/pull/1431/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qibo/pull/1431/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `99.94% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alecandido commented 2 months ago

Furthermore, the possibility to specify the basis of measurement gates with strings (for instance "Z" or ["X", "Z", "Y"]) has been added.

Try to keep features' implementation as separate as possible (and separate from bug fixes).

(Ok, this is really a tiny one, but it was worth to mention the general rule)

BrunoLiegiBastonLiegi commented 2 months ago

Yeah, let's say that rather than a new feature is a side effect of the fix in this case, but I fundamentally agree.

BrunoLiegiBastonLiegi commented 2 months ago

Ok, I should have addressed everything now, except the MeasurementResult.circuit attribute that is still there, but I would get rid of it if everyone agrees.

marcorossi5 commented 2 months ago

Hello guys, I've installed qibo from this source branch and checked if the qibo-webappdaemon works smoothly. Unfortunately we are having the following error raised by the following snippet:

with open(circuit_path, "r") as f:
      raw = json.load(f)
c = qibo.Circuit.from_dict(raw)
r = c(nshots=settings.get("nshots"))
print(f"Results: {r.samples()}")
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/nfs/users/tiiq/webappdaemon/src/daemon/slurm_circuit_computation.py", line 62, in <module>
    main(args.job_folder)
  File "/nfs/users/tiiq/webappdaemon/src/daemon/slurm_circuit_computation.py", line 55, in main
    print(f"Results: {r.samples()}")
  File "/nfs/users/tiiq/qibo/src/qibo/result.py", line 324, in samples
    qubits = self.measurement_gate.target_qubits
AttributeError: 'NoneType' object has no attribute 'target_qubits'
scarrazza commented 2 months ago

Do you have a simpler example using only qibo primitives?

marcorossi5 commented 2 months ago

I did a little bit of investigation. It seems that the circuit serialization done with circuit.raw looks different when launched on a CPU only queue (like sim) or on a QPU one (qw11q).

The script is the following:

import json
import qibo

c = qibo.Circuit(5)
c.add(qibo.gates.M(0))

with open("circuit.json", "w") as f:
    json.dump(c.raw, f)

print(c.raw)

Here you have the results:

Output on sim

{'queue': [{'name': 'measure', 'init_args': (0,), 'init_kwargs': {'register_name': None, 'collapse': False, 'basis': ['Z'], 'p0': None, 'p1': None}, '_target_qubits': (0,), '_control_qubits': (), '_class': 'M', 'measurement_result': {'samples': None}}], 'nqubits': 5, 'density_matrix': False, 'qibo_version': '0.2.12'}

Output on qw11q:

{'queue': [{'name': 'measure', 'init_args': [0], 'init_kwargs': {}, '_target_qubits': [0], '_control_qubits': [], '_class': 'M'}], 'nqubits': 5, 'density_matrix': False, 'qibo_version': '0.2.11'}

It seems that there's a mismatch when loading a circuit on the QPU that was created on CPU and viceversa.

The code that raises the following errors is:

import json
import qibo
with open("circuit.json") as f:
    raw = json.load(f)

c = qibo.Circuit.from_dict(raw)

If I create the circuit on CPU => load it on QPU:

Traceback (most recent call last):
  File "/nfs/users/tiiq/from_raw.py", line 6, in <module>
    c = qibo.Circuit.from_dict(raw)
  File "/nfs/tools/qibo/lib/python3.10/site-packages/qibo/models/circuit.py", line 1137, in from_dict
    circ.add(Gate.from_dict(gate))
  File "/nfs/tools/qibo/lib/python3.10/site-packages/qibo/gates/abstract.py", line 109, in from_dict
    gate = cls(*raw["init_args"], **raw["init_kwargs"])
  File "/nfs/tools/qibo/lib/python3.10/site-packages/qibo/gates/measurements.py", line 101, in __init__
    gate = basis_cls(qubit).basis_rotation()
TypeError: 'str' object is not callable
srun: error: fahid: task 0: Exited with exit code 1

If I create the circuit on QPU => load it on CPU:

Traceback (most recent call last):
  File "/nfs/users/tiiq/from_raw.py", line 6, in <module>
    c = qibo.Circuit.from_dict(raw)
  File "/nfs/users/tiiq/qibo/src/qibo/models/circuit.py", line 1137, in from_dict
    circ.add(Gate.from_dict(gate))
  File "/nfs/users/tiiq/qibo/src/qibo/gates/abstract.py", line 121, in from_dict
    if raw["_class"] == "M" and raw["measurement_result"]["samples"] is not None:
KeyError: 'measurement_result'
BrunoLiegiBastonLiegi commented 2 months ago

The output on sim should be the correct one, which was updated in this PR. The other one seems to be the old serialization of the M gate (the one in main).

Similarly below, the CPU -> QPU loading seems to fail because the M object is not able to receive strings as basis arguments, which is an additional feature introduced here. Viceversa, the QPU -> CPU loading fails because the object serialized on the QPU doesn't include attributes that are required to load a measurement now, i.e. the serialization of the MeasurementResult.

I would say that the CPU and QPU are using two different versions of qibo:

marcorossi5 commented 2 months ago

Let me check again the environments. If the branch mismatch is the cause, then we are fine.

scarrazza commented 2 months ago

I have just tried your code and it works, no crashes.

marcorossi5 commented 2 months ago

Yes, I agree. I was loading the wrong environment on the QPU, my bad. I guess the test is passed then