mirror of https://github.com/Qiskit/qiskit.git
Fix `QuantumCircuit.compose` on unusual types and registers (#9206)
* Fix `QuantumCircuit.compose` on unusual types and registers `QuantumCircuit.compose` previously had its own handling for converting bit specifiers into bit instances, meaning its functionality was often surprisingly less permissive than `append`, or just incorrect. This swaps the method to use the standard conversion utilities. The logic for handling mapping conditional registers was previously the old logic in `DAGCircuit`, which was called without respect to re-ordering of the clbits during the composition. The existing test case surrounding this that is modified by this commit made an incorrect assertion; in general, the condition was not the same after the composition. In this particular test case it was ok because both bits were being tested for the same value, but had the condition value been 0b01 or 0b10 it would have been incorrect. This commit updates the register-mapping logic to respect the reordering of clbits, and to create a new aliasing register to ensure the condition is represented exactly, if this is required. This fixes a category of bug where too-small registers could incorrectly be mapped to larger registers. This was not a valid transformation, because it created assertions about bits that were previously not involved in the condition's comparison. * Remove now-outdated DAG compose test This test was actually attempting to catch invalid state of `QuantumCircuit.compose`, but this series of commits changes the underlying function so that this test no longer needs to raise; there is valid alternative behaviour. * Fix lint * Add comment explaining list copy * Update test/python/circuit/test_compose.py Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: Julien Gacon <gaconju@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
Jake Lishman
GitHub
parent
7ed5de3eb5
commit
2629079167
|
|
@ -771,8 +771,8 @@ class QuantumCircuit:
|
|||
def compose(
|
||||
self,
|
||||
other: Union["QuantumCircuit", Instruction],
|
||||
qubits: Optional[Sequence[Union[Qubit, int]]] = None,
|
||||
clbits: Optional[Sequence[Union[Clbit, int]]] = None,
|
||||
qubits: Optional[Union[QubitSpecifier, Sequence[QubitSpecifier]]] = None,
|
||||
clbits: Optional[Union[ClbitSpecifier, Sequence[ClbitSpecifier]]] = None,
|
||||
front: bool = False,
|
||||
inplace: bool = False,
|
||||
wrap: bool = False,
|
||||
|
|
@ -842,98 +842,101 @@ class QuantumCircuit:
|
|||
"Cannot emit a new composed circuit while a control-flow context is active."
|
||||
)
|
||||
|
||||
if inplace:
|
||||
dest = self
|
||||
else:
|
||||
dest = self.copy()
|
||||
dest = self if inplace else self.copy()
|
||||
|
||||
# If self does not have any cregs but other does then we allow
|
||||
# that and add the registers to the output dest
|
||||
# As a special case, allow composing some clbits onto no clbits - normally the destination
|
||||
# has to be strictly larger. This allows composing final measurements onto unitary circuits.
|
||||
if isinstance(other, QuantumCircuit):
|
||||
if not self.clbits and other.clbits:
|
||||
dest.add_bits(other.clbits)
|
||||
for reg in other.cregs:
|
||||
dest.add_register(reg)
|
||||
|
||||
if wrap:
|
||||
try:
|
||||
other = other.to_gate()
|
||||
except QiskitError:
|
||||
other = other.to_instruction()
|
||||
if wrap and isinstance(other, QuantumCircuit):
|
||||
other = (
|
||||
other.to_gate()
|
||||
if all(isinstance(ins.operation, Gate) for ins in other.data)
|
||||
else other.to_instruction()
|
||||
)
|
||||
|
||||
if not isinstance(other, QuantumCircuit):
|
||||
if qubits is None:
|
||||
qubits = list(range(other.num_qubits))
|
||||
|
||||
qubits = self.qubits[: other.num_qubits]
|
||||
if clbits is None:
|
||||
clbits = list(range(other.num_clbits))
|
||||
|
||||
clbits = self.clbits[: other.num_clbits]
|
||||
if front:
|
||||
dest.data.insert(0, CircuitInstruction(other, qubits, clbits))
|
||||
# Need to keep a reference to the data for use after we've emptied it.
|
||||
old_data = list(dest.data)
|
||||
dest.clear()
|
||||
dest.append(other, qubits, clbits)
|
||||
for instruction in old_data:
|
||||
dest._append(instruction)
|
||||
else:
|
||||
dest.append(other, qargs=qubits, cargs=clbits)
|
||||
|
||||
if inplace:
|
||||
return None
|
||||
return dest
|
||||
|
||||
instrs = other.data
|
||||
|
||||
if other.num_qubits > dest.num_qubits or other.num_clbits > dest.num_clbits:
|
||||
raise CircuitError(
|
||||
"Trying to compose with another QuantumCircuit which has more 'in' edges."
|
||||
)
|
||||
|
||||
# number of qubits and clbits must match number in circuit or None
|
||||
identity_qubit_map = dict(zip(other.qubits, dest.qubits))
|
||||
identity_clbit_map = dict(zip(other.clbits, dest.clbits))
|
||||
edge_map = {}
|
||||
if qubits is None:
|
||||
qubit_map = identity_qubit_map
|
||||
elif len(qubits) != len(other.qubits):
|
||||
raise CircuitError(
|
||||
f"Number of items in qubits parameter ({len(qubits)}) does not"
|
||||
f" match number of qubits in the circuit ({len(other.qubits)})."
|
||||
)
|
||||
edge_map.update(zip(other.qubits, dest.qubits))
|
||||
else:
|
||||
qubit_map = {
|
||||
other.qubits[i]: (self.qubits[q] if isinstance(q, int) else q)
|
||||
for i, q in enumerate(qubits)
|
||||
}
|
||||
if clbits is None:
|
||||
clbit_map = identity_clbit_map
|
||||
elif len(clbits) != len(other.clbits):
|
||||
raise CircuitError(
|
||||
f"Number of items in clbits parameter ({len(clbits)}) does not"
|
||||
f" match number of clbits in the circuit ({len(other.clbits)})."
|
||||
)
|
||||
else:
|
||||
clbit_map = {
|
||||
other.clbits[i]: (self.clbits[c] if isinstance(c, int) else c)
|
||||
for i, c in enumerate(clbits)
|
||||
}
|
||||
mapped_qubits = dest.qbit_argument_conversion(qubits)
|
||||
if len(mapped_qubits) != len(other.qubits):
|
||||
raise CircuitError(
|
||||
f"Number of items in qubits parameter ({len(mapped_qubits)}) does not"
|
||||
f" match number of qubits in the circuit ({len(other.qubits)})."
|
||||
)
|
||||
edge_map.update(zip(other.qubits, mapped_qubits))
|
||||
|
||||
edge_map = {**qubit_map, **clbit_map} or {**identity_qubit_map, **identity_clbit_map}
|
||||
if clbits is None:
|
||||
edge_map.update(zip(other.clbits, dest.clbits))
|
||||
else:
|
||||
mapped_clbits = dest.cbit_argument_conversion(clbits)
|
||||
if len(mapped_clbits) != len(other.clbits):
|
||||
raise CircuitError(
|
||||
f"Number of items in clbits parameter ({len(mapped_clbits)}) does not"
|
||||
f" match number of clbits in the circuit ({len(other.clbits)})."
|
||||
)
|
||||
edge_map.update(zip(other.clbits, dest.cbit_argument_conversion(clbits)))
|
||||
|
||||
mapped_instrs = []
|
||||
for instr in instrs:
|
||||
condition_register_map = {}
|
||||
for instr in other.data:
|
||||
n_qargs = [edge_map[qarg] for qarg in instr.qubits]
|
||||
n_cargs = [edge_map[carg] for carg in instr.clbits]
|
||||
n_instr = instr.operation.copy()
|
||||
n_op = instr.operation.copy()
|
||||
|
||||
if getattr(instr.operation, "condition", None) is not None:
|
||||
from qiskit.dagcircuit import DAGCircuit # pylint: disable=cyclic-import
|
||||
# Map their registers over to ours, adding an extra one if there's no exact match.
|
||||
if getattr(n_op, "condition", None) is not None:
|
||||
target, value = n_op.condition
|
||||
if isinstance(target, Clbit):
|
||||
n_op.condition = (edge_map[target], value)
|
||||
else:
|
||||
if target.name not in condition_register_map:
|
||||
mapped_bits = [edge_map[bit] for bit in target]
|
||||
for our_creg in dest.cregs:
|
||||
if mapped_bits == list(our_creg):
|
||||
new_target = our_creg
|
||||
break
|
||||
else:
|
||||
new_target = ClassicalRegister(bits=[edge_map[bit] for bit in target])
|
||||
dest.add_register(new_target)
|
||||
condition_register_map[target.name] = new_target
|
||||
n_op.condition = (condition_register_map[target.name], value)
|
||||
|
||||
n_instr.condition = DAGCircuit._map_condition(
|
||||
edge_map, instr.operation.condition, self.cregs
|
||||
)
|
||||
|
||||
mapped_instrs.append(CircuitInstruction(n_instr, n_qargs, n_cargs))
|
||||
mapped_instrs.append(CircuitInstruction(n_op, n_qargs, n_cargs))
|
||||
|
||||
if front:
|
||||
# adjust new instrs before original ones and update all parameters
|
||||
mapped_instrs += dest.data
|
||||
dest.data.clear()
|
||||
dest._parameter_table.clear()
|
||||
dest.clear()
|
||||
append = dest._control_flow_scopes[-1].append if dest._control_flow_scopes else dest._append
|
||||
for instr in mapped_instrs:
|
||||
append(instr)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
The method :meth:`.QuantumCircuit.compose` now accepts the same set of qubit and clbit
|
||||
specifiers as other :class:`.QuantumCircuit` methods, such as :meth:`~.QuantumCircuit.append`.
|
||||
This means, for example, that Numpy integers will work. Fixed `#8691
|
||||
<https://github.com/Qiskit/qiskit-terra/issues/8691>`__.
|
||||
- |
|
||||
Fixed :meth:`.QuantumCircuit.compose` incorrectly mapping registers in conditions on the given
|
||||
circuit to complete registers on the base. Previously, the mapping was very imprecise; the bits
|
||||
used within each condition were not subject to the mapping, and instead an inaccurate attempt was
|
||||
made to find a corresponding register. This could also result in a condition on a smaller register
|
||||
being expanded to be on a larger register, which is not a valid transformation. Now, a
|
||||
condition on a single bit or a register will be composed to be on precisely the bits as defined
|
||||
by the ``clbits`` argument. A new aliasing register will be added to the base circuit to
|
||||
facilitate this, if necessary. Fixed `#6583 <https://github.com/Qiskit/qiskit-terra/issues/8691>__`.
|
||||
|
|
@ -16,6 +16,8 @@
|
|||
|
||||
import unittest
|
||||
|
||||
import numpy as np
|
||||
|
||||
from qiskit import transpile
|
||||
from qiskit.pulse import Schedule
|
||||
from qiskit.circuit import (
|
||||
|
|
@ -107,6 +109,30 @@ class TestCircuitCompose(QiskitTestCase):
|
|||
circuit_composed = self.circuit_left.compose(circuit_right, inplace=False)
|
||||
self.assertEqual(circuit_composed, circuit_expected)
|
||||
|
||||
def test_compose_inorder_unusual_types(self):
|
||||
"""Test that composition works in order, using Numpy integer types as well as regular
|
||||
integer types. In general, it should be permissible to use any of the same `QubitSpecifier`
|
||||
types (or similar for `Clbit`) that `QuantumCircuit.append` uses."""
|
||||
qreg = QuantumRegister(5, "rqr")
|
||||
creg = ClassicalRegister(2, "rcr")
|
||||
circuit_right = QuantumCircuit(qreg, creg)
|
||||
circuit_right.cx(qreg[0], qreg[3])
|
||||
circuit_right.x(qreg[1])
|
||||
circuit_right.y(qreg[2])
|
||||
circuit_right.z(qreg[4])
|
||||
circuit_right.measure([0, 1], [0, 1])
|
||||
|
||||
circuit_expected = self.circuit_left.copy()
|
||||
circuit_expected.cx(self.left_qubit0, self.left_qubit3)
|
||||
circuit_expected.x(self.left_qubit1)
|
||||
circuit_expected.y(self.left_qubit2)
|
||||
circuit_expected.z(self.left_qubit4)
|
||||
circuit_expected.measure(self.left_qubit0, self.left_clbit0)
|
||||
circuit_expected.measure(self.left_qubit1, self.left_clbit1)
|
||||
|
||||
circuit_composed = self.circuit_left.compose(circuit_right, np.arange(5), slice(0, 2))
|
||||
self.assertEqual(circuit_composed, circuit_expected)
|
||||
|
||||
def test_compose_inorder_inplace(self):
|
||||
"""Composing two circuits of the same width, default order, inplace.
|
||||
|
||||
|
|
@ -397,9 +423,9 @@ class TestCircuitCompose(QiskitTestCase):
|
|||
lqr_2_1: ──┤ X ├────┤ X ├────┼───┤M├─╫─
|
||||
└───┘ └─┬─┘ │ └╥┘ ║
|
||||
┌──┴──┐┌──┴──┐ ║ ║
|
||||
lcr_0: ════════════╡ ╞╡ ╞═╩══╬═
|
||||
│ = 3 ││ = 3 │ ║
|
||||
lcr_1: ════════════╡ ╞╡ ╞════╩═
|
||||
lcr_0: ════════════╡ ╞╡ ╞═╬══╩═
|
||||
│ = 3 ││ = 3 │ ║
|
||||
lcr_1: ════════════╡ ╞╡ ╞═╩════
|
||||
└─────┘└─────┘
|
||||
"""
|
||||
qreg = QuantumRegister(2, "rqr")
|
||||
|
|
@ -411,16 +437,46 @@ class TestCircuitCompose(QiskitTestCase):
|
|||
circuit_right.measure(qreg, creg)
|
||||
|
||||
# permuted subset of qubits and clbits
|
||||
circuit_composed = self.circuit_left.compose(circuit_right, qubits=[1, 4], clbits=[1, 0])
|
||||
circuit_composed = self.circuit_left.compose(circuit_right, qubits=[1, 4], clbits=[0, 1])
|
||||
|
||||
circuit_expected = self.circuit_left.copy()
|
||||
circuit_expected.x(self.left_qubit4).c_if(*self.condition)
|
||||
circuit_expected.h(self.left_qubit1).c_if(*self.condition)
|
||||
circuit_expected.measure(self.left_qubit4, self.left_clbit0)
|
||||
circuit_expected.measure(self.left_qubit1, self.left_clbit1)
|
||||
circuit_expected.measure(self.left_qubit1, self.left_clbit0)
|
||||
circuit_expected.measure(self.left_qubit4, self.left_clbit1)
|
||||
|
||||
self.assertEqual(circuit_composed, circuit_expected)
|
||||
|
||||
def test_compose_conditional_no_match(self):
|
||||
"""Test that compose correctly maps registers in conditions to the new circuit, even when
|
||||
there are no matching registers in the destination circuit.
|
||||
|
||||
Regression test of gh-6583 and gh-6584."""
|
||||
right = QuantumCircuit(QuantumRegister(3), ClassicalRegister(1), ClassicalRegister(1))
|
||||
right.h(1)
|
||||
right.cx(1, 2)
|
||||
right.cx(0, 1)
|
||||
right.h(0)
|
||||
right.measure([0, 1], [0, 1])
|
||||
right.z(2).c_if(right.cregs[0], 1)
|
||||
right.x(2).c_if(right.cregs[1], 1)
|
||||
test = QuantumCircuit(3, 3).compose(right, range(3), range(2))
|
||||
z = next(ins.operation for ins in test.data[::-1] if ins.operation.name == "z")
|
||||
x = next(ins.operation for ins in test.data[::-1] if ins.operation.name == "x")
|
||||
# The registers should have been mapped, including the bits inside them. Unlike the
|
||||
# previous test, there are no matching registers in the destination circuit, so the
|
||||
# composition needs to add new registers (bit groupings) over the existing mapped bits.
|
||||
self.assertIsNot(z.condition, None)
|
||||
self.assertIsInstance(z.condition[0], ClassicalRegister)
|
||||
self.assertEqual(len(z.condition[0]), len(right.cregs[0]))
|
||||
self.assertIs(z.condition[0][0], test.clbits[0])
|
||||
self.assertEqual(z.condition[1], 1)
|
||||
self.assertIsNot(x.condition, None)
|
||||
self.assertIsInstance(x.condition[0], ClassicalRegister)
|
||||
self.assertEqual(len(x.condition[0]), len(right.cregs[1]))
|
||||
self.assertEqual(z.condition[1], 1)
|
||||
self.assertIs(x.condition[0][0], test.clbits[1])
|
||||
|
||||
def test_compose_gate(self):
|
||||
"""Composing with a gate.
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ import unittest
|
|||
|
||||
from qiskit.circuit import QuantumRegister, ClassicalRegister, QuantumCircuit
|
||||
from qiskit.converters import circuit_to_dag, dag_to_circuit
|
||||
from qiskit.dagcircuit.exceptions import DAGCircuitError
|
||||
from qiskit.test import QiskitTestCase
|
||||
from qiskit.pulse import Schedule
|
||||
from qiskit.circuit.gate import Gate
|
||||
|
|
@ -424,31 +423,6 @@ class TestDagCompose(QiskitTestCase):
|
|||
|
||||
self.assertEqual(dag_composed, dag_expected)
|
||||
|
||||
def test_compose_raises_if_splitting_condition_creg(self):
|
||||
"""Verify compose raises if a condition is mapped to more than one creg.
|
||||
|
||||
┌───┐
|
||||
q_0: q_0: ─┤ H ├─
|
||||
└─┬─┘
|
||||
c0: 1/ + ┌──┴──┐ = DAGCircuitError
|
||||
c: 2/╡ = 2 ╞
|
||||
c1: 1/ └─────┘
|
||||
"""
|
||||
|
||||
qreg = QuantumRegister(1)
|
||||
creg1 = ClassicalRegister(1)
|
||||
creg2 = ClassicalRegister(1)
|
||||
|
||||
circuit_left = QuantumCircuit(qreg, creg1, creg2)
|
||||
|
||||
wide_creg = ClassicalRegister(2)
|
||||
|
||||
circuit_right = QuantumCircuit(qreg, wide_creg)
|
||||
circuit_right.h(0).c_if(wide_creg, 2)
|
||||
|
||||
with self.assertRaisesRegex(DAGCircuitError, "more than one creg"):
|
||||
circuit_left.compose(circuit_right)
|
||||
|
||||
def test_compose_calibrations(self):
|
||||
"""Test that compose carries over the calibrations."""
|
||||
dag_cal = QuantumCircuit(1)
|
||||
|
|
|
|||
Loading…
Reference in New Issue