From b20a7ceb58b0ec7e49004d3ce81bd3f2144e6f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elena=20Pe=C3=B1a=20Tapia?= <57907331+ElePT@users.noreply.github.com> Date: Mon, 24 Jun 2024 20:16:16 +0200 Subject: [PATCH 1/2] Add placeholders for all mising standard gates in Rust (#12646) * Add placeholders for all gates, mark TODOs * Update name for CPhase * Remove todo from Ux gates --- crates/circuit/src/imports.rs | 53 ++++++++++- crates/circuit/src/operations.rs | 156 ++++++++++++++++++++++++------- 2 files changed, 175 insertions(+), 34 deletions(-) diff --git a/crates/circuit/src/imports.rs b/crates/circuit/src/imports.rs index 7160798f56bb..76e808d1b308 100644 --- a/crates/circuit/src/imports.rs +++ b/crates/circuit/src/imports.rs @@ -79,6 +79,7 @@ pub static SINGLETON_CONTROLLED_GATE: ImportOnceCell = /// /// NOTE: the order here is significant, the StandardGate variant's number must match /// index of it's entry in this table. This is all done statically for performance +// TODO: replace placeholders with actual implementation static STDGATE_IMPORT_PATHS: [[&str; 2]; STANDARD_GATE_SIZE] = [ // ZGate = 0 ["qiskit.circuit.library.standard_gates.z", "ZGate"], @@ -131,12 +132,12 @@ static STDGATE_IMPORT_PATHS: [[&str; 2]; STANDARD_GATE_SIZE] = [ ["qiskit.circuit.library.standard_gates.sx", "SXdgGate"], // iSWAPGate = 23 ["qiskit.circuit.library.standard_gates.iswap", "iSwapGate"], - //XXMinusYYGate = 24 + // XXMinusYYGate = 24 [ "qiskit.circuit.library.standard_gates.xx_minus_yy", "XXMinusYYGate", ], - //XXPlusYYGate = 25 + // XXPlusYYGate = 25 [ "qiskit.circuit.library.standard_gates.xx_plus_yy", "XXPlusYYGate", @@ -147,6 +148,54 @@ static STDGATE_IMPORT_PATHS: [[&str; 2]; STANDARD_GATE_SIZE] = [ ["qiskit.circuit.library.standard_gates.u2", "U2Gate"], // U3Gate = 28 ["qiskit.circuit.library.standard_gates.u3", "U3Gate"], + // CRXGate = 29 + ["placeholder", "placeholder"], + // CRYGate = 30 + ["placeholder", "placeholder"], + // CRZGate = 31 + ["placeholder", "placeholder"], + // RGate 32 + ["placeholder", "placeholder"], + // CHGate = 33 + ["qiskit.circuit.library.standard_gates.h", "CHGate"], + // CPhaseGate = 34 + ["qiskit.circuit.library.standard_gates.p", "CPhaseGate"], + // CSGate = 35 + ["qiskit.circuit.library.standard_gates.s", "CSGate"], + // CSdgGate = 36 + ["qiskit.circuit.library.standard_gates.s", "CSdgGate"], + // CSXGate = 37 + ["qiskit.circuit.library.standard_gates.sx", "CSXGate"], + // CSwapGate = 38 + ["qiskit.circuit.library.standard_gates.swap", "CSwapGate"], + // CUGate = 39 + ["qiskit.circuit.library.standard_gates.u", "CUGate"], + // CU1Gate = 40 + ["qiskit.circuit.library.standard_gates.u1", "CU1Gate"], + // CU3Gate = 41 + ["qiskit.circuit.library.standard_gates.u3", "CU3Gate"], + // C3XGate = 42 + ["placeholder", "placeholder"], + // C3SXGate = 43 + ["placeholder", "placeholder"], + // C4XGate = 44 + ["placeholder", "placeholder"], + // DCXGate = 45 + ["placeholder", "placeholder"], + // CCZGate = 46 + ["placeholder", "placeholder"], + // RCCXGate = 47 + ["placeholder", "placeholder"], + // RC3XGate = 48 + ["placeholder", "placeholder"], + // RXXGate = 49 + ["placeholder", "placeholder"], + // RYYGate = 50 + ["placeholder", "placeholder"], + // RZZGate = 51 + ["placeholder", "placeholder"], + // RZXGate = 52 + ["placeholder", "placeholder"], ]; /// A mapping from the enum variant in crate::operations::StandardGate to the python object for the diff --git a/crates/circuit/src/operations.rs b/crates/circuit/src/operations.rs index 451b04947388..af7dabc86216 100644 --- a/crates/circuit/src/operations.rs +++ b/crates/circuit/src/operations.rs @@ -208,46 +208,106 @@ pub enum StandardGate { U1Gate = 26, U2Gate = 27, U3Gate = 28, + CRXGate = 29, + CRYGate = 30, + CRZGate = 31, + RGate = 32, + CHGate = 33, + CPhaseGate = 34, + CSGate = 35, + CSdgGate = 36, + CSXGate = 37, + CSwapGate = 38, + CUGate = 39, + CU1Gate = 40, + CU3Gate = 41, + C3XGate = 42, + C3SXGate = 43, + C4XGate = 44, + DCXGate = 45, + CCZGate = 46, + RCCXGate = 47, + RC3XGate = 48, + RXXGate = 49, + RYYGate = 50, + RZZGate = 51, + RZXGate = 52, } +// TODO: replace all 34s (placeholders) with actual number static STANDARD_GATE_NUM_QUBITS: [u32; STANDARD_GATE_SIZE] = [ - 1, 1, 1, 2, 2, 2, 3, 1, 1, 1, 2, 2, 1, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 1, 1, 1, + 1, 1, 1, 2, 2, 2, 3, 1, 1, 1, // 0-9 + 2, 2, 1, 0, 1, 1, 1, 1, 1, 1, // 10-19 + 1, 1, 1, 2, 2, 2, 1, 1, 1, 34, // 20-29 + 34, 34, 34, 2, 2, 2, 2, 2, 3, 2, // 30-39 + 2, 2, 34, 34, 34, 34, 34, 34, 34, 34, // 40-49 + 34, 34, 34, // 50-52 ]; +// TODO: replace all 34s (placeholders) with actual number static STANDARD_GATE_NUM_PARAMS: [u32; STANDARD_GATE_SIZE] = [ - 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1, 3, 0, 0, 0, 0, 0, 0, 2, 2, 1, 2, 3, + 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, // 0-9 + 0, 0, 0, 1, 0, 0, 1, 3, 0, 0, // 10-19 + 0, 0, 0, 0, 2, 2, 1, 2, 3, 34, // 20-29 + 34, 34, 34, 0, 1, 0, 0, 0, 0, 3, // 30-39 + 1, 3, 34, 34, 34, 34, 34, 34, 34, 34, // 40-49 + 34, 34, 34, // 50-52 ]; static STANDARD_GATE_NAME: [&str; STANDARD_GATE_SIZE] = [ - "z", - "y", - "x", - "cz", - "cy", - "cx", - "ccx", - "rx", - "ry", - "rz", - "ecr", - "swap", - "sx", - "global_phase", - "id", - "h", - "p", - "u", - "s", - "sdg", - "t", - "tdg", - "sxdg", - "iswap", - "xx_minus_yy", - "xx_plus_yy", - "u1", - "u2", - "u3", + "z", // 0 + "y", // 1 + "x", // 2 + "cz", // 3 + "cy", // 4 + "cx", // 5 + "ccx", // 6 + "rx", // 7 + "ry", // 8 + "rz", // 9 + "ecr", // 10 + "swap", // 11 + "sx", // 12 + "global_phase", // 13 + "id", // 14 + "h", // 15 + "p", // 16 + "u", // 17 + "s", // 18 + "sdg", // 19 + "t", // 20 + "tdg", // 21 + "sxdg", // 22 + "iswap", // 23 + "xx_minus_yy", // 24 + "xx_plus_yy", // 25 + "u1", // 26 + "u2", // 27 + "u3", // 28 + "crx", // 29 + "cry", // 30 + "crz", // 31 + "r", // 32 + "ch", // 33 + "cp", // 34 + "cs", // 35 + "csdg", // 36 + "csx", // 37 + "cswap", // 38 + "cu", // 39 + "cu1", // 40 + "cu3", // 41 + "c3x", // 42 + "c3sx", // 43 + "c4x", // 44 + "dcx", // 45 + "ccz", // 46 + "rccx", // 47 + "rc3x", // 48 + "rxx", // 49 + "ryy", // 50 + "rzz", // 51 + "rzx", // 52 ]; #[pymethods] @@ -296,7 +356,7 @@ impl StandardGate { // // Remove this when std::mem::variant_count() is stabilized (see // https://github.com/rust-lang/rust/issues/73662 ) -pub const STANDARD_GATE_SIZE: usize = 29; +pub const STANDARD_GATE_SIZE: usize = 53; impl Operation for StandardGate { fn name(&self) -> &str { @@ -453,6 +513,21 @@ impl Operation for StandardGate { } _ => None, }, + Self::CRXGate | Self::CRYGate | Self::CRZGate => todo!(), + Self::RGate => todo!(), + Self::CHGate => todo!(), + Self::CPhaseGate => todo!(), + Self::CSGate => todo!(), + Self::CSdgGate => todo!(), + Self::CSXGate => todo!(), + Self::CSwapGate => todo!(), + Self::CUGate | Self::CU1Gate | Self::CU3Gate => todo!(), + Self::C3XGate | Self::C3SXGate | Self::C4XGate => todo!(), + Self::DCXGate => todo!(), + Self::CCZGate => todo!(), + Self::RCCXGate | Self::RC3XGate => todo!(), + Self::RXXGate | Self::RYYGate | Self::RZZGate => todo!(), + Self::RZXGate => todo!(), } } @@ -878,6 +953,23 @@ impl Operation for StandardGate { .expect("Unexpected Qiskit python bug"), ) }), + Self::CRXGate | Self::CRYGate | Self::CRZGate => todo!(), + Self::RGate => todo!(), + Self::CHGate => todo!(), + Self::CPhaseGate => todo!(), + Self::CSGate => todo!(), + Self::CSdgGate => todo!(), + Self::CSXGate => todo!(), + Self::CSwapGate => todo!(), + Self::CUGate => todo!(), + Self::CU1Gate => todo!(), + Self::CU3Gate => todo!(), + Self::C3XGate | Self::C3SXGate | Self::C4XGate => todo!(), + Self::DCXGate => todo!(), + Self::CCZGate => todo!(), + Self::RCCXGate | Self::RC3XGate => todo!(), + Self::RXXGate | Self::RYYGate | Self::RZZGate => todo!(), + Self::RZXGate => todo!(), } } From 8b1f75ffafc70596bcf45480aa2f6d59d822e337 Mon Sep 17 00:00:00 2001 From: Jake Lishman Date: Mon, 24 Jun 2024 19:21:20 +0100 Subject: [PATCH 2/2] Deprecate tuple-like access to `CircuitInstruction` (#12640) This has been the legacy path since `CircuitInstruction` was added in gh-8093. It's more performant to use the attribute-access patterns, and with more of the internals moving to Rust and potentially needing more use of additional class methods and attributes, we need to start shifting people away from the old form. --- crates/circuit/src/circuit_instruction.rs | 39 +++++++++++++++++-- crates/circuit/src/imports.rs | 2 + ...-circuit-instruction-8a332ab09de73766.yaml | 23 +++++++++++ test/python/circuit/test_circuit_data.py | 6 ++- test/utils/base.py | 9 +++++ 5 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index 93e73ccbc42f..781a776c1566 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/crates/circuit/src/circuit_instruction.rs @@ -14,7 +14,7 @@ use std::cell::RefCell; use pyo3::basic::CompareOp; -use pyo3::exceptions::PyValueError; +use pyo3::exceptions::{PyDeprecationWarning, PyValueError}; use pyo3::prelude::*; use pyo3::types::{IntoPyDict, PyList, PyTuple, PyType}; use pyo3::{intern, IntoPy, PyObject, PyResult}; @@ -22,7 +22,7 @@ use smallvec::{smallvec, SmallVec}; use crate::imports::{ get_std_gate_class, populate_std_gate_map, GATE, INSTRUCTION, OPERATION, - SINGLETON_CONTROLLED_GATE, SINGLETON_GATE, + SINGLETON_CONTROLLED_GATE, SINGLETON_GATE, WARNINGS_WARN, }; use crate::interner::Index; use crate::operations::{OperationType, Param, PyGate, PyInstruction, PyOperation, StandardGate}; @@ -572,26 +572,31 @@ impl CircuitInstruction { #[cfg(not(feature = "cache_pygates"))] pub fn __getitem__(&self, py: Python<'_>, key: &Bound) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().get_item(key)?.into_py(py)) } #[cfg(feature = "cache_pygates")] pub fn __getitem__(&mut self, py: Python<'_>, key: &Bound) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().get_item(key)?.into_py(py)) } #[cfg(not(feature = "cache_pygates"))] pub fn __iter__(&self, py: Python<'_>) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().iter()?.into_py(py)) } #[cfg(feature = "cache_pygates")] pub fn __iter__(&mut self, py: Python<'_>) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; Ok(self._legacy_format(py)?.as_any().iter()?.into_py(py)) } - pub fn __len__(&self) -> usize { - 3 + pub fn __len__(&self, py: Python) -> PyResult { + warn_on_legacy_circuit_instruction_iteration(py)?; + Ok(3) } pub fn __richcmp__( @@ -939,3 +944,29 @@ pub(crate) fn convert_py_to_operation_type( } Err(PyValueError::new_err(format!("Invalid input: {}", py_op))) } + +/// Issue a Python `DeprecationWarning` about using the legacy tuple-like interface to +/// `CircuitInstruction`. +/// +/// Beware the `stacklevel` here doesn't work quite the same way as it does in Python as Rust-space +/// calls are completely transparent to Python. +#[inline] +fn warn_on_legacy_circuit_instruction_iteration(py: Python) -> PyResult<()> { + WARNINGS_WARN + .get_bound(py) + .call1(( + intern!( + py, + concat!( + "Treating CircuitInstruction as an iterable is deprecated legacy behavior", + " since Qiskit 1.2, and will be removed in Qiskit 2.0.", + " Instead, use the `operation`, `qubits` and `clbits` named attributes." + ) + ), + py.get_type_bound::(), + // Stack level. Compared to Python-space calls to `warn`, this is unusually low + // beacuse all our internal call structure is now Rust-space and invisible to Python. + 1, + )) + .map(|_| ()) +} diff --git a/crates/circuit/src/imports.rs b/crates/circuit/src/imports.rs index 76e808d1b308..92700f3274e7 100644 --- a/crates/circuit/src/imports.rs +++ b/crates/circuit/src/imports.rs @@ -72,6 +72,8 @@ pub static SINGLETON_GATE: ImportOnceCell = pub static SINGLETON_CONTROLLED_GATE: ImportOnceCell = ImportOnceCell::new("qiskit.circuit.singleton", "SingletonControlledGate"); +pub static WARNINGS_WARN: ImportOnceCell = ImportOnceCell::new("warnings", "warn"); + /// A mapping from the enum variant in crate::operations::StandardGate to the python /// module path and class name to import it. This is used to populate the conversion table /// when a gate is added directly via the StandardGate path and there isn't a Python object diff --git a/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml b/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml new file mode 100644 index 000000000000..d656ee5cb823 --- /dev/null +++ b/releasenotes/notes/deprecate-legacy-circuit-instruction-8a332ab09de73766.yaml @@ -0,0 +1,23 @@ +--- +deprecations_circuits: + - | + Treating :class:`.CircuitInstruction` as a tuple-like iterable is deprecated, and this legacy + path way will be removed in Qiskit 2.0. You should use the attribute-access fields + :attr:`~.CircuitInstruction.operation`, :attr:`~.CircuitInstruction.qubits`, and + :attr:`~.CircuitInstruction.clbits` instead. For example:: + + from qiskit.circuit import QuantumCircuit + + qc = QuantumCircuit(2, 2) + qc.h(0) + qc.cx(0, 1) + qc.measure([0, 1], [0, 1]) + + # Deprecated. + for op, qubits, clbits in qc.data: + pass + # New style. + for instruction in qc.data: + op = instruction.operation + qubits = instruction.qubits + clbits = instruction.clbits diff --git a/test/python/circuit/test_circuit_data.py b/test/python/circuit/test_circuit_data.py index 35ae27b2fcfb..55028c8e883e 100644 --- a/test/python/circuit/test_circuit_data.py +++ b/test/python/circuit/test_circuit_data.py @@ -416,7 +416,11 @@ def to_legacy(instruction): return (instruction.operation, list(instruction.qubits), list(instruction.clbits)) expected = [to_legacy(instruction) for instruction in qc.data] - actual = [tuple(instruction) for instruction in qc.data] + + with self.assertWarnsRegex( + DeprecationWarning, "Treating CircuitInstruction as an iterable is deprecated" + ): + actual = [tuple(instruction) for instruction in qc.data] self.assertEqual(actual, expected) def test_getitem_by_insertion_order(self): diff --git a/test/utils/base.py b/test/utils/base.py index 63a8bf4384f0..bebf03008858 100644 --- a/test/utils/base.py +++ b/test/utils/base.py @@ -215,6 +215,15 @@ def setUpClass(cls): module=r"seaborn(\..*)?", ) + # Safe to remove once https://github.com/Qiskit/qiskit-aer/pull/2179 is in a release version + # of Aer. + warnings.filterwarnings( + "default", + category=DeprecationWarning, + message="Treating CircuitInstruction as an iterable is deprecated", + module=r"qiskit_aer(\.[a-zA-Z0-9_]+)*", + ) + allow_DeprecationWarning_modules = [ "test.python.pulse.test_builder", "test.python.pulse.test_block",