Skip to content

Commit

Permalink
Fix extraction of controlled parametric gates (#13067)
Browse files Browse the repository at this point in the history
The `mutable` check in the controlled-gate `OperationFromPython`
extraction logic to check for a mutated `base_gate` was overzealous,
and would return false positives for parametric controlled gates.  The
only modification to `base_gate` of a standard-library gate that would
not have caused data-model problems from Python space would be setting
the base-gate label, which is used for a public feature of the circuit
visualisers.

The change to `get_standard_gate_name_mapping` is just a minor
convenience to make the gate objects directly appendable to a circuit;
previously, each `Parameter` object was distinct and had a UUID clash
with others of the same name, so could not be used together.  The new
behaviour is purely a convenience for tests; it largely should not be
useful for users to directly append these gates.

(cherry picked from commit 8f33084)
  • Loading branch information
jakelishman authored and mergify[bot] committed Sep 3, 2024
1 parent abe8dfb commit ead2f4a
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 28 deletions.
12 changes: 9 additions & 3 deletions crates/circuit/src/circuit_instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,18 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
// mapping to avoid an `isinstance` check on `ControlledGate` - a standard gate has
// nonzero `num_ctrl_qubits` iff it is a `ControlledGate`.
//
// `ControlledGate` also has a `base_gate` attribute, and we don't track enough in Rust
// space to handle the case that that was mutated away from a standard gate.
// `ControlledGate` also has a `base_gate` attribute related to its historical
// implementation, which technically allows mutations from Python space. The only
// mutation of a standard gate's `base_gate` that wouldn't have already broken the
// Python-space data model is setting a label, so we just catch that case and default
// back to non-standard-gate handling in that case.
if standard.num_ctrl_qubits() != 0
&& ((ob.getattr(intern!(py, "ctrl_state"))?.extract::<usize>()?
!= (1 << standard.num_ctrl_qubits()) - 1)
|| ob.getattr(intern!(py, "mutable"))?.extract()?)
|| !ob
.getattr(intern!(py, "base_gate"))?
.getattr(intern!(py, "label"))?
.is_none())
{
break 'standard;
}
Expand Down
57 changes: 32 additions & 25 deletions qiskit/circuit/library/standard_gates/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,45 +54,51 @@ def get_standard_gate_name_mapping():
from qiskit.circuit.delay import Delay
from qiskit.circuit.reset import Reset

lambda_ = Parameter("λ")
theta = Parameter("ϴ")
phi = Parameter("φ")
gamma = Parameter("γ")
beta = Parameter("β")
time = Parameter("t")

# Standard gates library mapping, multicontrolled gates not included since they're
# variable width
gates = [
IGate(),
SXGate(),
XGate(),
CXGate(),
RZGate(Parameter("λ")),
RGate(Parameter("ϴ"), Parameter("φ")),
Reset(),
RZGate(lambda_),
RGate(theta, phi),
C3SXGate(),
CCXGate(),
DCXGate(),
CHGate(),
CPhaseGate(Parameter("ϴ")),
CRXGate(Parameter("ϴ")),
CRYGate(Parameter("ϴ")),
CRZGate(Parameter("ϴ")),
CPhaseGate(theta),
CRXGate(theta),
CRYGate(theta),
CRZGate(theta),
CSwapGate(),
CSXGate(),
CUGate(Parameter("ϴ"), Parameter("φ"), Parameter("λ"), Parameter("γ")),
CU1Gate(Parameter("λ")),
CU3Gate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
CUGate(theta, phi, lambda_, gamma),
CU1Gate(lambda_),
CU3Gate(theta, phi, lambda_),
CYGate(),
CZGate(),
CCZGate(),
GlobalPhaseGate(Parameter("ϴ")),
GlobalPhaseGate(theta),
HGate(),
PhaseGate(Parameter("ϴ")),
PhaseGate(theta),
RCCXGate(),
RC3XGate(),
RXGate(Parameter("ϴ")),
RXXGate(Parameter("ϴ")),
RYGate(Parameter("ϴ")),
RYYGate(Parameter("ϴ")),
RZZGate(Parameter("ϴ")),
RZXGate(Parameter("ϴ")),
XXMinusYYGate(Parameter("ϴ"), Parameter("β")),
XXPlusYYGate(Parameter("ϴ"), Parameter("β")),
RXGate(theta),
RXXGate(theta),
RYGate(theta),
RYYGate(theta),
RZZGate(theta),
RZXGate(theta),
XXMinusYYGate(theta, beta),
XXPlusYYGate(theta, beta),
ECRGate(),
SGate(),
SdgGate(),
Expand All @@ -103,13 +109,14 @@ def get_standard_gate_name_mapping():
SXdgGate(),
TGate(),
TdgGate(),
UGate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
U1Gate(Parameter("λ")),
U2Gate(Parameter("φ"), Parameter("λ")),
U3Gate(Parameter("ϴ"), Parameter("φ"), Parameter("λ")),
UGate(theta, phi, lambda_),
U1Gate(lambda_),
U2Gate(phi, lambda_),
U3Gate(theta, phi, lambda_),
YGate(),
ZGate(),
Delay(Parameter("t")),
Delay(time),
Reset(),
Measure(),
]
name_mapping = {gate.name: gate for gate in gates}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
Parametric controlled standard-library gates (such as :class:`.CRXGate`) will now get correctly
extracted to a Rust-space standard gate when using :meth:`.QuantumCircuit.append` and the gate
object. Previously there was a discrepancy where using the :meth:`.QuantumCircuit.crx` method
would cause a correct extraction in Rust space, but the :meth:`~.QuantumCirucit.append` form
would not. The bug should generally not have caused any unsoundness from Python.
17 changes: 17 additions & 0 deletions test/python/circuit/test_rust_equivalence.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,20 @@ def test_non_default_controls(self):
circuit.append(gate, [0, 1, 2])
self.assertIsNotNone(getattr(gate, "_standard_gate", None))
np.testing.assert_almost_equal(Operator(circuit.data[0].operation).to_matrix(), op)

def test_extracted_as_standard_gate(self):
"""Test that every gate in the standard library gets correctly extracted as a Rust-space
`StandardGate` in its default configuration when passed through `append`."""
standards = set()
qc = QuantumCircuit(4)
for name, gate in get_standard_gate_name_mapping().items():
if gate._standard_gate is None:
# Not a standard gate.
continue
standards.add(name)
qc.append(gate, qc.qubits[: gate.num_qubits], [])
# Sanity check: the test should have found at least one standard gate in the mapping.
self.assertNotEqual(standards, set())

extracted = {inst.name for inst in qc.data if inst.is_standard_gate()}
self.assertEqual(standards, extracted)

0 comments on commit ead2f4a

Please sign in to comment.