Combine Python types in PackedInstructions (Qiskit#15277)

* Combine Python types in PackedInstructions

This commit migrates the 3 python variants in PackedOperation to be a
single variant. The 3 types PyGate, PyInstruction, and PyOperation are
very similar and are only separate for typing reasons. This commit moves
to only having a single struct and adding the typing information as an
enum that wraps that struct representation. This is then propogated to
the PackedInstruction type and move us from 3 separate fields in the
struct to just a single one. This frees up more space for other
operation types in the future. OperationRef keeps the distinct variants
for Gate, Instruction, and Operation. This simplifies the required
changes as most places interact with OperationRef instead of the owned
types.

The other motivation for this is as an early step towards fixing Qiskit#14240.
This commit simplifies and consolidates the places that explicitly wrap
python usage in the circuit crate. This will make it easier to
conditionally compile things when we start to do that.

The other option considered was also moving to a single struct and setting
a `py_op_type` field inside the struct that was an enum to differentiate
the Python type. However, doing this inside the struct would require
significant changes to the usage of Python types in a circuit. Instead of
matching on the type off of OperationRef it would need to be a double
mapping first on OperationRef then on `py_op_type` field. Using the
outer enum prevents this and makes it a bit easier to match in places.

* Add missing alignment on PyOperationTypes

As we insert PyOperationTypes in the PackedInstruction we need to ensure
the alignment so that there are free bits to put the discriminant of the
PackedOperation in the u64. This was overlooked in earlier versions of
this PR branch and would have caused issues on other platforms
potentially.

* Fix typo in rebase

Author: mtreinish

crates/accelerate/src/twirling.rs

@@ -12,7 +12,6 @@
 
 use std::f64::consts::PI;
 
-use crate::QiskitError;
 use hashbrown::HashMap;
 use ndarray::ArrayView2;
 use ndarray::linalg::kron;
@@ -29,6 +28,9 @@ use qiskit_circuit::instruction::Instruction;
 use qiskit_circuit::operations::StandardGate::{I, X, Y, Z};
 use qiskit_circuit::operations::{Operation, OperationRef, Param, StandardGate};
 use qiskit_circuit::packed_instruction::PackedInstruction;
+
+use crate::QiskitError;
+
 use qiskit_circuit::{BlocksMode, NoBlocks, VarsMode};
 use qiskit_transpiler::passes::run_optimize_1q_gates_decomposition;
 use qiskit_transpiler::target::Target;

crates/circuit/src/circuit_data.rs

@@ -27,7 +27,8 @@ use crate::imports::{ANNOTATED_OPERATION, QUANTUM_CIRCUIT};
 use crate::interner::{Interned, InternedMap, Interner};
 use crate::object_registry::{ObjectRegistry, ObjectRegistryError};
 use crate::operations::{
-    ControlFlow, ControlFlowView, Operation, OperationRef, Param, PythonOperation, StandardGate,
+    ControlFlow, ControlFlowView, Operation, OperationRef, Param, PyOperationTypes,
+    PythonOperation, StandardGate,
 };
 use crate::packed_instruction::{PackedInstruction, PackedOperation};
 use crate::parameter::parameter_expression::{ParameterError, ParameterExpression};
@@ -799,13 +800,16 @@ impl CircuitData {
             let memo = PyDict::new(py);
             for inst in &self.data {
                 let new_op = match inst.op.view() {
+                    OperationRef::Gate(gate) => {
+                        PyOperationTypes::Gate(gate.py_deepcopy(py, Some(&memo))?).into()
+                    }
                     OperationRef::ControlFlow(cf) => cf.clone().into(),
-                    OperationRef::Gate(gate) => gate.py_deepcopy(py, Some(&memo))?.into(),
                     OperationRef::Instruction(instruction) => {
-                        instruction.py_deepcopy(py, Some(&memo))?.into()
+                        PyOperationTypes::Instruction(instruction.py_deepcopy(py, Some(&memo))?)
+                            .into()
                     }
                     OperationRef::Operation(operation) => {
-                        operation.py_deepcopy(py, Some(&memo))?.into()
+                        PyOperationTypes::Operation(operation.py_deepcopy(py, Some(&memo))?).into()
                     }
                     OperationRef::StandardGate(gate) => gate.into(),
                     OperationRef::StandardInstruction(instruction) => instruction.into(),
@@ -825,10 +829,14 @@ impl CircuitData {
         } else if copy_instructions {
             for inst in &self.data {
                 let new_op = match inst.op.view() {
+                    OperationRef::Gate(gate) => PyOperationTypes::Gate(gate.py_copy(py)?).into(),
+                    OperationRef::Instruction(instruction) => {
+                        PyOperationTypes::Instruction(instruction.py_copy(py)?).into()
+                    }
+                    OperationRef::Operation(operation) => {
+                        PyOperationTypes::Operation(operation.py_copy(py)?).into()
+                    }
                     OperationRef::ControlFlow(cf) => cf.clone().into(),
-                    OperationRef::Gate(gate) => gate.py_copy(py)?.into(),
-                    OperationRef::Instruction(instruction) => instruction.py_copy(py)?.into(),
-                    OperationRef::Operation(operation) => operation.py_copy(py)?.into(),
                     OperationRef::StandardGate(gate) => gate.into(),
                     OperationRef::StandardInstruction(instruction) => instruction.into(),
                     OperationRef::Unitary(unitary) => unitary.clone().into(),

crates/circuit/src/circuit_instruction.rs

@@ -29,7 +29,7 @@ use crate::imports::{CONTROLLED_GATE, GATE, INSTRUCTION, OPERATION, WARNINGS_WAR
 use crate::instruction::{Instruction, Parameters, create_py_op};
 use crate::operations::{
     ArrayType, BoxDuration, ControlFlow, ControlFlowInstruction, ControlFlowType, Operation,
-    OperationRef, Param, PauliProductMeasurement, PyGate, PyInstruction, PyOperation, StandardGate,
+    OperationRef, Param, PauliProductMeasurement, PyInstruction, PyOperationTypes, StandardGate,
     StandardInstruction, StandardInstructionType, UnitaryGate,
 };
 use crate::packed_instruction::PackedOperation;
@@ -256,7 +256,7 @@ impl CircuitInstruction {
         match self.operation.view() {
             OperationRef::StandardGate(standard) => Ok(standard.num_ctrl_qubits() != 0),
             OperationRef::Gate(gate) => gate
-                .gate
+                .instruction
                 .bind(py)
                 .is_instance(CONTROLLED_GATE.get_bound(py)),
             _ => Ok(false),
@@ -868,13 +868,15 @@ impl<'a, 'py, T: CircuitBlock> FromPyObject<'a, 'py> for OperationFromPython<T>
 
         if ob_type.is_subclass(GATE.get_bound(py))? {
             let params = get_params()?;
-            let operation = PackedOperation::from_gate(Box::new(PyGate {
-                qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
-                clbits: 0,
-                params: params.len()? as u32,
-                op_name: ob.getattr(intern!(py, "name"))?.extract()?,
-                gate: ob.to_owned().unbind(),
-            }));
+            let operation = PackedOperation::from_py_operation(Box::new(PyOperationTypes::Gate(
+                PyInstruction {
+                    qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
+                    clbits: 0,
+                    params: params.len()? as u32,
+                    op_name: ob.getattr(intern!(py, "name"))?.extract()?,
+                    instruction: ob.to_owned().unbind(),
+                },
+            )));
             let params = extract_params(operation.view(), &params)?;
             return Ok(OperationFromPython {
                 operation,
@@ -884,13 +886,15 @@ impl<'a, 'py, T: CircuitBlock> FromPyObject<'a, 'py> for OperationFromPython<T>
         }
         if ob_type.is_subclass(INSTRUCTION.get_bound(py))? {
             let params = get_params()?;
-            let operation = PackedOperation::from_instruction(Box::new(PyInstruction {
-                qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
-                clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?,
-                params: params.len()? as u32,
-                op_name: ob.getattr(intern!(py, "name"))?.extract()?,
-                instruction: ob.to_owned().unbind(),
-            }));
+            let operation = PackedOperation::from_py_operation(Box::new(
+                PyOperationTypes::Instruction(PyInstruction {
+                    qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
+                    clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?,
+                    params: params.len()? as u32,
+                    op_name: ob.getattr(intern!(py, "name"))?.extract()?,
+                    instruction: ob.to_owned().unbind(),
+                }),
+            ));
             let params = extract_params(operation.view(), &params)?;
             return Ok(OperationFromPython {
                 operation,
@@ -900,13 +904,15 @@ impl<'a, 'py, T: CircuitBlock> FromPyObject<'a, 'py> for OperationFromPython<T>
         }
         if ob_type.is_subclass(OPERATION.get_bound(py))? {
             let params = get_params()?;
-            let operation = PackedOperation::from_operation(Box::new(PyOperation {
-                qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
-                clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?,
-                params: params.len()? as u32,
-                op_name: ob.getattr(intern!(py, "name"))?.extract()?,
-                operation: ob.to_owned().unbind(),
-            }));
+            let operation = PackedOperation::from_py_operation(Box::new(
+                PyOperationTypes::Operation(PyInstruction {
+                    qubits: ob.getattr(intern!(py, "num_qubits"))?.extract()?,
+                    clbits: ob.getattr(intern!(py, "num_clbits"))?.extract()?,
+                    params: params.len()? as u32,
+                    op_name: ob.getattr(intern!(py, "name"))?.extract()?,
+                    instruction: ob.to_owned().unbind(),
+                }),
+            ));
             let params = extract_params(operation.view(), &params)?;
             return Ok(OperationFromPython {
                 operation,

crates/circuit/src/converters.rs

@@ -18,7 +18,7 @@ use crate::bit::{ShareableClbit, ShareableQubit};
 use crate::circuit_data::{CircuitData, CircuitDataError, CircuitVar};
 use crate::dag_circuit::DAGIdentifierInfo;
 use crate::dag_circuit::{DAGCircuit, NodeType};
-use crate::operations::{OperationRef, PythonOperation};
+use crate::operations::{OperationRef, PyOperationTypes, PythonOperation};
 use crate::{Clbit, Qubit};
 
 /// An extractable representation of a QuantumCircuit reserved only for
@@ -134,13 +134,20 @@ pub fn dag_to_circuit(
                 let op = match instr.op.view() {
                     OperationRef::ControlFlow(cf) => cf.clone().into(),
                     OperationRef::Gate(gate) => {
-                        Python::attach(|py| gate.py_deepcopy(py, None))?.into()
+                        PyOperationTypes::Gate(Python::attach(|py| gate.py_deepcopy(py, None))?)
+                            .into()
                     }
                     OperationRef::Instruction(instruction) => {
-                        Python::attach(|py| instruction.py_deepcopy(py, None))?.into()
+                        PyOperationTypes::Instruction(Python::attach(|py| {
+                            instruction.py_deepcopy(py, None)
+                        })?)
+                        .into()
                     }
                     OperationRef::Operation(operation) => {
-                        Python::attach(|py| operation.py_deepcopy(py, None))?.into()
+                        PyOperationTypes::Operation(Python::attach(|py| {
+                            operation.py_deepcopy(py, None)
+                        })?)
+                        .into()
                     }
                     OperationRef::StandardGate(gate) => gate.into(),
                     OperationRef::StandardInstruction(instruction) => instruction.into(),

crates/circuit/src/dag_circuit.rs

@@ -33,8 +33,8 @@ use crate::interner::{Interned, InternedMap, Interner};
 use crate::object_registry::ObjectRegistry;
 use crate::operations::{
     ArrayType, BoxDuration, Condition, ControlFlow, ControlFlowInstruction, ControlFlowView,
-    Operation, OperationRef, Param, PythonOperation, StandardGate, StandardInstruction,
-    SwitchTarget,
+    Operation, OperationRef, Param, PyOperationTypes, PythonOperation, StandardGate,
+    StandardInstruction, SwitchTarget,
 };
 use crate::packed_instruction::{PackedInstruction, PackedOperation};
 use crate::parameter::parameter_expression::ParameterExpression;
@@ -2545,21 +2545,26 @@ impl DAGCircuit {
                                         && check_args())
                                 }
                                 [OperationRef::Gate(op1), OperationRef::Gate(op2)] => {
-                                    Ok(op1.gate.bind(py).eq(&op2.gate)? && check_args())
+                                    Ok(op1.instruction.bind(py).eq(&op2.instruction)?
+                                        && check_args())
                                 }
                                 [OperationRef::Operation(op1), OperationRef::Operation(op2)] => {
-                                    Ok(op1.operation.bind(py).eq(&op2.operation)? && check_args())
+                                    Ok(op1.instruction.bind(py).eq(&op2.instruction)?
+                                        && check_args())
                                 }
                                 // Handle the edge case where we end up with a Python object and a standard
                                 // gate/instruction.
                                 // This typically only happens if we have a ControlledGate in Python
                                 // and we have mutable state set.
                                 [OperationRef::StandardGate(_), OperationRef::Gate(op2)] => {
-                                    Ok(slf.unpack_py_op(py, inst1)?.bind(py).eq(&op2.gate)?
+                                    Ok(slf.unpack_py_op(py, inst1)?.bind(py).eq(&op2.instruction)?
                                         && check_args())
                                 }
                                 [OperationRef::Gate(op1), OperationRef::StandardGate(_)] => {
-                                    Ok(other.unpack_py_op(py, inst2)?.bind(py).eq(&op1.gate)?
+                                    Ok(other
+                                        .unpack_py_op(py, inst2)?
+                                        .bind(py)
+                                        .eq(&op1.instruction)?
                                         && check_args())
                                 }
                                 [
@@ -2815,13 +2820,13 @@ impl DAGCircuit {
                 ) => Ok(left == right),
                 (OperationRef::Unitary(left), OperationRef::Unitary(right)) => Ok(left == right),
                 (OperationRef::Gate(left), OperationRef::Gate(right)) => {
-                    Python::attach(|py| left.gate.bind(py).eq(&right.gate))
+                    Python::attach(|py| left.instruction.bind(py).eq(&right.instruction))
                 }
                 (OperationRef::Instruction(left), OperationRef::Instruction(right)) => {
                     Python::attach(|py| left.instruction.bind(py).eq(&right.instruction))
                 }
                 (OperationRef::Operation(left), OperationRef::Operation(right)) => {
-                    Python::attach(|py| left.operation.bind(py).eq(&right.operation))
+                    Python::attach(|py| left.instruction.bind(py).eq(&right.instruction))
                 }
                 _ => Ok(false),
             }
@@ -7858,13 +7863,20 @@ impl DAGCircuit {
                     match instr.op.view() {
                         OperationRef::ControlFlow(cf) => cf.clone().into(),
                         OperationRef::Gate(gate) => {
-                            Python::attach(|py| gate.py_deepcopy(py, None))?.into()
+                            PyOperationTypes::Gate(Python::attach(|py| gate.py_deepcopy(py, None))?)
+                                .into()
                         }
                         OperationRef::Instruction(instruction) => {
-                            Python::attach(|py| instruction.py_deepcopy(py, None))?.into()
+                            PyOperationTypes::Instruction(Python::attach(|py| {
+                                instruction.py_deepcopy(py, None)
+                            })?)
+                            .into()
                         }
                         OperationRef::Operation(operation) => {
-                            Python::attach(|py| operation.py_deepcopy(py, None))?.into()
+                            PyOperationTypes::Operation(Python::attach(|py| {
+                                operation.py_deepcopy(py, None)
+                            })?)
+                            .into()
                         }
                         OperationRef::StandardGate(gate) => gate.into(),
                         OperationRef::StandardInstruction(instruction) => instruction.into(),

crates/circuit/src/dag_node.rs

@@ -17,7 +17,7 @@ use std::sync::OnceLock;
 use crate::TupleLikeArg;
 use crate::circuit_data::CircuitData;
 use crate::circuit_instruction::{CircuitInstruction, OperationFromPython, extract_params};
-use crate::operations::{Operation, OperationRef, Param, PythonOperation};
+use crate::operations::{Operation, OperationRef, Param, PyOperationTypes, PythonOperation};
 
 use ahash::AHasher;
 use approx::relative_eq;
@@ -255,9 +255,15 @@ impl DAGOpNode {
         if deepcopy {
             instruction.operation = match instruction.operation.view() {
                 OperationRef::ControlFlow(cf) => cf.clone().into(),
-                OperationRef::Gate(gate) => gate.py_deepcopy(py, None)?.into(),
-                OperationRef::Instruction(instruction) => instruction.py_deepcopy(py, None)?.into(),
-                OperationRef::Operation(operation) => operation.py_deepcopy(py, None)?.into(),
+                OperationRef::Gate(gate) => {
+                    PyOperationTypes::Gate(gate.py_deepcopy(py, None)?).into()
+                }
+                OperationRef::Instruction(instruction) => {
+                    PyOperationTypes::Instruction(instruction.py_deepcopy(py, None)?).into()
+                }
+                OperationRef::Operation(operation) => {
+                    PyOperationTypes::Operation(operation.py_deepcopy(py, None)?).into()
+                }
                 OperationRef::StandardGate(gate) => gate.into(),
                 OperationRef::StandardInstruction(instruction) => instruction.into(),
                 OperationRef::Unitary(unitary) => unitary.clone().into(),
@@ -301,12 +307,16 @@ impl DAGOpNode {
         Ok(CircuitInstruction {
             operation: if deepcopy {
                 match self.instruction.operation.view() {
-                    OperationRef::ControlFlow(cf) => cf.clone().into(),
-                    OperationRef::Gate(gate) => gate.py_deepcopy(py, None)?.into(),
+                    OperationRef::Gate(gate) => {
+                        PyOperationTypes::Gate(gate.py_deepcopy(py, None)?).into()
+                    }
                     OperationRef::Instruction(instruction) => {
-                        instruction.py_deepcopy(py, None)?.into()
+                        PyOperationTypes::Instruction(instruction.py_deepcopy(py, None)?).into()
+                    }
+                    OperationRef::Operation(operation) => {
+                        PyOperationTypes::Operation(operation.py_deepcopy(py, None)?).into()
                     }
-                    OperationRef::Operation(operation) => operation.py_deepcopy(py, None)?.into(),
+                    OperationRef::ControlFlow(cf) => cf.clone().into(),
                     OperationRef::StandardGate(gate) => gate.into(),
                     OperationRef::StandardInstruction(instruction) => instruction.into(),
                     OperationRef::Unitary(unitary) => unitary.clone().into(),

crates/circuit/src/instruction.rs

@@ -186,9 +186,9 @@ pub fn create_py_op(
         OperationRef::StandardInstruction(instruction) => {
             instruction.create_py_op(py, params.map(|p| p.unwrap_params()), label)
         }
-        OperationRef::Gate(gate) => Ok(gate.gate.clone_ref(py)),
+        OperationRef::Gate(gate) => Ok(gate.instruction.clone_ref(py)),
         OperationRef::Instruction(instruction) => Ok(instruction.instruction.clone_ref(py)),
-        OperationRef::Operation(operation) => Ok(operation.operation.clone_ref(py)),
+        OperationRef::Operation(operation) => Ok(operation.instruction.clone_ref(py)),
         OperationRef::Unitary(unitary) => unitary.create_py_op(py, label),
     }
 }