From 947105076f790f071bb27c09059d17acb9b912c4 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Mon, 4 May 2026 15:54:19 -0400 Subject: [PATCH 1/3] DNM: #16134 Use rust-native errors in `DAGCircuit` This should be step 1 for splitting `PyDAGCircuit` from `DAGCircuit`. The following commits assure that any rust accessible method for `DAGCircuit` that: 1. Does not take a `py` token; 2. Is a public method. does not need to depend on `PyErr` to process any errors. It is ensured that these errors: - Maintain the level of detail that the previous ones did. - Correctly map to the same Python exceptions as before. The enumeration for these errors is called `DAGCircuitInnerError` (subject to change) and is implemented with `thiserror`. I chose this over `anyhow` because it is better for specifying where the error comes from based on its variant rather than the message it contains. Lint: Use names instead of underscores Add: `anyhow!` usage for `DAGCircuitError - Use `anyhow!` for any error types that don't require tracking of any variables coming from the `DAG` or that are part of the function arguments. - Consolidate the amount of `DAG` error variants down from 40+ to 21. Can be consolidated further. Fix: Existing error formatting - Use `this_error` to fix the formatting of current errors causing conflicts with their messages. - Add `is_var` argument to differentiate Variables and Stretches in `VariableNotInlined`. Fix: consolidate `RegistryAbsentObject` error. Add: errors for `size` and `depth`. --- Cargo.lock | 1 + crates/circuit/Cargo.toml | 1 + crates/circuit/src/circuit_instruction.rs | 1 + crates/circuit/src/converters.rs | 1 + crates/circuit/src/dag_circuit.rs | 734 +++++++++++------- crates/transpiler/src/passes/sabre/route.rs | 4 +- .../src/passes/unroll_3q_or_more.rs | 5 +- 7 files changed, 443 insertions(+), 304 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 16faf816f562..5c996b0a38f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2313,6 +2313,7 @@ name = "qiskit-circuit" version = "2.5.0-dev" dependencies = [ "ahash 0.8.12", + "anyhow", "approx", "bitfield-struct", "bytemuck", diff --git a/crates/circuit/Cargo.toml b/crates/circuit/Cargo.toml index ae3e5d85b2fc..dd9a6309a2be 100644 --- a/crates/circuit/Cargo.toml +++ b/crates/circuit/Cargo.toml @@ -36,6 +36,7 @@ ahash = "0.8.12" unicode-segmentation = "1.13" lexical-core = "1.0.6" lexical-write-float = "1.0.6" +anyhow.workspace = true [dependencies.pyo3] workspace = true diff --git a/crates/circuit/src/circuit_instruction.rs b/crates/circuit/src/circuit_instruction.rs index 36b75476f58f..d834dea4c4ea 100644 --- a/crates/circuit/src/circuit_instruction.rs +++ b/crates/circuit/src/circuit_instruction.rs @@ -595,6 +595,7 @@ impl CircuitBlock for CircuitData { impl CircuitBlock for DAGCircuit { fn extract_py_block(ob: Bound) -> PyResult { Self::from_circuit_data(&ob.borrow().inner, false, None, None, None, None) + .map_err(Into::into) } } impl CircuitBlock for NoBlocks { diff --git a/crates/circuit/src/converters.rs b/crates/circuit/src/converters.rs index 0b853c7ffb08..30086ca50839 100644 --- a/crates/circuit/src/converters.rs +++ b/crates/circuit/src/converters.rs @@ -102,6 +102,7 @@ pub fn circuit_to_dag( qubit_indices, clbit_indices, ) + .map_err(Into::into) } /// Convert a :class:`.DAGCircuit` to a :class:`.PyCircuitData`. diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index ff093d873d4f..1546499594a5 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -11,9 +11,11 @@ // that they have been altered from the originals. use std::cmp::Ordering; +use std::fmt::Debug; use std::hash::Hash; use std::sync::Arc; +use anyhow::anyhow; use foldhash::fast::RandomState; use smallvec::SmallVec; @@ -38,8 +40,10 @@ use crate::operations::{ }; use crate::packed_instruction::{PackedInstruction, PackedOperation}; use crate::parameter::parameter_expression::ParameterExpression; -use crate::register_data::RegisterData; -use crate::var_stretch_container::{StretchType, VarStretchContainer, VarType}; +use crate::register_data::{RegisterAlreadyExists, RegisterData}; +use crate::var_stretch_container::{ + StretchType, VarStretchContainer, VarStretchContainerError, VarType, +}; use crate::variable_mapper::VariableMapper; use crate::{ Block, BlockMapper, BlocksMode, Clbit, ControlFlowBlocks, Qubit, Stretch, TupleLikeArg, Var, @@ -101,6 +105,110 @@ static SEMANTIC_EQ_SYMMETRIC: [&str; 4] = ["barrier", "swap", "break_loop", "con pub use rustworkx_core::petgraph::stable_graph::NodeIndex; +/// Possible errors that can be received from any operation on the +/// [`DAGCircuit`] +#[derive(Debug, thiserror::Error)] +pub enum DAGCircuitInnerError { + #[error("qubits not idle: {0:?}")] + QubitsNotIdle(Vec), + #[error("clbits not idle: {0:?}")] + ClbitsNotIdle(Vec), + #[error("Wire {0:?} not found in circuit")] + WireNotInCircuit(Wire), + #[error("Duplicate wire {0:?}")] + DuplicateWire(Wire), + #[error( + "{ty} not in circuit. {0}", + ty = match .0 { + RegisterRef::Quantum(_) => "qreg", + RegisterRef::Classical(_) => "creg", + } + )] + RegisterNotInCircuit(RegisterRef), + #[error(transparent)] + WireOutOfRange(anyhow::Error), + #[error( + "Invalid parameter type for global phase, only float and parameter expression are supported" + )] + ObjGlobalPhase, + #[error(transparent)] + RegistryAddError(#[from] crate::object_registry::AddError), + #[error(transparent)] + RegistryAbsentObject(crate::object_registry::AbsentObject), + #[error( + "{ty} '{name}' to be inlined is not in the base DAG. If you wanted it to be automatically added, use `inline_captures=False`.", + ty = (if *is_var {"Variable"} else {"Stretch"}) + )] + VariableNotInlined { name: String, is_var: bool }, + #[error("Wire '{0:?}' not in self")] + MissingWire(ConcreteWire), + #[error("Specified node {} is not in this graph", .0.index())] + NodeNotInGraph(NodeIndex), + /// Errors that don't have anything that needs to be tracked. Only a message. + #[error(transparent)] + General(#[from] anyhow::Error), + #[error(transparent)] + VarStretchContainer(#[from] VarStretchContainerError), + #[error("Wire {0:?} not found in output map")] + WireNotInOutput(ConcreteWire), + #[error("No register with '{0:?}' to map this expression onto.")] + RejectNewRegister(Vec), + #[error(transparent)] + RegisterData(#[from] RegisterAlreadyExists), + #[error(transparent)] + Circuit(#[from] crate::circuit_data::CircuitDataError), + #[error(transparent)] + Infallible(#[from] Infallible), + // For special Python cases. + #[error(transparent)] + Python(PyErr), +} + +impl From> for DAGCircuitInnerError { + fn from(val: crate::object_registry::AbsentObject) -> Self { + Self::RegistryAbsentObject(val.erase_type()) + } +} + +impl From for PyErr { + fn from(value: DAGCircuitInnerError) -> Self { + match value { + DAGCircuitInnerError::RegistryAddError(err) => err.into(), + DAGCircuitInnerError::RegistryAbsentObject(err) => err.into(), + DAGCircuitInnerError::Python(err) => err, + DAGCircuitInnerError::WireOutOfRange(_) => PyValueError::new_err(value.to_string()), + DAGCircuitInnerError::NodeNotInGraph(_) => PyIndexError::new_err(value.to_string()), + DAGCircuitInnerError::ObjGlobalPhase => PyTypeError::new_err(value.to_string()), + DAGCircuitInnerError::Circuit(err) => err.into(), + _ => DAGCircuitError::new_err(value.to_string()), + } + } +} + +/// Enumeration to capture concrete wires +#[derive(Debug)] +pub enum ConcreteWire { + Qubit(ShareableQubit), + Clbit(ShareableClbit), + Var(Var), +} + +/// Enumeration to capture all possible registers. +#[derive(Debug)] +pub enum RegisterRef { + Quantum(QuantumRegister), + Classical(ClassicalRegister), +} + +impl std::fmt::Display for RegisterRef { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RegisterRef::Quantum(quantum_register) => quantum_register.fmt(f), + RegisterRef::Classical(classical_register) => classical_register.fmt(f), + } + } +} + #[derive(Clone, Debug)] pub enum NodeType { QubitIn(Qubit), @@ -267,11 +375,10 @@ fn condition_resources(condition: &Bound) -> PyResult }) } -fn reject_new_register(reg: &ClassicalRegister) -> PyResult<()> { - Err(DAGCircuitError::new_err(format!( - "No register with '{:?}' to map this expression onto.", - reg.bits().collect_vec() - ))) +fn reject_new_register(reg: &ClassicalRegister) -> Result<(), DAGCircuitInnerError> { + Err(DAGCircuitInnerError::RejectNewRegister( + reg.bits().collect_vec(), + )) } #[pyclass( @@ -914,7 +1021,7 @@ impl DAGCircuit { /// Args: /// angle (float, :class:`.ParameterExpression`): The phase angle. #[setter(global_phase)] - pub fn set_global_phase(&mut self, angle: Param) -> PyResult<()> { + pub fn set_global_phase(&mut self, angle: Param) -> Result<(), DAGCircuitInnerError> { self.set_global_phase_param(angle).map(|_| ()) } @@ -966,10 +1073,8 @@ impl DAGCircuit { } /// Add all wires in a quantum register. - pub fn add_qreg(&mut self, qreg: QuantumRegister) -> PyResult<()> { - self.qregs - .add_register(qreg.clone(), true) - .map_err(|_| DAGCircuitError::new_err(format!("duplicate register {}", qreg.name())))?; + pub fn add_qreg(&mut self, qreg: QuantumRegister) -> Result<(), DAGCircuitInnerError> { + self.qregs.add_register(qreg.clone(), true)?; for (index, bit) in qreg.bits().enumerate() { if self.qubits.find(&bit).is_none() { @@ -983,10 +1088,8 @@ impl DAGCircuit { } /// Add all wires in a classical register. - pub fn add_creg(&mut self, creg: ClassicalRegister) -> PyResult<()> { - self.cregs - .add_register(creg.clone(), true) - .map_err(|_| DAGCircuitError::new_err(format!("duplicate register {}", creg.name())))?; + pub fn add_creg(&mut self, creg: ClassicalRegister) -> Result<(), DAGCircuitInnerError> { + self.cregs.add_register(creg.clone(), true)?; for (index, bit) in creg.bits().enumerate() { if self.clbits.find(&bit).is_none() { @@ -1073,7 +1176,7 @@ impl DAGCircuit { ))); } }; - self.remove_clbits(bit_iter) + self.remove_clbits(bit_iter).map_err(Into::into) } /// Remove classical registers from the circuit, leaving underlying bits @@ -1084,7 +1187,7 @@ impl DAGCircuit { /// the circuit. #[pyo3(name = "remove_cregs", signature = (*cregs))] fn py_remove_cregs(&mut self, cregs: Vec) -> PyResult<()> { - self.remove_cregs(cregs) + self.remove_cregs(cregs).map_err(Into::into) } /// Remove quantum bits from the circuit. All bits MUST be idle. @@ -1111,7 +1214,7 @@ impl DAGCircuit { ))); } }; - self.remove_qubits(bit_iter) + self.remove_qubits(bit_iter).map_err(Into::into) } /// Remove quantum registers from the circuit, leaving underlying bits @@ -1206,6 +1309,7 @@ impl DAGCircuit { #[pyo3(name = "copy_empty_like", signature = (*, vars_mode=VarsMode::Alike))] pub fn py_copy_empty_like(&self, vars_mode: VarsMode) -> PyResult { self.copy_empty_like_with_capacity(0, 0, vars_mode, BlocksMode::Drop) + .map_err(Into::into) } /// Put ``self`` into the canonical physical form, with the given number of qubits. @@ -1626,17 +1730,18 @@ impl DAGCircuit { /// DAGCircuitError: if an unknown :class:`.ControlFlowOp` is present in a call with /// ``recurse=True``, or any control flow is present in a non-recursive call. #[pyo3(signature= (*, recurse=false))] - pub fn size(&self, recurse: bool) -> PyResult { + pub fn size(&self, recurse: bool) -> Result { let mut length = self.num_ops(); if !self.has_control_flow() { return Ok(length); } if !recurse { - return Err(DAGCircuitError::new_err(concat!( + return Err(anyhow!(concat!( "Size with control flow is ambiguous.", " You may use `recurse=True` to get a result", " but see this method's documentation for the meaning of this." - ))); + )) + .into()); } // Handle recursively. @@ -1683,7 +1788,7 @@ impl DAGCircuit { /// DAGCircuitError: if unknown control flow is present in a recursive call, or any control /// flow is present in a non-recursive call. #[pyo3(signature= (*, recurse=false))] - pub fn depth(&self, recurse: bool) -> PyResult { + pub fn depth(&self, recurse: bool) -> Result { if self.qubits.is_empty() && self.clbits.is_empty() && self.num_vars() == 0 { return Ok(0); } @@ -1691,15 +1796,16 @@ impl DAGCircuit { let weight_fn = |_| -> Result { Ok(1) }; return match rustworkx_core::dag_algo::longest_path(&self.dag, weight_fn).unwrap() { Some(res) => Ok(res.1 - 1), - None => Err(DAGCircuitError::new_err("not a DAG")), + None => Err(anyhow!("not a DAG").into()), }; } if !recurse { - return Err(DAGCircuitError::new_err(concat!( + return Err(anyhow!(concat!( "Depth with control flow is ambiguous.", " You may use `recurse=True` to get a result", " but see this method's documentation for the meaning of this." - ))); + )) + .into()); } // Handle recursively. let mut node_lookup: HashMap = HashMap::new(); @@ -1729,7 +1835,7 @@ impl DAGCircuit { }; match rustworkx_core::dag_algo::longest_path(&self.dag, weight_fn).unwrap() { Some(res) => Ok(res.1 - 1), - None => Err(DAGCircuitError::new_err("not a DAG")), + None => Err(anyhow!("not a DAG").into()), } } @@ -2900,7 +3006,7 @@ impl DAGCircuit { let cargs_set: HashSet<&ShareableClbit> = HashSet::from_iter(cargs_list.iter().cloned()); if self.may_have_additional_wires(&node) { - let (add_cargs, _add_vars) = Python::attach(|py| self.additional_wires(py, &node))?; + let (add_cargs, _add_vars) = self.additional_wires(&node)?; for wire in add_cargs { let clbit = self.clbits.get(wire).unwrap(); if !cargs_set.contains(clbit) { @@ -3014,7 +3120,7 @@ impl DAGCircuit { input_dag.vars_stretches.vars().objects().iter().collect(); let node_vars = if self.may_have_additional_wires(&node) { - let (_additional_clbits, additional_vars) = self.additional_wires(py, &node)?; + let (_additional_clbits, additional_vars) = self.additional_wires(&node)?; let var_set: HashSet<&expr::Var> = additional_vars .into_iter() .map(|v| self.vars_stretches.vars().get(v).unwrap()) @@ -4414,6 +4520,7 @@ impl DAGCircuit { #[pyo3(name = "add_captured_stretch")] fn py_add_captured_stretch(&mut self, stretch: expr::Stretch) -> PyResult<()> { self.add_stretch(stretch, StretchType::Capture) + .map_err(Into::into) } /// Add a declared local variable to the circuit. @@ -4433,6 +4540,7 @@ impl DAGCircuit { #[pyo3(name = "add_declared_stretch")] fn py_add_declared_stretch(&mut self, stretch: expr::Stretch) -> PyResult<()> { self.add_stretch(stretch, StretchType::Declare) + .map_err(Into::into) } /// Total number of classical variables tracked by the circuit. @@ -4815,7 +4923,7 @@ impl DAGCircuit { fn extract_blocks_from_circuit_parameters( &mut self, params: Option<&Parameters>, - ) -> PyResult>>> { + ) -> Result>>, DAGCircuitInnerError> { params .map(|params| { params @@ -4836,7 +4944,7 @@ impl DAGCircuit { fn unpack_blocks_to_circuit_parameters( &self, params: Option<&Parameters>, - ) -> PyResult>> { + ) -> Result>, DAGCircuitInnerError> { Ok(params .map(|params| { params.try_map_blocks_ref(|block| CircuitData::from_dag_ref(&self.blocks[*block])) @@ -4927,7 +5035,11 @@ impl DAGCircuit { /// `Interned<[Qubit]>` and `Interned<[Clbit]>` keys from `self` are valid in the output DAG. /// /// This does not include any pre-allocated capacity. - pub fn copy_empty_like(&self, vars_mode: VarsMode, blocks_mode: BlocksMode) -> PyResult { + pub fn copy_empty_like( + &self, + vars_mode: VarsMode, + blocks_mode: BlocksMode, + ) -> Result { self.copy_empty_like_with_capacity(0, 0, vars_mode, blocks_mode) } @@ -4940,7 +5052,7 @@ impl DAGCircuit { &self, vars_mode: VarsMode, blocks_mode: BlocksMode, - ) -> PyResult { + ) -> Result { self.copy_empty_like_with_capacity( self.dag.node_count().saturating_sub(2 * self.width()), self.dag.edge_count(), @@ -4960,7 +5072,7 @@ impl DAGCircuit { num_edges: usize, vars_mode: VarsMode, blocks_mode: BlocksMode, - ) -> PyResult { + ) -> Result { let mut out = self.qubitless_empty_like_with_capacity( self.num_qubits(), num_ops, @@ -4997,7 +5109,7 @@ impl DAGCircuit { num_ops: usize, num_edges: usize, blocks_mode: BlocksMode, - ) -> PyResult { + ) -> Result { let mut out = self.qubitless_empty_like_with_capacity( num_qubits, num_ops, @@ -5032,7 +5144,7 @@ impl DAGCircuit { num_edges: usize, vars_mode: VarsMode, blocks_mode: BlocksMode, - ) -> PyResult { + ) -> Result { let (num_vars, num_stretches) = match vars_mode { VarsMode::Drop => (0, 0), _ => (self.num_vars(), self.num_stretches()), @@ -5244,20 +5356,21 @@ impl DAGCircuit { self.vars_stretches.iter_stretches(StretchType::Declare) } - pub fn remove_qubits>(&mut self, qubits: T) -> PyResult<()> { + pub fn remove_qubits>( + &mut self, + qubits: T, + ) -> Result<(), DAGCircuitInnerError> { let qubits: HashSet = qubits.into_iter().collect(); let mut busy_bits = Vec::new(); for bit in qubits.iter() { if !self.is_wire_idle(Wire::Qubit(*bit)) { - busy_bits.push(self.qubits.get(*bit).unwrap()); + busy_bits.push(*bit); } } if !busy_bits.is_empty() { - return Err(DAGCircuitError::new_err(format!( - "qubits not idle: {busy_bits:?}" - ))); + return Err(DAGCircuitInnerError::QubitsNotIdle(busy_bits)); } // Remove any references to bits. @@ -5340,21 +5453,24 @@ impl DAGCircuit { } /// Remove the specified quantum registers - fn remove_qregs>(&mut self, qregs: T) -> PyResult<()> { + fn remove_qregs>( + &mut self, + qregs: T, + ) -> Result<(), DAGCircuitInnerError> { // let self_bound_cregs = self.cregs.bind(py); let mut valid_regs: Vec = Vec::new(); for qregs in qregs.into_iter() { if let Some(reg) = self.qregs.get(qregs.name()) { if reg != &qregs { - return Err(DAGCircuitError::new_err(format!( - "creg not in circuit: {reg:?}" - ))); + return Err(DAGCircuitInnerError::RegisterNotInCircuit( + RegisterRef::Quantum(qregs), + )); } valid_regs.push(qregs); } else { - return Err(DAGCircuitError::new_err(format!( - "creg not in circuit: {qregs:?}" - ))); + return Err(DAGCircuitInnerError::RegisterNotInCircuit( + RegisterRef::Quantum(qregs), + )); } } @@ -5373,19 +5489,20 @@ impl DAGCircuit { /// Remove the given clbits in the circuit /// /// This will reorder all the bits in the circuit. - pub fn remove_clbits>(&mut self, clbits: T) -> PyResult<()> { + pub fn remove_clbits>( + &mut self, + clbits: T, + ) -> Result<(), DAGCircuitInnerError> { let clbits: HashSet = clbits.into_iter().collect(); let mut busy_bits = Vec::new(); for bit in clbits.iter() { if !self.is_wire_idle(Wire::Clbit(*bit)) { - busy_bits.push(self.clbits.get(*bit).unwrap()); + busy_bits.push(*bit); } } if !busy_bits.is_empty() { - return Err(DAGCircuitError::new_err(format!( - "clbits not idle: {busy_bits:?}" - ))); + return Err(DAGCircuitInnerError::ClbitsNotIdle(busy_bits)); } // Remove any references to bits. @@ -5476,20 +5593,20 @@ impl DAGCircuit { pub fn remove_cregs>( &mut self, cregs: T, - ) -> PyResult<()> { + ) -> Result<(), DAGCircuitInnerError> { let mut valid_regs: Vec = Vec::new(); for creg in cregs { if let Some(reg) = self.cregs.get(creg.name()) { if reg != &creg { - return Err(DAGCircuitError::new_err(format!( - "creg not in circuit: {reg:?}" - ))); + return Err(DAGCircuitInnerError::RegisterNotInCircuit( + RegisterRef::Classical(creg), + )); } valid_regs.push(creg); } else { - return Err(DAGCircuitError::new_err(format!( - "creg not in circuit: {creg:?}" - ))); + return Err(DAGCircuitInnerError::RegisterNotInCircuit( + RegisterRef::Classical(creg), + )); } } @@ -5778,7 +5895,10 @@ impl DAGCircuit { /// `clbits` and any `Block`s) to have been added to the DAG. The function may panic, or other /// produce an invalid data structure if not. #[inline] - pub fn push_back(&mut self, instr: PackedInstruction) -> PyResult { + pub fn push_back( + &mut self, + instr: PackedInstruction, + ) -> Result { self.push_external(instr, Direction::Outgoing) } @@ -5788,7 +5908,10 @@ impl DAGCircuit { /// `clbits` and any `Block`s) to have been added to the DAG. The function may panic, or other /// produce an invalid data structure if not. #[inline] - pub fn push_front(&mut self, instr: PackedInstruction) -> PyResult { + pub fn push_front( + &mut self, + instr: PackedInstruction, + ) -> Result { self.push_external(instr, Direction::Incoming) } @@ -5798,7 +5921,11 @@ impl DAGCircuit { /// /// Explicitly, [Direction::Outgoing] is equivalent to [push_back], whereas /// [Direction::Incoming] is equivalent to [push_front]. - fn push_external(&mut self, instr: PackedInstruction, dir: Direction) -> PyResult { + fn push_external( + &mut self, + instr: PackedInstruction, + dir: Direction, + ) -> Result { self.track_instruction(&instr); let (all_cbits, vars) = self.get_classical_resources(&instr)?; let qubits_id = instr.qubits; @@ -5862,13 +5989,12 @@ impl DAGCircuit { fn get_classical_resources( &self, instr: &PackedInstruction, - ) -> PyResult<(Vec, Option>)> { + ) -> Result<(Vec, Option>), DAGCircuitInnerError> { let (all_clbits, vars): (Vec, Option>) = { if self.may_have_additional_wires(instr) { let mut clbits: IndexSet = IndexSet::from_iter(self.cargs_interner.get(instr.clbits).iter().copied()); - let (additional_clbits, additional_vars) = - Python::attach(|py| self.additional_wires(py, instr))?; + let (additional_clbits, additional_vars) = self.additional_wires(instr)?; for clbit in additional_clbits { clbits.insert(clbit); } @@ -5889,7 +6015,7 @@ impl DAGCircuit { params: Option>, label: Option, #[cfg(feature = "cache_pygates")] py_op: Option>, - ) -> PyResult { + ) -> Result { self.inner_apply_op( op, qargs, @@ -5911,7 +6037,7 @@ impl DAGCircuit { params: Option>, label: Option, #[cfg(feature = "cache_pygates")] py_op: Option>, - ) -> PyResult { + ) -> Result { self.inner_apply_op( op, qargs, @@ -5935,11 +6061,11 @@ impl DAGCircuit { label: Option, #[cfg(feature = "cache_pygates")] py_op: Option>, front: bool, - ) -> PyResult { + ) -> Result { // Check that all qargs are within an acceptable range qargs.iter().try_for_each(|qarg| { if qarg.index() >= self.num_qubits() { - return Err(PyValueError::new_err(format!( + return Err(DAGCircuitInnerError::WireOutOfRange(anyhow!( "Qubit index {} is out of range. This DAGCircuit currently has only {} qubits.", qarg.0, self.num_qubits() @@ -5951,7 +6077,7 @@ impl DAGCircuit { // Check that all cargs are within an acceptable range cargs.iter().try_for_each(|carg| { if carg.index() >= self.num_clbits() { - return Err(PyValueError::new_err(format!( + return Err(DAGCircuitInnerError::WireOutOfRange(anyhow!( "Clbit index {} is out of range. This DAGCircuit currently has only {} clbits.", carg.0, self.num_clbits() @@ -6078,29 +6204,29 @@ impl DAGCircuit { fn additional_wires( &self, - py: Python, instr: &PackedInstruction, - ) -> PyResult<(Vec, Vec)> { - let wires_from_expr = |node: &expr::Expr| -> PyResult<(Vec, Vec)> { - let mut clbits = Vec::new(); - let mut vars: Vec = Vec::new(); - for var in node.vars() { - match var { - expr::Var::Bit { bit } => { - clbits.push(self.clbits.find(bit).unwrap()); - } - expr::Var::Register { register, .. } => { - for bit in register.bits() { - clbits.push(self.clbits.find(&bit).unwrap()); + ) -> Result<(Vec, Vec), DAGCircuitInnerError> { + let wires_from_expr = + |node: &expr::Expr| -> Result<(Vec, Vec), DAGCircuitInnerError> { + let mut clbits = Vec::new(); + let mut vars: Vec = Vec::new(); + for var in node.vars() { + match var { + expr::Var::Bit { bit } => { + clbits.push(self.clbits.find(bit).unwrap()); + } + expr::Var::Register { register, .. } => { + for bit in register.bits() { + clbits.push(self.clbits.find(&bit).unwrap()); + } + } + expr::Var::Standalone { .. } => { + vars.push(self.vars_stretches.vars().find(var).unwrap()) } - } - expr::Var::Standalone { .. } => { - vars.push(self.vars_stretches.vars().find(var).unwrap()) } } - } - Ok((clbits, vars)) - }; + Ok((clbits, vars)) + }; let mut clbits = Vec::new(); let mut vars = Vec::new(); @@ -6159,23 +6285,30 @@ impl DAGCircuit { } } } else if let OperationRef::Instruction(instr) = instr.op.view() { - let op = instr.instruction.bind(py); - if op.is_instance(imports::STORE_OP.get_bound(py))? { - let (expr_clbits, expr_vars) = wires_from_expr(&op.getattr("lvalue")?.extract()?)?; - for bit in expr_clbits { - clbits.push(bit); - } - for var in expr_vars { - vars.push(var); - } - let (expr_clbits, expr_vars) = wires_from_expr(&op.getattr("rvalue")?.extract()?)?; - for bit in expr_clbits { - clbits.push(bit); - } - for var in expr_vars { - vars.push(var); + // TODO: Remove this case or bring StoreOp to Rust. + Python::attach(|py| -> PyResult<()> { + let op = instr.instruction.bind(py); + if op.is_instance(imports::STORE_OP.get_bound(py))? { + let (expr_clbits, expr_vars) = + wires_from_expr(&op.getattr("lvalue")?.extract()?)?; + for bit in expr_clbits { + clbits.push(bit); + } + for var in expr_vars { + vars.push(var); + } + let (expr_clbits, expr_vars) = + wires_from_expr(&op.getattr("rvalue")?.extract()?)?; + for bit in expr_clbits { + clbits.push(bit); + } + for var in expr_vars { + vars.push(var); + } } - } + Ok(()) + }) + .map_err(DAGCircuitInnerError::Python)? } Ok((clbits, vars)) } @@ -6192,11 +6325,11 @@ impl DAGCircuit { /// /// Raises: /// DAGCircuitError: if trying to add duplicate wire - fn add_wire(&mut self, wire: Wire) -> PyResult<(NodeIndex, NodeIndex)> { + fn add_wire(&mut self, wire: Wire) -> Result<(NodeIndex, NodeIndex), DAGCircuitInnerError> { let (in_node, out_node) = match wire { Wire::Qubit(qubit) => { if qubit.index() < self.qubit_io_map.len() { - return Err(DAGCircuitError::new_err("qubit wire already exists!")); + return Err(anyhow!("Wire {qubit:?} already exists in circuit").into()); } let in_node = self.dag.add_node(NodeType::QubitIn(qubit)); let out_node = self.dag.add_node(NodeType::QubitOut(qubit)); @@ -6205,7 +6338,7 @@ impl DAGCircuit { } Wire::Clbit(clbit) => { if clbit.index() < self.clbit_io_map.len() { - return Err(DAGCircuitError::new_err("classical wire already exists!")); + return Err(anyhow!("Wire {clbit:?} already exists in circuit").into()); } let in_node = self.dag.add_node(NodeType::ClbitIn(clbit)); let out_node = self.dag.add_node(NodeType::ClbitOut(clbit)); @@ -6214,7 +6347,7 @@ impl DAGCircuit { } Wire::Var(var) => { if var.index() < self.var_io_map.len() { - return Err(DAGCircuitError::new_err("var wire already exists!")); + return Err(anyhow!("Wire {var:?} already exists in circuit").into()); } let in_node = self.dag.add_node(NodeType::VarIn(var)); let out_node = self.dag.add_node(NodeType::VarOut(var)); @@ -6241,14 +6374,14 @@ impl DAGCircuit { /// /// Unlike the general [set_global_phase_param], this is infallible. #[inline] - pub fn set_global_phase_param(&mut self, angle: Param) -> PyResult { + pub fn set_global_phase_param(&mut self, angle: Param) -> Result { match angle { Param::Float(angle) => Ok(self.set_global_phase_f64(angle)), Param::ParameterExpression(angle) => Ok(std::mem::replace( &mut self.global_phase, Param::ParameterExpression(angle), )), - Param::Obj(_) => Err(PyTypeError::new_err("Invalid type for global phase")), + Param::Obj(_) => Err(DAGCircuitInnerError::ObjGlobalPhase), } } @@ -6278,7 +6411,10 @@ impl DAGCircuit { self.dag.remove_node(out_node); } - pub fn add_qubit_unchecked(&mut self, bit: ShareableQubit) -> PyResult { + pub fn add_qubit_unchecked( + &mut self, + bit: ShareableQubit, + ) -> Result { let qubit = self.qubits.add_unique_within_capacity(bit.clone()); self.qubit_locations .insert(bit, BitLocations::new((self.qubits.len() - 1) as u32, [])); @@ -6286,7 +6422,10 @@ impl DAGCircuit { Ok(qubit) } - pub fn add_clbit_unchecked(&mut self, bit: ShareableClbit) -> PyResult { + pub fn add_clbit_unchecked( + &mut self, + bit: ShareableClbit, + ) -> Result { let clbit = self.clbits.add_unique_within_capacity(bit.clone()); self.clbit_locations .insert(bit, BitLocations::new((self.clbits.len() - 1) as u32, [])); @@ -6597,17 +6736,14 @@ impl DAGCircuit { clbit_map: Option<&HashMap>, var_map: Option<&HashMap>, block_map: Option<&HashMap>, - ) -> PyResult> { + ) -> Result, DAGCircuitInnerError> { if self.dag.node_weight(node_index).is_none() { - return Err(PyIndexError::new_err(format!( - "Specified node {} is not in this graph", - node_index.index() - ))); + return Err(DAGCircuitInnerError::NodeNotInGraph(node_index)); } let node = match &self.dag[node_index] { NodeType::Operation(op) => op.clone(), - _ => return Err(DAGCircuitError::new_err("expected node")), + _ => return Err(anyhow!("Expected op-node for node {node_index:?}").into()), }; let qubit_map = match qubit_map { @@ -6616,11 +6752,12 @@ impl DAGCircuit { let node_qubits = self.qargs_interner.get(node.qubits); let other_qubits = (0..other.num_qubits()).map(Qubit::new); if node_qubits.len() != other_qubits.len() { - return Err(DAGCircuitError::new_err(format!( + return Err(anyhow!( "Replacement DAG has {} qubits, expected {}", other_qubits.len(), node_qubits.len() - ))); + ) + .into()); } &HashMap::::from_iter(other_qubits.zip(node_qubits.iter().copied())) } @@ -6632,11 +6769,12 @@ impl DAGCircuit { let node_clbits = self.cargs_interner.get(node.clbits); let other_clbits = (0..other.num_clbits()).map(Clbit::new); if node_clbits.len() != other_clbits.len() { - return Err(DAGCircuitError::new_err(format!( + return Err(anyhow!( "Replacement DAG has {} clbits, expected {}", other_clbits.len(), node_clbits.len() - ))); + ) + .into()); } &HashMap::::from_iter(other_clbits.zip(node_clbits.iter().copied())) } @@ -6766,12 +6904,9 @@ impl DAGCircuit { clbit_map: &HashMap, var_map: &HashMap, block_map: &HashMap, - ) -> PyResult> { + ) -> Result, DAGCircuitInnerError> { if self.dag.node_weight(node).is_none() { - return Err(PyIndexError::new_err(format!( - "Specified node {} is not in this graph", - node.index() - ))); + return Err(DAGCircuitInnerError::NodeNotInGraph(node)); } // Add wire from pred to succ if no ops on mapped wire on ``other`` @@ -7043,15 +7178,12 @@ impl DAGCircuit { /// # Returns: /// /// The [Var] index of the variable in the DAGCircuit. - fn add_var(&mut self, var: expr::Var, var_type: VarType) -> PyResult { + fn add_var(&mut self, var: expr::Var, var_type: VarType) -> Result { // The setup of the initial graph structure between an "in" and an "out" node is the same as // the bit-related `_add_wire`, but this logically needs to do different bookkeeping around // tracking the properties - let var_idx = self - .vars_stretches - .add_var(var, var_type) - .map_err(|e| DAGCircuitError::new_err(e.to_string()))?; + let var_idx = self.vars_stretches.add_var(var, var_type)?; self.add_wire(Wire::Var(var_idx))?; Ok(var_idx) } @@ -7066,47 +7198,44 @@ impl DAGCircuit { /// # Returns: /// /// The [Stretch] index of the stretch in the DAGCircuit. - fn add_stretch(&mut self, stretch: expr::Stretch, stretch_type: StretchType) -> PyResult<()> { - self.vars_stretches - .add_stretch(stretch, stretch_type) - .map_err(|e| DAGCircuitError::new_err(e.to_string()))?; + fn add_stretch( + &mut self, + stretch: expr::Stretch, + stretch_type: StretchType, + ) -> Result<(), DAGCircuitInnerError> { + self.vars_stretches.add_stretch(stretch, stretch_type)?; Ok(()) } - fn check_op_addition(&self, inst: &PackedInstruction) -> PyResult<()> { + fn check_op_addition(&self, inst: &PackedInstruction) -> Result<(), DAGCircuitInnerError> { for b in self.qargs_interner.get(inst.qubits) { if self.qubit_io_map.len() - 1 < b.index() { - return Err(DAGCircuitError::new_err(format!( - "qubit {:?} not found in output map", - self.qubits.get(*b).unwrap() + return Err(DAGCircuitInnerError::WireNotInOutput(ConcreteWire::Qubit( + self.qubits.get(*b).cloned().unwrap(), ))); } } for b in self.cargs_interner.get(inst.clbits) { if !self.clbit_io_map.len() - 1 < b.index() { - return Err(DAGCircuitError::new_err(format!( - "clbit {:?} not found in output map", - self.clbits.get(*b).unwrap() + return Err(DAGCircuitInnerError::WireNotInOutput(ConcreteWire::Clbit( + self.clbits.get(*b).cloned().unwrap(), ))); } } if self.may_have_additional_wires(inst) { - let (clbits, vars) = Python::attach(|py| self.additional_wires(py, inst))?; + let (clbits, vars) = self.additional_wires(inst)?; for b in clbits { if !self.clbit_io_map.len() - 1 < b.index() { - return Err(DAGCircuitError::new_err(format!( - "clbit {:?} not found in output map", - self.clbits.get(b).unwrap() + return Err(DAGCircuitInnerError::WireNotInOutput(ConcreteWire::Clbit( + self.clbits.get(b).cloned().unwrap(), ))); } } for v in vars { if !self.var_io_map.len() - 1 < v.index() { - return Err(DAGCircuitError::new_err(format!( - "var {v:?} not found in output map" - ))); + return Err(DAGCircuitInnerError::WireNotInOutput(ConcreteWire::Var(v))); } } } @@ -7297,11 +7426,9 @@ impl DAGCircuit { } } - pub fn add_global_phase(&mut self, value: &Param) -> PyResult<()> { + pub fn add_global_phase(&mut self, value: &Param) -> Result<(), DAGCircuitInnerError> { match value { - Param::Obj(_) => Err(PyTypeError::new_err( - "Invalid parameter type, only float and parameter expression are supported", - )), + Param::Obj(_) => Err(DAGCircuitInnerError::ObjGlobalPhase), _ => self .set_global_phase_param(add_global_phase(&self.global_phase, value)) .map(|_| ()), @@ -7313,14 +7440,17 @@ impl DAGCircuit { /// Args: /// py: The python token necessary for control flow recursion /// recurse: Whether to recurse into control flow ops or not - pub fn count_ops(&self, recurse: bool) -> PyResult> { + pub fn count_ops( + &self, + recurse: bool, + ) -> Result, DAGCircuitInnerError> { if !recurse || !self.has_control_flow() { Ok(self.op_names.clone()) } else { fn inner( dag: &DAGCircuit, counts: &mut IndexMap, - ) -> PyResult<()> { + ) -> Result<(), DAGCircuitInnerError> { for (key, value) in dag.op_names.iter() { counts .entry(key.clone()) @@ -7355,7 +7485,7 @@ impl DAGCircuit { } /// Extends the DAG with valid instances of [PackedInstruction]. - pub fn extend(&mut self, iter: I) -> PyResult> + pub fn extend(&mut self, iter: I) -> Result, DAGCircuitInnerError> where I: IntoIterator, { @@ -7367,10 +7497,10 @@ impl DAGCircuit { /// Extends the DAG with valid instances of [PackedInstruction], where the iterator produces the /// results in a fallible manner. - pub fn try_extend(&mut self, iter: I) -> PyResult> + pub fn try_extend(&mut self, iter: I) -> Result, DAGCircuitInnerError> where I: IntoIterator>, - PyErr: From, + DAGCircuitInnerError: From, { let mut new_nodes = Vec::new(); let mut replacement_dag = DAGCircuit::new(); @@ -7389,7 +7519,7 @@ impl DAGCircuit { copy_op: bool, qubit_order: Option>, clbit_order: Option>, - ) -> PyResult { + ) -> Result { // Extract necessary attributes let qc_data = qc.data; Self::from_circuit_data( @@ -7410,7 +7540,7 @@ impl DAGCircuit { metadata: Option>, qubit_order: Option>, clbit_order: Option>, - ) -> PyResult { + ) -> Result { let num_qubits = qc_data.num_qubits(); let num_clbits = qc_data.num_clbits(); let num_ops = qc_data.len(); @@ -7444,76 +7574,68 @@ impl DAGCircuit { // Add the qubits depending on order, and produce the qargs map. let qarg_map = if let Some(qubit_ordering) = qubit_order { let mut ordered_vec = Vec::from_iter((0..num_qubits as u32).map(Qubit)); - qubit_ordering - .into_iter() - .try_for_each(|qubit| -> PyResult<()> { - let shareable_qubit = qc_data.qubits().get(qubit).ok_or_else(|| { - DAGCircuitError::new_err(format!("Qubit index {} not found", qubit.0)) - })?; + qubit_ordering.into_iter().try_for_each( + |qubit| -> Result<(), DAGCircuitInnerError> { + let shareable_qubit = qc_data + .qubits() + .get(qubit) + .ok_or_else(|| DAGCircuitInnerError::WireNotInCircuit(qubit.into()))?; if new_dag.qubits.find(shareable_qubit).is_some() { - return Err(DAGCircuitError::new_err(format!( - "duplicate qubits {:?}", - &qubit - ))); + return Err(DAGCircuitInnerError::DuplicateWire(qubit.into())); } let qubit_index = qc_data.qubits().find(shareable_qubit).unwrap(); ordered_vec[qubit_index.index()] = new_dag.add_qubit_unchecked(shareable_qubit.clone())?; Ok(()) - })?; + }, + )?; // The `Vec::get` use is because an arbitrary interner might contain old references to // bit instances beyond `num_qubits`, such as if it's from a DAG that had wires removed. new_dag.merge_qargs(qc_data.qargs_interner(), |bit| { ordered_vec.get(bit.index()).copied() }) } else { - qc_data - .qubits() - .objects() - .iter() - .try_for_each(|qubit| -> PyResult<_> { + qc_data.qubits().objects().iter().try_for_each( + |qubit| -> Result<(), DAGCircuitInnerError> { new_dag.add_qubit_unchecked(qubit.clone())?; Ok(()) - })?; + }, + )?; new_dag.merge_qargs(qc_data.qargs_interner(), |bit| Some(*bit)) }; // Add the clbits depending on order, and produce the cargs map. let carg_map = if let Some(clbit_ordering) = clbit_order { let mut ordered_vec = Vec::from_iter((0..num_clbits as u32).map(Clbit)); - clbit_ordering - .into_iter() - .try_for_each(|clbit| -> PyResult<()> { - let shareable_clbit = qc_data.clbits().get(clbit).ok_or_else(|| { - DAGCircuitError::new_err(format!("Clbit index {} not found", clbit.0)) - })?; + clbit_ordering.into_iter().try_for_each( + |clbit| -> Result<(), DAGCircuitInnerError> { + let shareable_clbit = qc_data + .clbits() + .get(clbit) + .ok_or_else(|| DAGCircuitInnerError::WireNotInCircuit(clbit.into()))?; if new_dag.clbits.find(shareable_clbit).is_some() { - return Err(DAGCircuitError::new_err(format!( - "duplicate clbits {:?}", - &clbit - ))); + return Err(DAGCircuitInnerError::DuplicateWire(clbit.into())); }; let clbit_index = qc_data.clbits().find(shareable_clbit).unwrap(); ordered_vec[clbit_index.index()] = new_dag.add_clbit_unchecked(shareable_clbit.clone())?; Ok(()) - })?; + }, + )?; // The `Vec::get` use is because an arbitrary interner might contain old references to // bit instances beyond `num_clbits`, such as if it's from a DAG that had wires removed. new_dag.merge_cargs(qc_data.cargs_interner(), |bit| { ordered_vec.get(bit.index()).copied() }) } else { - qc_data - .clbits() - .objects() - .iter() - .try_for_each(|clbit| -> PyResult<()> { + qc_data.clbits().objects().iter().try_for_each( + |clbit| -> Result<(), DAGCircuitInnerError> { new_dag.add_clbit_unchecked(clbit.clone())?; Ok(()) - })?; + }, + )?; new_dag.merge_cargs(qc_data.cargs_interner(), |bit| Some(*bit)) }; @@ -7535,48 +7657,51 @@ impl DAGCircuit { new_dag.blocks = qc_data.blocks().try_map_without_references(|block| { DAGCircuit::from_circuit_data(block, copy_op, None, None, None, None) })?; - new_dag.try_extend(qc_data.iter().map(|instr| -> PyResult { - Ok(PackedInstruction { - op: if copy_op { - match instr.op.view() { - OperationRef::ControlFlow(cf) => cf.clone().into(), - OperationRef::Gate(gate) => { - PyOperationTypes::Gate(Python::attach(|py| gate.py_deepcopy(py, None))?) + new_dag.try_extend(qc_data.iter().map( + |instr| -> Result { + Ok(PackedInstruction { + op: if copy_op { + match instr.op.view() { + OperationRef::ControlFlow(cf) => cf.clone().into(), + OperationRef::Gate(gate) => PyOperationTypes::Gate( + Python::attach(|py| gate.py_deepcopy(py, None)) + .map_err(DAGCircuitInnerError::Python)?, + ) + .into(), + OperationRef::Instruction(instruction) => { + PyOperationTypes::Instruction( + Python::attach(|py| instruction.py_deepcopy(py, None)) + .map_err(DAGCircuitInnerError::Python)?, + ) .into() + } + OperationRef::Operation(operation) => PyOperationTypes::Operation( + Python::attach(|py| operation.py_deepcopy(py, None)) + .map_err(DAGCircuitInnerError::Python)?, + ) + .into(), + OperationRef::StandardGate(gate) => gate.into(), + OperationRef::StandardInstruction(instruction) => instruction.into(), + OperationRef::Unitary(unitary) => unitary.clone().into(), + OperationRef::PauliProductMeasurement(ppm) => { + PauliBased::PauliProductMeasurement(ppm.clone()).into() + } + OperationRef::PauliProductRotation(rotation) => { + PauliBased::PauliProductRotation(rotation.clone()).into() + } } - OperationRef::Instruction(instruction) => { - PyOperationTypes::Instruction(Python::attach(|py| { - instruction.py_deepcopy(py, None) - })?) - .into() - } - OperationRef::Operation(operation) => { - PyOperationTypes::Operation(Python::attach(|py| { - operation.py_deepcopy(py, None) - })?) - .into() - } - OperationRef::StandardGate(gate) => gate.into(), - OperationRef::StandardInstruction(instruction) => instruction.into(), - OperationRef::Unitary(unitary) => unitary.clone().into(), - OperationRef::PauliProductMeasurement(ppm) => { - PauliBased::PauliProductMeasurement(ppm.clone()).into() - } - OperationRef::PauliProductRotation(rotation) => { - PauliBased::PauliProductRotation(rotation.clone()).into() - } - } - } else { - instr.op.clone() - }, - qubits: qarg_map[instr.qubits], - clbits: carg_map[instr.clbits], - params: instr.params.clone(), - label: instr.label.clone(), - #[cfg(feature = "cache_pygates")] - py_op: OnceLock::new(), - }) - }))?; + } else { + instr.op.clone() + }, + qubits: qarg_map[instr.qubits], + clbits: carg_map[instr.clbits], + params: instr.params.clone(), + label: instr.label.clone(), + #[cfg(feature = "cache_pygates")] + py_op: OnceLock::new(), + }) + }, + ))?; Ok(new_dag) } @@ -7591,22 +7716,20 @@ impl DAGCircuit { cycle_check: bool, qubit_pos_map: &HashMap, clbit_pos_map: &HashMap, - ) -> PyResult { + ) -> Result { let mut block_qargs: HashSet = HashSet::new(); let mut block_cargs: HashSet = HashSet::new(); for nd in block_ids { let NodeType::Operation(instr) = self.dag.node_weight(*nd).ok_or_else(|| { - DAGCircuitError::new_err("Node in 'node_block' not found in DAG.") + DAGCircuitInnerError::General(anyhow!("Node in 'node_block' not found in DAG.")) })? else { - return Err(DAGCircuitError::new_err( - "Nodes in 'node_block' must be of type 'DAGOpNode'.", - )); + return Err(anyhow!("Nodes in 'node_block' must be of type 'DAGOpNode'.").into()); }; block_qargs.extend(self.qargs_interner.get(instr.qubits)); block_cargs.extend(self.cargs_interner.get(instr.clbits)); if self.may_have_additional_wires(instr) { - let (additional_clbits, _) = Python::attach(|py| self.additional_wires(py, instr))?; + let (additional_clbits, _) = self.additional_wires(instr)?; for clbit in additional_clbits { block_cargs.insert(clbit); } @@ -7626,11 +7749,11 @@ impl DAGCircuit { block_cargs.sort_by_key(|c| clbit_pos_map[c]); if op.num_qubits() as usize != block_qargs.len() { - return Err(DAGCircuitError::new_err(format!( + return Err(anyhow!( "Number of qubits in the replacement operation ({}) is not equal to the number of qubits in the block ({})!", op.num_qubits(), block_qargs.len() - ))); + ).into()); } let qubits = self.qargs_interner.insert_owned(block_qargs); @@ -7659,25 +7782,27 @@ impl DAGCircuit { block_ids .iter() .for_each(|node| self.untrack_instruction_in_place(*node)); - let out = match self - .dag - .contract_nodes(block_ids.iter().copied(), weight, cycle_check) - { - Ok(node) => { - self.track_instruction_in_place(node); - Ok(node) - } - Err(e) => { - for old in block_ids { - self.track_instruction_in_place(*old); + let out: Result = + match self + .dag + .contract_nodes(block_ids.iter().copied(), weight, cycle_check) + { + Ok(node) => { + self.track_instruction_in_place(node); + Ok(node) } - Err(match e { - ContractError::DAGWouldCycle => DAGCircuitError::new_err( - "Replacing the specified node block would introduce a cycle", - ), - }) - } - }; + Err(e) => { + for old in block_ids { + self.track_instruction_in_place(*old); + } + Err(match e { + ContractError::DAGWouldCycle => { + anyhow!("Replacing the specified node block would introduce a cycle") + .into() + } + }) + } + }; extra_block_refs .into_iter() .for_each(|block| _ = self.blocks.decrement(block)); @@ -7691,20 +7816,22 @@ impl DAGCircuit { clbits: Option<&[Clbit]>, block_map: HashMap, inline_captures: bool, - ) -> PyResult<()> { + ) -> Result<(), DAGCircuitInnerError> { if other.qubits.len() > self.qubits.len() || other.clbits.len() > self.clbits.len() { if other.qubits.len() > self.qubits.len() { - return Err(DAGCircuitError::new_err(dag_compose_width_error( + return Err(anyhow!(dag_compose_width_error( "qubits", other.qubits.len(), self.qubits.len(), - ))); + )) + .into()); } - return Err(DAGCircuitError::new_err(dag_compose_width_error( + return Err(anyhow!(dag_compose_width_error( "classical bits", other.clbits.len(), self.clbits.len(), - ))); + )) + .into()); } let qubit_map = match qubits { @@ -7717,10 +7844,12 @@ impl DAGCircuit { .collect(), Some(qubits) => { if qubits.len() != other.qubits.len() { - return Err(DAGCircuitError::new_err(concat!( - "Number of items in qubits parameter does not", - " match number of qubits in the circuit." - ))); + return Err(anyhow!( + "Qubits parameter has {} qubits, expected {}", + qubits.len(), + other.qubits.len(), + ) + .into()); } let other_qubits = other.qubits.objects(); @@ -7729,13 +7858,13 @@ impl DAGCircuit { .cloned() .zip(qubits.iter().map(|qubit| { self.qubits.get(*qubit).ok_or_else(|| { - DAGCircuitError::new_err(format!("Qubit index {} not found", qubit.0)) + DAGCircuitInnerError::WireNotInCircuit((*qubit).into()) }) })) .map(|(other_qubit, self_qubit_result)| { self_qubit_result.map(|self_qubit| (other_qubit, self_qubit.clone())) }) - .collect::>>()? + .collect::, DAGCircuitInnerError>>()? } }; @@ -7749,10 +7878,12 @@ impl DAGCircuit { .collect(), Some(clbits) => { if clbits.len() != other.clbits.len() { - return Err(DAGCircuitError::new_err(concat!( - "Number of items in clbits parameter does not", - " match number of clbits in the circuit." - ))); + return Err(anyhow!( + "Clbits parameter has {} clbits, expected {}", + clbits.len(), + other.qubits.len(), + ) + .into()); } let other_clbits = other.clbits.objects(); other_clbits @@ -7760,13 +7891,13 @@ impl DAGCircuit { .cloned() .zip(clbits.iter().map(|clbit| { self.clbits.get(*clbit).ok_or_else(|| { - DAGCircuitError::new_err(format!("Clbit index {} not found", clbit.0)) + DAGCircuitInnerError::WireNotInCircuit((*clbit).into()) }) })) .map(|(other_clbit, self_clbit_result)| { self_clbit_result.map(|self_clbit| (other_clbit, self_clbit.clone())) }) - .collect::>>()? + .collect::, DAGCircuitInnerError>>()? } }; @@ -7789,17 +7920,18 @@ impl DAGCircuit { let expr::Var::Standalone { name, .. } = var else { panic!("var capture not standalone"); }; - return Err(DAGCircuitError::new_err(format!( - "Variable '{name}' to be inlined is not in the base DAG. If you wanted it to be automatically added, use `inline_captures=False`." - ))); + return Err(DAGCircuitInnerError::VariableNotInlined { + name: name.clone(), + is_var: true, + }); } } for stretch in other.vars_stretches.iter_stretches(StretchType::Capture) { if self.vars_stretches.stretches().find(stretch).is_none() { - return Err(DAGCircuitError::new_err(format!( - "Stretch '{}' to be inlined is not in the base DAG. If you wanted it to be automatically added, use `inline_captures=False`.", - stretch.name - ))); + return Err(DAGCircuitInnerError::VariableNotInlined { + name: stretch.name.clone(), + is_var: false, + }); } } } else { @@ -7840,8 +7972,8 @@ impl DAGCircuit { if wire_in_dag.is_none() || (self.qubit_io_map.len() - 1 < wire_in_dag.unwrap().index()) { - return Err(DAGCircuitError::new_err(format!( - "wire {m_wire:?} not in self", + return Err(DAGCircuitInnerError::MissingWire(ConcreteWire::Qubit( + m_wire.clone(), ))); } } @@ -7852,8 +7984,8 @@ impl DAGCircuit { if wire_in_dag.is_none() || self.clbit_io_map.len() - 1 < wire_in_dag.unwrap().index() { - return Err(DAGCircuitError::new_err(format!( - "wire {m_wire:?} not in self", + return Err(DAGCircuitInnerError::MissingWire(ConcreteWire::Clbit( + m_wire.clone(), ))); } } @@ -7972,19 +8104,19 @@ impl DAGCircuit { new_op: PackedOperation, params: Option>, label: Option<&str>, - ) -> PyResult<()> { + ) -> Result<(), DAGCircuitInnerError> { let old_packed = self.dag[node_index].unwrap_operation(); if old_packed.op.num_qubits() != new_op.num_qubits() || old_packed.op.num_clbits() != new_op.num_clbits() { - return Err(DAGCircuitError::new_err(format!( + return Err(anyhow!( "Cannot replace node of width ({} qubits, {} clbits) with operation of mismatched width ({} qubits, {} clbits)", old_packed.op.num_qubits(), old_packed.op.num_clbits(), new_op.num_qubits(), new_op.num_clbits() - ))); + ).into()); } let new_inst = PackedInstruction { op: new_op, @@ -8054,8 +8186,7 @@ impl DAGCircuit { py_op: OnceLock::from(op.clone().unbind()), }; - let (additional_clbits, additional_vars) = - Python::attach(|py| self.additional_wires(py, &new_instr))?; + let (additional_clbits, additional_vars) = self.additional_wires(&new_instr)?; new_wires.extend(additional_clbits.iter().map(|x| Wire::Clbit(*x))); new_wires.extend(additional_vars.iter().map(|x| Wire::Var(*x))); @@ -8107,7 +8238,7 @@ impl DAGCircuit { } // Adds `num_vars` var wires in bulk - fn add_var_wires(&mut self, num_vars: usize) -> PyResult<()> { + fn add_var_wires(&mut self, num_vars: usize) -> Result<(), DAGCircuitInnerError> { for var_idx in 0..num_vars { self.add_wire(Wire::Var(Var::new(var_idx)))?; } @@ -8230,7 +8361,7 @@ impl DAGCircuitBuilder { params: Option>, label: Option, #[cfg(feature = "cache_pygates")] py_op: Option>, - ) -> PyResult { + ) -> Result { let instruction = self.pack_instruction( op, qubits, @@ -8244,7 +8375,10 @@ impl DAGCircuitBuilder { } /// Pushes a valid [PackedInstruction] to the back of the circuit. - pub fn push_back(&mut self, instr: PackedInstruction) -> PyResult { + pub fn push_back( + &mut self, + instr: PackedInstruction, + ) -> Result { self.dag.track_instruction(&instr); let (all_cbits, vars) = self.dag.get_classical_resources(&instr)?; let qubits_id = instr.qubits; @@ -8408,7 +8542,7 @@ impl DAGCircuitBuilder { } /// Adds a new value to the global phase of the inner [DAGCircuit]. - pub fn add_global_phase(&mut self, param: &Param) -> PyResult<()> { + pub fn add_global_phase(&mut self, param: &Param) -> Result<(), DAGCircuitInnerError> { self.dag.add_global_phase(param) } diff --git a/crates/transpiler/src/passes/sabre/route.rs b/crates/transpiler/src/passes/sabre/route.rs index 763bde4c1259..c6616d7dff74 100644 --- a/crates/transpiler/src/passes/sabre/route.rs +++ b/crates/transpiler/src/passes/sabre/route.rs @@ -212,7 +212,7 @@ impl RoutingResult<'_> { None, dag.insert_qargs(&[Qubit(map_fn(swap[1]).0), Qubit(map_fn(swap[0]).0)]), ); - dag.push_back(swap) + dag.push_back(swap).map_err(Into::into) }; // The size here is pretty arbitrary, providing it can fit at least 2q operations in. let mut apply_scratch = Vec::with_capacity(4); @@ -228,7 +228,7 @@ impl RoutingResult<'_> { qubits: dag.insert_qargs(&apply_scratch), ..inst.clone() }; - dag.push_back(new_inst) + dag.push_back(new_inst).map_err(Into::into) }; let mut dag = dag.into_builder(); diff --git a/crates/transpiler/src/passes/unroll_3q_or_more.rs b/crates/transpiler/src/passes/unroll_3q_or_more.rs index 5a31cfca67d0..1ce2d8d4f3c0 100644 --- a/crates/transpiler/src/passes/unroll_3q_or_more.rs +++ b/crates/transpiler/src/passes/unroll_3q_or_more.rs @@ -12,6 +12,7 @@ use pyo3::prelude::*; use pyo3::wrap_pyfunction; +use qiskit_circuit::dag_circuit::DAGCircuitInnerError; use rustworkx_core::petgraph::stable_graph::NodeIndex; use crate::QiskitError; @@ -25,7 +26,7 @@ pub enum Unroll3qError { #[error("Cannot unroll all 3q or more gates. No rule to expand {0}")] NoDefinition(String), #[error("Failed to substitute the definition")] - SubstitutionError(PyErr), + SubstitutionError(#[from] DAGCircuitInnerError), } #[pyfunction] @@ -36,7 +37,7 @@ pub fn py_unroll_3q_or_more(dag: &mut DAGCircuit, target: Option<&Target>) -> Py "Cannot unroll all 3q or more gates. No rule to expand {}", e )), - Unroll3qError::SubstitutionError(e) => e, + Unroll3qError::SubstitutionError(e) => e.into(), }) } From cc1713bb37f3a2461aa72a7557ccb156cc820ac5 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Fri, 15 May 2026 17:15:22 -0400 Subject: [PATCH 2/3] Use rust-native error for CommutationAnalysis The following commits modify `CommutationAnalysis` to use rust-native errors throughout its pipeline. This is done mostly by implementing `DAGCircuitInnerError` based on #16134 as a possible error in `CommutationError`. --- crates/transpiler/src/commutation_checker.rs | 4 ++++ crates/transpiler/src/passes/commutation_analysis.rs | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/transpiler/src/commutation_checker.rs b/crates/transpiler/src/commutation_checker.rs index cf58d7e2cd18..82e38d4ddf73 100644 --- a/crates/transpiler/src/commutation_checker.rs +++ b/crates/transpiler/src/commutation_checker.rs @@ -15,6 +15,7 @@ use ndarray::Array2; use ndarray::linalg::kron; use num_complex::Complex64; use num_complex::ComplexFloat; +use qiskit_circuit::dag_circuit::DAGCircuitInnerError; use qiskit_circuit::object_registry::PyObjectAsKey; use qiskit_circuit::standard_gate::standard_generators::standard_gate_exponent; use qiskit_quantum_info::sparse_observable::{PySparseObservable, SparseObservable}; @@ -54,6 +55,8 @@ pub enum CommutationError { HashingNaN, #[error("Invalid hash type: parameterized")] HashingParameter, + #[error(transparent)] + DAGCircuit(#[from] DAGCircuitInnerError), } impl From for PyErr { @@ -63,6 +66,7 @@ impl From for PyErr { CommutationError::HashingParameter | CommutationError::FirstInstructionTooLarge => { QiskitError::new_err(value.to_string()) } + CommutationError::DAGCircuit(err) => err.into(), _ => PyRuntimeError::new_err(value.to_string()), } } diff --git a/crates/transpiler/src/passes/commutation_analysis.rs b/crates/transpiler/src/passes/commutation_analysis.rs index 560178404bf5..e8e91ded72ef 100644 --- a/crates/transpiler/src/passes/commutation_analysis.rs +++ b/crates/transpiler/src/passes/commutation_analysis.rs @@ -19,7 +19,7 @@ use indexmap::IndexMap; use rayon::prelude::*; use rustworkx_core::petgraph::stable_graph::NodeIndex; -use crate::commutation_checker::CommutationChecker; +use crate::commutation_checker::{CommutationChecker, CommutationError}; use qiskit_circuit::Qubit; use qiskit_circuit::dag_circuit::{DAGCircuit, NodeType, Wire}; @@ -54,7 +54,7 @@ pub fn analyze_commutations( dag: &mut DAGCircuit, commutation_checker: &CommutationChecker, approximation_degree: f64, -) -> PyResult<(CommutationSet, NodeIndices)> { +) -> Result<(CommutationSet, NodeIndices), CommutationError> { let mut commutation_set = vec![vec![]; dag.num_qubits()]; let mut node_indices: NodeIndices = vec![IndexMap::default(); dag.num_qubits()]; @@ -62,7 +62,7 @@ pub fn analyze_commutations( |qubit: usize, commutation_entry: &mut Vec>, node_indices: &mut IndexMap| - -> PyResult<()> { + -> Result<(), CommutationError> { let wire = Wire::Qubit(Qubit(qubit as u32)); for current_gate_idx in dag.nodes_on_wire(wire) { @@ -143,12 +143,12 @@ pub fn analyze_commutations( .zip(node_indices.par_iter_mut()) .enumerate() .map( - |(qubit, (local_commutation_set, local_indices))| -> PyResult<()> { + |(qubit, (local_commutation_set, local_indices))| -> Result<(), CommutationError> { evaluate_qubit(qubit, local_commutation_set, local_indices)?; Ok(()) }, ) - .collect::>()?; + .collect::>()?; Ok((commutation_set, node_indices)) } else { for qubit in 0..dag.num_qubits() { From 18f1e62e2a2d3ebb82e8f9bb9d83770b6f3e5a39 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Sat, 16 May 2026 11:02:33 -0400 Subject: [PATCH 3/3] Use rust-native error for `CommutationCancellation` The following commits modify `CommutationCancellation` to use rust-native errors throughout its pipeline. We implement the `CommutationCancelError` enumeration which derives from both `DAGCircuitError` and `CommutationError` as well as adding two new variants for operations with parameter expressions or undefined angles. --- .../src/passes/commutation_cancellation.rs | 69 +++++++++++++++---- 1 file changed, 56 insertions(+), 13 deletions(-) diff --git a/crates/transpiler/src/passes/commutation_cancellation.rs b/crates/transpiler/src/passes/commutation_cancellation.rs index 9bd5f3910d8d..c481ab7a42ba 100644 --- a/crates/transpiler/src/passes/commutation_cancellation.rs +++ b/crates/transpiler/src/passes/commutation_cancellation.rs @@ -17,15 +17,48 @@ use pyo3::prelude::*; use pyo3::{Bound, PyResult, pyfunction, wrap_pyfunction}; use indexmap::IndexMap; +use qiskit_circuit::packed_instruction::PackedOperation; use smallvec::{SmallVec, smallvec}; use super::analyze_commutations; -use crate::commutation_checker::CommutationChecker; +use crate::commutation_checker::{CommutationChecker, CommutationError}; use qiskit_circuit::Qubit; -use qiskit_circuit::dag_circuit::{DAGCircuit, NodeType}; +use qiskit_circuit::dag_circuit::{DAGCircuit, DAGCircuitInnerError, NodeType}; use qiskit_circuit::operations::{Operation, Param, StandardGate}; use qiskit_synthesis::QiskitError; +/// Possible errors during the `CommutationCancellation` pass +#[derive(Debug, thiserror::Error)] +pub enum CommutationCancelError { + #[error("Rotational gate with parameter expression encountered in cancellation {0:?}")] + RotationalParameter(PackedOperation), + #[error("Angle for operation {0} is not defined")] + OperationUndefinedAngle(String), + #[error(transparent)] + DAGCircuit(#[from] DAGCircuitInnerError), + #[error(transparent)] + Analysis(#[from] CommutationError), +} + +impl From for PyErr { + fn from(value: CommutationCancelError) -> Self { + match value { + CommutationCancelError::RotationalParameter(_) => { + QiskitError::new_err(value.to_string()) + } + CommutationCancelError::OperationUndefinedAngle(_) => { + PyRuntimeError::new_err(value.to_string()) + } + CommutationCancelError::Analysis(commutation_analysis_error) => { + commutation_analysis_error.into() + } + CommutationCancelError::DAGCircuit(dagcircuit_inner_error) => { + dagcircuit_inner_error.into() + } + } + } +} + const _CUTOFF_PRECISION: f64 = 1e-5; static ROTATION_GATES: [&str; 4] = ["p", "u1", "rz", "rx"]; @@ -65,14 +98,24 @@ struct CancellationSetKey { second_index: Option, } -#[pyfunction] +#[pyfunction(name = "cancel_commutations")] #[pyo3(signature = (dag, commutation_checker, basis_gates=None, approximation_degree=1.))] -pub fn cancel_commutations( +fn py_cancel_commutations( dag: &mut DAGCircuit, commutation_checker: &mut CommutationChecker, basis_gates: Option>, approximation_degree: f64, ) -> PyResult<()> { + cancel_commutations(dag, commutation_checker, basis_gates, approximation_degree) + .map_err(Into::into) +} + +pub fn cancel_commutations( + dag: &mut DAGCircuit, + commutation_checker: &mut CommutationChecker, + basis_gates: Option>, + approximation_degree: f64, +) -> Result<(), CommutationCancelError> { let basis = basis_gates.unwrap_or_default(); let z_var_gate = dag .get_op_counts() @@ -236,10 +279,9 @@ pub fn cancel_commutations( let node_angle = match node_op.params_view().first() { Some(Param::Float(f)) => *f, _ => { - return Err(QiskitError::new_err(format!( - "Rotational gate with parameter expression encountered in cancellation {:?}", - node_op.op - ))); + return Err(CommutationCancelError::RotationalParameter( + node_op.op.clone(), + )); } }; let phase_shift = z_phase_shift(node_op_name, node_angle); @@ -250,9 +292,9 @@ pub fn cancel_commutations( "s" => Ok((FRAC_PI_2, z_phase_shift("p", FRAC_PI_2))), "z" => Ok((PI, z_phase_shift("p", PI))), "x" => Ok((PI, FRAC_PI_2)), - _ => Err(PyRuntimeError::new_err(format!( - "Angle for operation {node_op_name} is not defined" - ))), + _ => Err(CommutationCancelError::OperationUndefinedAngle( + node_op_name.to_owned(), + )), } }?; total_angle += node_angle; @@ -279,7 +321,8 @@ pub fn cancel_commutations( dag.insert_1q_on_incoming_qubit((*new_op, &[total_angle]), cancel_set[0]); } - dag.add_global_phase(&Param::Float(total_phase))?; + dag.add_global_phase(&Param::Float(total_phase)) + .map_err(CommutationCancelError::DAGCircuit)?; for node in cancel_set { dag.remove_op_node(*node); @@ -292,6 +335,6 @@ pub fn cancel_commutations( } pub fn commutation_cancellation_mod(m: &Bound) -> PyResult<()> { - m.add_wrapped(wrap_pyfunction!(cancel_commutations))?; + m.add_wrapped(wrap_pyfunction!(py_cancel_commutations))?; Ok(()) }