Skip to content
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

Fix barrier instructions in tomography experiments #1059

Merged
merged 5 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def circuits(self):
prep_circ = self._prep_circ_basis.circuit(prep_element, self._prep_physical_qubits)
circ.reset(self._prep_indices)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this one actually causing an issue? All 1-qubit instructions in terra are setup to broadcast over the arguments so shouldn't have any issues with qpy, and unless that changes I dont think we need to change this (And if there is specifically an issue with tuple, then it can be converted to a list)

eg reset doc string indicates it works with more than 1-qubit:

reset(qubit: 'QubitSpecifier') -> 'InstructionSet' method of qiskit.circuit.quantumcircuit.QuantumCircuit instance
    Reset the quantum bit(s) to their default state.
    
    Args:
        qubit: qubit(s) to reset.
    
    Returns:
        qiskit.circuit.InstructionSet: handle to the added instruction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it back. You and I put more weight on different parts of the quoted text 🙂 The qubit: 'QubitSpecifier' type annotation says to me that it accepts only a single qubit.

Copy link
Collaborator

@chriseclectic chriseclectic Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just went with qubit(s) :D

circ.compose(prep_circ, self._prep_indices, inplace=True)
circ.barrier(self._prep_indices)
circ.barrier(*self._prep_indices)

# Add target circuit
# Have to use compose since circuit.to_instruction has a bug
Expand All @@ -231,7 +231,7 @@ def circuits(self):
# Add tomography measurement
if meas_element:
meas_circ = self._meas_circ_basis.circuit(meas_element, self._meas_physical_qubits)
circ.barrier(self._meas_indices)
circ.barrier(*self._meas_indices)
circ.compose(meas_circ, self._meas_indices, meas_clbits, inplace=True)

# Add metadata
Expand Down
10 changes: 10 additions & 0 deletions releasenotes/notes/tomo-barriers-aae4aafedaca5c3d.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
fixes:
- |
Fix qpy serialization and deserialization of tomography experiments. The
barrier instructions in tomography experiments were created with the wrong
Python type which qpy did not support. This issue was most acute when using
`qiskit-ibm-provider` which submits circuits to the provider using qpy.
There could have been subtler issues with circuit timing using a different
provider if the barriers were not separating important circuit
instructions. See `#1060 <https://github.com/Qiskit/qiskit-experiments/issues/1060>`_.
27 changes: 25 additions & 2 deletions test/library/tomography/test_process_tomography.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,24 @@
"""
ProcessTomography experiment tests
"""
import io
from test.base import QiskitExperimentsTestCase

import ddt
import numpy as np
from qiskit import QuantumCircuit

import qiskit.quantum_info as qi
from qiskit import QuantumCircuit, qpy
from qiskit.circuit.library import XGate, CXGate
from qiskit.result import LocalReadoutMitigator
import qiskit.quantum_info as qi

from qiskit_aer import AerSimulator
from qiskit_aer.noise import NoiseModel

from qiskit_experiments.database_service import ExperimentEntryNotFound
from qiskit_experiments.library import ProcessTomography, MitigatedProcessTomography
from qiskit_experiments.library.tomography import ProcessTomographyAnalysis, basis

from .tomo_utils import (
FITTERS,
filter_results,
Expand Down Expand Up @@ -90,6 +96,23 @@ def test_full_qpt_analysis_none(self):
self.assertExperimentDone(expdata)
self.assertFalse(expdata.analysis_results())

def test_circuit_serialization(self):
"""Test simple circuit serialization"""
circ = QuantumCircuit(2)
circ.h(0)
circ.s(0)
circ.cx(0, 1)

exp = ProcessTomography(circ)
circs = exp.circuits()

qpy_file = io.BytesIO()
qpy.dump(circs, qpy_file)
qpy_file.seek(0)
new_circs = qpy.load(qpy_file)

self.assertEqual(circs, new_circs)

def test_cvxpy_gaussian_lstsq_cx(self):
"""Test fitter with high fidelity threshold"""
seed = 1234
Expand Down
25 changes: 23 additions & 2 deletions test/library/tomography/test_state_tomography.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@
"""
StateTomography experiment tests
"""
import io
from test.base import QiskitExperimentsTestCase
from math import sqrt

import ddt
import numpy as np
from qiskit import QuantumCircuit

import qiskit.quantum_info as qi
from qiskit import QuantumCircuit, qpy
from qiskit.circuit.library import XGate
from qiskit.result import LocalReadoutMitigator
import qiskit.quantum_info as qi

from qiskit_aer import AerSimulator
from qiskit_aer.noise import NoiseModel

Expand Down Expand Up @@ -191,6 +195,23 @@ def test_exp_circuits_measurement_indices(self, meas_qubits):
fid = qi.state_fidelity(target_state, qi.Statevector(target_circ))
self.assertGreater(fid, 0.99, msg="target_state is incorrect")

def test_circuit_serialization(self):
"""Test simple circuit serialization"""
circ = QuantumCircuit(2)
circ.h(0)
circ.s(0)
circ.cx(0, 1)

exp = StateTomography(circ)
circs = exp.circuits()

qpy_file = io.BytesIO()
qpy.dump(circs, qpy_file)
qpy_file.seek(0)
new_circs = qpy.load(qpy_file)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond scope of this PR, but we should probably add similar tests for serialization of generated circuits for all experiments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #1063 to keep track of this.

Copy link
Collaborator

@chriseclectic chriseclectic Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can probably use the round trip json serialization test function (like we use for experiment data), since json serializer should serialize a list of circuits via qpy.


self.assertEqual(circs, new_circs)

@ddt.data([0], [1], [2], [0, 1], [1, 0], [0, 2], [2, 0], [1, 2], [2, 1])
def test_full_exp_measurement_indices(self, meas_qubits):
"""Test subset state tomography generation"""
Expand Down