From 5b41efab2dccb289bd741cdead7315be555ee0b5 Mon Sep 17 00:00:00 2001 From: Kevin Hartman Date: Mon, 26 Aug 2024 14:47:13 -0400 Subject: [PATCH 1/4] Avoid isinstance check on every node in DAG during recursive depth and size calls. --- crates/circuit/src/dag_circuit.rs | 66 +++++++++++++++---------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index bb5fff5e343a..a8f36dce9e7e 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -2168,14 +2168,15 @@ def _format(operand): } let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); - for node_index in - self.op_nodes_by_py_type(imports::CONTROL_FLOW_OP.get_bound(py).downcast()?, true) - { - let NodeType::Operation(node) = &self.dag[node_index] else { - return Err(DAGCircuitError::new_err("unknown control-flow type")); + for node in self.dag.node_weights() { + let NodeType::Operation(node) = node else { + continue; }; + if !node.op.control_flow() { + continue; + } let OperationRef::Instruction(inst) = node.op.view() else { - unreachable!("Control Flow operations must be a PyInstruction"); + panic!("control flow op must be an instruction"); }; let inst_bound = inst.instruction.bind(py); if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { @@ -2239,34 +2240,33 @@ def _format(operand): Ok(if recurse { let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); let mut node_lookup: HashMap = HashMap::new(); - - for node_index in - self.op_nodes_by_py_type(imports::CONTROL_FLOW_OP.get_bound(py).downcast()?, true) - { - if let NodeType::Operation(node) = &self.dag[node_index] { - if let OperationRef::Instruction(inst) = node.op.view() { - let inst_bound = inst.instruction.bind(py); - let weight = - if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { - node.params_view().len() - } else { - 1 - }; - if weight == 0 { - node_lookup.insert(node_index, 0); - } else { - let raw_blocks = inst_bound.getattr("blocks")?; - let blocks = raw_blocks.downcast::()?; - let mut block_weights: Vec = Vec::with_capacity(blocks.len()); - for block in blocks.iter() { - let inner_dag: &DAGCircuit = - &circuit_to_dag.call1((block,))?.extract()?; - block_weights.push(inner_dag.depth(py, true)?); - } - node_lookup - .insert(node_index, weight * block_weights.iter().max().unwrap()); - } + for (node_index, node) in self.dag.node_references() { + let NodeType::Operation(node) = node else { + continue; + }; + if !node.op.control_flow() { + continue; + } + let OperationRef::Instruction(inst) = node.op.view() else { + panic!("control flow op must be an instruction") + }; + let inst_bound = inst.instruction.bind(py); + let weight = if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { + node.params_view().len() + } else { + 1 + }; + if weight == 0 { + node_lookup.insert(node_index, 0); + } else { + let raw_blocks = inst_bound.getattr("blocks")?; + let blocks = raw_blocks.downcast::()?; + let mut block_weights: Vec = Vec::with_capacity(blocks.len()); + for block in blocks.iter() { + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block,))?.extract()?; + block_weights.push(inner_dag.depth(py, true)?); } + node_lookup.insert(node_index, weight * block_weights.iter().max().unwrap()); } } From 80ff154cd81771dc546e1e0efe18e6aa6e221c36 Mon Sep 17 00:00:00 2001 From: Kevin Hartman Date: Tue, 27 Aug 2024 17:00:11 -0400 Subject: [PATCH 2/4] Avoid unnecessary downcast. --- crates/circuit/src/dag_circuit.rs | 33 ++++++++++++------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index a8f36dce9e7e..0a69c6f0922f 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -2180,27 +2180,21 @@ def _format(operand): }; let inst_bound = inst.instruction.bind(py); if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { - let raw_blocks = inst_bound.getattr("blocks")?; - let blocks: &Bound = raw_blocks.downcast()?; - let block_zero = blocks.get_item(0).unwrap(); - let inner_dag: &DAGCircuit = - &circuit_to_dag.call1((block_zero.clone(),))?.extract()?; + let blocks = inst_bound.getattr("blocks")?; + let block_zero = blocks.get_item(0)?; + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block_zero,))?.extract()?; length += node.params_view().len() * inner_dag.size(py, true)? } else if inst_bound.is_instance(imports::WHILE_LOOP_OP.get_bound(py))? { - let raw_blocks = inst_bound.getattr("blocks")?; - let blocks: &Bound = raw_blocks.downcast()?; - let block_zero = blocks.get_item(0).unwrap(); - let inner_dag: &DAGCircuit = - &circuit_to_dag.call1((block_zero.clone(),))?.extract()?; + let blocks = inst_bound.getattr("blocks")?; + let block_zero = blocks.get_item(0)?; + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block_zero,))?.extract()?; length += inner_dag.size(py, true)? } else if inst_bound.is_instance(imports::IF_ELSE_OP.get_bound(py))? || inst_bound.is_instance(imports::SWITCH_CASE_OP.get_bound(py))? { - let raw_blocks = inst_bound.getattr("blocks")?; - let blocks: &Bound = raw_blocks.downcast()?; - for block in blocks.iter() { - let inner_dag: &DAGCircuit = - &circuit_to_dag.call1((block.clone(),))?.extract()?; + let blocks = inst_bound.getattr("blocks")?; + for block in blocks.iter()? { + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block?,))?.extract()?; length += inner_dag.size(py, true)?; } } else { @@ -2259,11 +2253,10 @@ def _format(operand): if weight == 0 { node_lookup.insert(node_index, 0); } else { - let raw_blocks = inst_bound.getattr("blocks")?; - let blocks = raw_blocks.downcast::()?; - let mut block_weights: Vec = Vec::with_capacity(blocks.len()); - for block in blocks.iter() { - let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block,))?.extract()?; + let blocks = inst_bound.getattr("blocks")?; + let mut block_weights: Vec = Vec::with_capacity(blocks.len()?); + for block in blocks.iter()? { + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block?,))?.extract()?; block_weights.push(inner_dag.depth(py, true)?); } node_lookup.insert(node_index, weight * block_weights.iter().max().unwrap()); From af244c2f44c2268a7ca92c9419f7a7a80db0c06e Mon Sep 17 00:00:00 2001 From: Kevin Hartman Date: Tue, 27 Aug 2024 17:39:25 -0400 Subject: [PATCH 3/4] Check if there is control flow before doing loops. --- crates/circuit/src/dag_circuit.rs | 131 +++++++++++++++--------------- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 0a69c6f0922f..587fa0ed7a3a 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -2153,20 +2153,18 @@ def _format(operand): #[pyo3(signature= (*, recurse=false))] fn size(&self, py: Python, recurse: bool) -> PyResult { let mut length = self.dag.node_count() - (self.width() * 2); - if !recurse { - if CONTROL_FLOW_OP_NAMES - .iter() - .any(|n| self.op_names.contains_key(&n.to_string())) - { - return Err(DAGCircuitError::new_err(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." - ))); - } + if !self.has_control_flow() { return Ok(length); } + if !recurse { + return Err(DAGCircuitError::new_err(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." + ))); + } + // Handle recursively. let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); for node in self.dag.node_weights() { let NodeType::Operation(node) = node else { @@ -2230,60 +2228,60 @@ def _format(operand): if self.qubits.is_empty() && self.clbits.is_empty() && self.vars_info.is_empty() { return Ok(0); } + if !self.has_control_flow() { + 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")), + }; + } + if !recurse { + return Err(DAGCircuitError::new_err(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." + ))); + } - Ok(if recurse { - let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); - let mut node_lookup: HashMap = HashMap::new(); - for (node_index, node) in self.dag.node_references() { - let NodeType::Operation(node) = node else { - continue; - }; - if !node.op.control_flow() { - continue; - } - let OperationRef::Instruction(inst) = node.op.view() else { - panic!("control flow op must be an instruction") - }; - let inst_bound = inst.instruction.bind(py); - let weight = if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { - node.params_view().len() - } else { - 1 - }; - if weight == 0 { - node_lookup.insert(node_index, 0); - } else { - let blocks = inst_bound.getattr("blocks")?; - let mut block_weights: Vec = Vec::with_capacity(blocks.len()?); - for block in blocks.iter()? { - let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block?,))?.extract()?; - block_weights.push(inner_dag.depth(py, true)?); - } - node_lookup.insert(node_index, weight * block_weights.iter().max().unwrap()); - } - } - - let weight_fn = |edge: EdgeReference<'_, Wire>| -> Result { - Ok(*node_lookup.get(&edge.target()).unwrap_or(&1)) + // Handle recursively. + let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); + let mut node_lookup: HashMap = HashMap::new(); + for (node_index, node) in self.dag.node_references() { + let NodeType::Operation(node) = node else { + continue; }; - match rustworkx_core::dag_algo::longest_path(&self.dag, weight_fn).unwrap() { - Some(res) => res.1, - None => return Err(DAGCircuitError::new_err("not a DAG")), + if !node.op.control_flow() { + continue; } - } else { - if CONTROL_FLOW_OP_NAMES - .iter() - .any(|x| self.op_names.contains_key(&x.to_string())) - { - return Err(DAGCircuitError::new_err("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.")); + let OperationRef::Instruction(inst) = node.op.view() else { + panic!("control flow op must be an instruction") + }; + let inst_bound = inst.instruction.bind(py); + let weight = if inst_bound.is_instance(imports::FOR_LOOP_OP.get_bound(py))? { + node.params_view().len() + } else { + 1 + }; + if weight == 0 { + node_lookup.insert(node_index, 0); + } else { + let blocks = inst_bound.getattr("blocks")?; + let mut block_weights: Vec = Vec::with_capacity(blocks.len()?); + for block in blocks.iter()? { + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block?,))?.extract()?; + block_weights.push(inner_dag.depth(py, true)?); + } + node_lookup.insert(node_index, weight * block_weights.iter().max().unwrap()); } + } - let weight_fn = |_| -> Result { Ok(1) }; - match rustworkx_core::dag_algo::longest_path(&self.dag, weight_fn).unwrap() { - Some(res) => res.1, - None => return Err(DAGCircuitError::new_err("not a DAG")), - } - } - 1) + let weight_fn = |edge: EdgeReference<'_, Wire>| -> Result { + Ok(*node_lookup.get(&edge.target()).unwrap_or(&1)) + }; + 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")), + } } /// Return the total number of qubits + clbits used by the circuit. @@ -4555,11 +4553,7 @@ def _format(operand): /// Mapping[str, int]: a mapping of operation names to the number of times it appears. #[pyo3(signature = (*, recurse=true))] fn count_ops(&self, py: Python, recurse: bool) -> PyResult { - if !recurse - || !CONTROL_FLOW_OP_NAMES - .iter() - .any(|x| self.op_names.contains_key(*x)) - { + if !recurse || !self.has_control_flow() { Ok(self.op_names.to_object(py)) } else { fn inner( @@ -5333,6 +5327,13 @@ impl DAGCircuit { ) } + #[inline] + fn has_control_flow(&self) -> bool { + CONTROL_FLOW_OP_NAMES + .iter() + .any(|x| self.op_names.contains_key(&x.to_string())) + } + fn is_wire_idle(&self, py: Python, wire: &Wire) -> PyResult { let (input_node, output_node) = match wire { Wire::Qubit(qubit) => ( From 3c7063085f1ee26bd5773b2b34c4552cc4761a69 Mon Sep 17 00:00:00 2001 From: Kevin Hartman Date: Tue, 27 Aug 2024 18:20:43 -0400 Subject: [PATCH 4/4] Avoid py_op_nodes in DAGCircuit::count_ops. --- crates/circuit/src/dag_circuit.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 587fa0ed7a3a..6947f831f35b 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -4568,16 +4568,19 @@ def _format(operand): .or_insert(*value); } let circuit_to_dag = imports::CIRCUIT_TO_DAG.get_bound(py); - for node in dag.py_op_nodes( - py, - Some(imports::CONTROL_FLOW_OP.get_bound(py).downcast()?), - true, - )? { - let raw_blocks = node.getattr(py, "op")?.getattr(py, "blocks")?; - let blocks: &Bound = raw_blocks.downcast_bound::(py)?; - for block in blocks.iter() { - let inner_dag: &DAGCircuit = - &circuit_to_dag.call1((block.clone(),))?.extract()?; + for node in dag.dag.node_weights() { + let NodeType::Operation(node) = node else { + continue; + }; + if !node.op.control_flow() { + continue; + } + let OperationRef::Instruction(inst) = node.op.view() else { + panic!("control flow op must be an instruction") + }; + let blocks = inst.instruction.bind(py).getattr("blocks")?; + for block in blocks.iter()? { + let inner_dag: &DAGCircuit = &circuit_to_dag.call1((block?,))?.extract()?; inner(py, inner_dag, counts)?; } }