From 22cdeb5d47ee82d84da137c76f25a2a4d1aea12d Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Sun, 25 Aug 2024 13:05:03 -0400 Subject: [PATCH 01/12] Initial: Add add_from_iter method to DAGCircuit - Introduce a method that adds a chain of `PackedInstruction` continuously avoiding the re-linking of each bit's output-node until the very end of the iterator. - TODO: Add handling of vars - Add header for a `from_iter` function that will create a `DAGCircuit` based on a chain of `PackedInstruction`. --- crates/circuit/src/dag_circuit.rs | 115 ++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 90a4c75408fa..ab1c00286c11 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6347,6 +6347,121 @@ impl DAGCircuit { Err(DAGCircuitError::new_err("Specified node is not an op node")) } } + + /// Adds valid instances of [PackedInstruction] to the back of the Circuit. + pub fn add_from_iter(&mut self, py: Python, iter: I) -> PyResult> + where + I: IntoIterator, + { + // Create HashSets to keep track of each bit/var's last node + let mut qubit_last_nodes: HashMap = HashMap::default(); + let mut clbit_last_nodes: HashMap = HashMap::default(); + // TODO: Keep track of vars + + // Store new nodes to return + let mut new_nodes = vec![]; + for instr in iter { + let op_name = instr.op.name(); + // TODO: Use _vars + let (all_cbits, _vars): (Vec, Option>) = { + // Check if the clbits are already included + if self.may_have_additional_wires(py, &instr) { + let mut clbits: HashSet = + HashSet::from_iter(self.cargs_interner.get(instr.clbits).iter().copied()); + let (additional_clbits, additional_vars) = + self.additional_wires(py, instr.op.view(), instr.condition())?; + for clbit in additional_clbits { + clbits.insert(clbit); + } + (clbits.into_iter().collect(), Some(additional_vars)) + } else { + (self.cargs_interner.get(instr.clbits).to_vec(), None) + } + }; + + // Increment the operation count + self.increment_op(op_name); + + // Get the correct qubit indices + let qubits_id = instr.qubits; + + // Insert op-node to graph. + let new_node = self.dag.add_node(NodeType::Operation(instr)); + new_nodes.push(new_node); + + // For each qubit and cl_bit retrieve the last_nodes + let mut nodes_to_connect: HashSet<(NodeIndex, Wire)> = HashSet::default(); + // Check all the qubits in this instruction. + for qubit in self.qargs_interner.get(qubits_id) { + // Retrieve each qubit's last node + let qubit_last_node = if let Some((node, wire)) = qubit_last_nodes.remove(qubit) { + (node, wire) + } else { + let output_node = self.qubit_io_map[qubit.0 as usize][1]; + let (edge_id, predecessor_node) = self + .dag + .edges_directed(output_node, Incoming) + .next() + .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) + .unwrap(); + self.dag.remove_edge(edge_id); + predecessor_node + }; + qubit_last_nodes + .entry(*qubit) + .and_modify(|val| *val = (new_node, qubit_last_node.1.clone())); + nodes_to_connect.insert(qubit_last_node); + } + + // Check all the clbits in this instruction. + for clbit in all_cbits { + let clbit_last_node = if let Some((node, wire)) = clbit_last_nodes.remove(&clbit) { + (node, wire) + } else { + let output_node = self.clbit_io_map[clbit.0 as usize][1]; + let (edge_id, predecessor_node) = self + .dag + .edges_directed(output_node, Incoming) + .next() + .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) + .unwrap(); + self.dag.remove_edge(edge_id); + predecessor_node + }; + clbit_last_nodes + .entry(clbit) + .and_modify(|val| *val = (new_node, clbit_last_node.1.clone())); + nodes_to_connect.insert(clbit_last_node); + } + + // TODO: Check all the vars in this instruction. + + for (node, wire) in nodes_to_connect { + self.dag.add_edge(node, new_node, wire); + } + } + + // Add the output_nodes back + for (qubit, (node, wire)) in qubit_last_nodes { + let output_node = self.qubit_io_map[qubit.0 as usize][1]; + self.dag.add_edge(node, output_node, wire); + } + + for (clbit, (node, wire)) in clbit_last_nodes { + let output_node = self.clbit_io_map[clbit.0 as usize][1]; + self.dag.add_edge(node, output_node, wire); + } + + Ok(new_nodes) + } + + /// Creates an instance of DAGCircuit from an iterator over `PackedInstruction`. + fn from_iter(_py: Python, _iter: I) -> PyResult + where + I: IntoIterator, + { + todo!() + } } /// Add to global phase. Global phase can only be Float or ParameterExpression so this From a4ed753c03f7a919e17ccdefa703bc1b03785bd5 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Sun, 25 Aug 2024 13:11:07 -0400 Subject: [PATCH 02/12] Fix: leverage new methods in layers - Fix incorrect re-insertion of last_node. --- crates/circuit/src/dag_circuit.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index ab1c00286c11..add7dc66dd32 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -4350,9 +4350,7 @@ def _format(operand): let mut new_layer = self.copy_empty_like(py, vars_mode)?; - for (node, _) in op_nodes { - new_layer.push_back(py, node.clone())?; - } + new_layer.add_from_iter(py, op_nodes.iter().map(|(inst, _)| (*inst).clone()))?; let new_layer_op_nodes = new_layer.op_nodes(false).filter_map(|node_index| { match new_layer.dag.node_weight(node_index) { @@ -6409,7 +6407,8 @@ impl DAGCircuit { }; qubit_last_nodes .entry(*qubit) - .and_modify(|val| *val = (new_node, qubit_last_node.1.clone())); + .and_modify(|val| *val = (new_node, qubit_last_node.1.clone())) + .or_insert((new_node, qubit_last_node.1.clone())); nodes_to_connect.insert(qubit_last_node); } @@ -6430,7 +6429,8 @@ impl DAGCircuit { }; clbit_last_nodes .entry(clbit) - .and_modify(|val| *val = (new_node, clbit_last_node.1.clone())); + .and_modify(|val| *val = (new_node, clbit_last_node.1.clone())) + .or_insert((new_node, clbit_last_node.1.clone())); nodes_to_connect.insert(clbit_last_node); } From 5cf74175b7c0321a15edcfc9b8335143c689e54d Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Sun, 25 Aug 2024 13:27:17 -0400 Subject: [PATCH 03/12] Fix: Keep track of Vars for add_from_iter - Remove `from_iter` --- crates/circuit/src/dag_circuit.rs | 55 ++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index add7dc66dd32..ee12c7ad3e2e 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6354,14 +6354,15 @@ impl DAGCircuit { // Create HashSets to keep track of each bit/var's last node let mut qubit_last_nodes: HashMap = HashMap::default(); let mut clbit_last_nodes: HashMap = HashMap::default(); - // TODO: Keep track of vars + // TODO: Refactor once Vars are in rust + // Dict [ Var: (int, VarWeight)] + let vars_last_nodes: Bound = PyDict::new_bound(py); // Store new nodes to return let mut new_nodes = vec![]; for instr in iter { let op_name = instr.op.name(); - // TODO: Use _vars - let (all_cbits, _vars): (Vec, Option>) = { + let (all_cbits, vars): (Vec, Option>) = { // Check if the clbits are already included if self.may_have_additional_wires(py, &instr) { let mut clbits: HashSet = @@ -6407,7 +6408,6 @@ impl DAGCircuit { }; qubit_last_nodes .entry(*qubit) - .and_modify(|val| *val = (new_node, qubit_last_node.1.clone())) .or_insert((new_node, qubit_last_node.1.clone())); nodes_to_connect.insert(qubit_last_node); } @@ -6429,38 +6429,63 @@ impl DAGCircuit { }; clbit_last_nodes .entry(clbit) - .and_modify(|val| *val = (new_node, clbit_last_node.1.clone())) .or_insert((new_node, clbit_last_node.1.clone())); nodes_to_connect.insert(clbit_last_node); } - // TODO: Check all the vars in this instruction. + // If available, check all the vars in this instruction + if let Some(vars_available) = vars { + for var in vars_available { + let var_last_node = if vars_last_nodes.contains(&var)? { + let (node, wire): (usize, PyObject) = + vars_last_nodes.get_item(&var)?.unwrap().extract()?; + (NodeIndex::new(node), Wire::Var(wire)) + } else { + let output_node = self.var_output_map.get(py, &var).unwrap(); + let (edge_id, predecessor_node) = self + .dag + .edges_directed(output_node, Incoming) + .next() + .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) + .unwrap(); + self.dag.remove_edge(edge_id); + predecessor_node + }; + if let Wire::Var(var) = &var_last_node.1 { + vars_last_nodes.set_item(var, (new_node.index(), var))? + } + nodes_to_connect.insert(var_last_node); + } + } + + // Add all of the new edges for (node, wire) in nodes_to_connect { self.dag.add_edge(node, new_node, wire); } } - // Add the output_nodes back + // Add the output_nodes back to qargs for (qubit, (node, wire)) in qubit_last_nodes { let output_node = self.qubit_io_map[qubit.0 as usize][1]; self.dag.add_edge(node, output_node, wire); } + // Add the output_nodes back to cargs for (clbit, (node, wire)) in clbit_last_nodes { let output_node = self.clbit_io_map[clbit.0 as usize][1]; self.dag.add_edge(node, output_node, wire); } - Ok(new_nodes) - } + // Add the output_nodes back to vars + for item in vars_last_nodes.items() { + let (var, (node, wire)): (PyObject, (usize, PyObject)) = item.extract()?; + let output_node = self.var_output_map.get(py, &var).unwrap(); + self.dag + .add_edge(NodeIndex::new(node), output_node, Wire::Var(wire)); + } - /// Creates an instance of DAGCircuit from an iterator over `PackedInstruction`. - fn from_iter(_py: Python, _iter: I) -> PyResult - where - I: IntoIterator, - { - todo!() + Ok(new_nodes) } } From e6c4fff4542dce1457bc92a403374e776711ebf2 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Mon, 26 Aug 2024 09:25:16 -0400 Subject: [PATCH 04/12] Fix: Incorrect modification of last nodes in `add_from_iter`. - Use `entry` api to either modify or insert a value if missing. --- crates/circuit/src/dag_circuit.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index ee12c7ad3e2e..a7de1d87d878 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6408,6 +6408,7 @@ impl DAGCircuit { }; qubit_last_nodes .entry(*qubit) + .and_modify(|entry| *entry = (new_node, qubit_last_node.1.clone())) .or_insert((new_node, qubit_last_node.1.clone())); nodes_to_connect.insert(qubit_last_node); } @@ -6429,6 +6430,7 @@ impl DAGCircuit { }; clbit_last_nodes .entry(clbit) + .and_modify(|entry| *entry = (new_node, clbit_last_node.1.clone())) .or_insert((new_node, clbit_last_node.1.clone())); nodes_to_connect.insert(clbit_last_node); } @@ -6453,7 +6455,7 @@ impl DAGCircuit { }; if let Wire::Var(var) = &var_last_node.1 { - vars_last_nodes.set_item(var, (new_node.index(), var))? + vars_last_nodes.set_item(var, (new_node.index(), var))?; } nodes_to_connect.insert(var_last_node); } From 24c8b6a6e3cd6239366047bfd3754f43900aabd4 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Mon, 26 Aug 2024 13:28:16 -0400 Subject: [PATCH 05/12] Fix: Cycling edges in when adding vars. - A bug that adds duplicate edges to vars has been temporarily fixed. However, the root of this problem hasn't been found yet. A proper fix is pending. For now skip those instances. --- crates/circuit/src/dag_circuit.rs | 99 ++++++++++++++++--------------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index a7de1d87d878..5ff4b9a16bce 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6352,8 +6352,8 @@ impl DAGCircuit { I: IntoIterator, { // Create HashSets to keep track of each bit/var's last node - let mut qubit_last_nodes: HashMap = HashMap::default(); - let mut clbit_last_nodes: HashMap = HashMap::default(); + let mut qubit_last_nodes: HashMap = HashMap::default(); + let mut clbit_last_nodes: HashMap = HashMap::default(); // TODO: Refactor once Vars are in rust // Dict [ Var: (int, VarWeight)] let vars_last_nodes: Bound = PyDict::new_bound(py); @@ -6389,102 +6389,103 @@ impl DAGCircuit { new_nodes.push(new_node); // For each qubit and cl_bit retrieve the last_nodes - let mut nodes_to_connect: HashSet<(NodeIndex, Wire)> = HashSet::default(); + let mut nodes_to_connect: HashSet = HashSet::default(); // Check all the qubits in this instruction. for qubit in self.qargs_interner.get(qubits_id) { // Retrieve each qubit's last node - let qubit_last_node = if let Some((node, wire)) = qubit_last_nodes.remove(qubit) { - (node, wire) + let qubit_last_node = if let Some(node) = qubit_last_nodes.remove(qubit) { + node } else { let output_node = self.qubit_io_map[qubit.0 as usize][1]; let (edge_id, predecessor_node) = self .dag .edges_directed(output_node, Incoming) .next() - .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) + .map(|edge| (edge.id(), edge.source())) .unwrap(); self.dag.remove_edge(edge_id); predecessor_node }; - qubit_last_nodes - .entry(*qubit) - .and_modify(|entry| *entry = (new_node, qubit_last_node.1.clone())) - .or_insert((new_node, qubit_last_node.1.clone())); - nodes_to_connect.insert(qubit_last_node); + qubit_last_nodes.entry(*qubit).or_insert(new_node); + if !nodes_to_connect.contains(&qubit_last_node) { + self.dag + .add_edge(qubit_last_node, new_node, Wire::Qubit(*qubit)); + nodes_to_connect.insert(qubit_last_node); + } } // Check all the clbits in this instruction. for clbit in all_cbits { - let clbit_last_node = if let Some((node, wire)) = clbit_last_nodes.remove(&clbit) { - (node, wire) + let clbit_last_node = if let Some(node) = clbit_last_nodes.remove(&clbit) { + node } else { let output_node = self.clbit_io_map[clbit.0 as usize][1]; let (edge_id, predecessor_node) = self .dag .edges_directed(output_node, Incoming) .next() - .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) + .map(|edge| (edge.id(), edge.source())) .unwrap(); self.dag.remove_edge(edge_id); predecessor_node }; - clbit_last_nodes - .entry(clbit) - .and_modify(|entry| *entry = (new_node, clbit_last_node.1.clone())) - .or_insert((new_node, clbit_last_node.1.clone())); - nodes_to_connect.insert(clbit_last_node); + clbit_last_nodes.entry(clbit).or_insert(new_node); + if !nodes_to_connect.contains(&clbit_last_node) { + self.dag + .add_edge(clbit_last_node, new_node, Wire::Clbit(clbit)); + nodes_to_connect.insert(clbit_last_node); + } } // If available, check all the vars in this instruction - if let Some(vars_available) = vars { - for var in vars_available { - let var_last_node = if vars_last_nodes.contains(&var)? { - let (node, wire): (usize, PyObject) = - vars_last_nodes.get_item(&var)?.unwrap().extract()?; - (NodeIndex::new(node), Wire::Var(wire)) - } else { - let output_node = self.var_output_map.get(py, &var).unwrap(); - let (edge_id, predecessor_node) = self - .dag - .edges_directed(output_node, Incoming) - .next() - .map(|edge| (edge.id(), (edge.source(), edge.weight().clone()))) - .unwrap(); - self.dag.remove_edge(edge_id); - predecessor_node - }; + for var in vars.iter().flatten() { + let var_last_node = if let Some(result) = vars_last_nodes.get_item(var)? { + let node: usize = result.extract()?; + vars_last_nodes.del_item(var)?; + NodeIndex::new(node) + } else { + let output_node = self.var_output_map.get(py, var).unwrap(); + let (edge_id, predecessor_node) = self + .dag + .edges_directed(output_node, Incoming) + .next() + .map(|edge| (edge.id(), edge.source())) + .unwrap(); + self.dag.remove_edge(edge_id); + predecessor_node + }; - if let Wire::Var(var) = &var_last_node.1 { - vars_last_nodes.set_item(var, (new_node.index(), var))?; + vars_last_nodes.set_item(var, new_node.index())?; + if !nodes_to_connect.contains(&var_last_node) { + if var_last_node == new_node { + // TODO: Fix instances of duplicate nodes for Vars + continue; } + self.dag + .add_edge(var_last_node, new_node, Wire::Var(var.clone_ref(py))); nodes_to_connect.insert(var_last_node); } } - - // Add all of the new edges - for (node, wire) in nodes_to_connect { - self.dag.add_edge(node, new_node, wire); - } } // Add the output_nodes back to qargs - for (qubit, (node, wire)) in qubit_last_nodes { + for (qubit, node) in qubit_last_nodes { let output_node = self.qubit_io_map[qubit.0 as usize][1]; - self.dag.add_edge(node, output_node, wire); + self.dag.add_edge(node, output_node, Wire::Qubit(qubit)); } // Add the output_nodes back to cargs - for (clbit, (node, wire)) in clbit_last_nodes { + for (clbit, node) in clbit_last_nodes { let output_node = self.clbit_io_map[clbit.0 as usize][1]; - self.dag.add_edge(node, output_node, wire); + self.dag.add_edge(node, output_node, Wire::Clbit(clbit)); } // Add the output_nodes back to vars for item in vars_last_nodes.items() { - let (var, (node, wire)): (PyObject, (usize, PyObject)) = item.extract()?; + let (var, node): (PyObject, usize) = item.extract()?; let output_node = self.var_output_map.get(py, &var).unwrap(); self.dag - .add_edge(NodeIndex::new(node), output_node, Wire::Var(wire)); + .add_edge(NodeIndex::new(node), output_node, Wire::Var(var)); } Ok(new_nodes) From 0821affa11cfe6228199ce88595f41af4bb36646 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Mon, 26 Aug 2024 14:11:24 -0400 Subject: [PATCH 06/12] Fix: Remove set collecting all nodes to be connected. - A set collecting all the new nodes to connect with a new node was preventing additional wires to connect to subsequent nodes. --- crates/circuit/src/dag_circuit.rs | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 5ff4b9a16bce..f9c610c574b6 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6388,8 +6388,6 @@ impl DAGCircuit { let new_node = self.dag.add_node(NodeType::Operation(instr)); new_nodes.push(new_node); - // For each qubit and cl_bit retrieve the last_nodes - let mut nodes_to_connect: HashSet = HashSet::default(); // Check all the qubits in this instruction. for qubit in self.qargs_interner.get(qubits_id) { // Retrieve each qubit's last node @@ -6407,11 +6405,8 @@ impl DAGCircuit { predecessor_node }; qubit_last_nodes.entry(*qubit).or_insert(new_node); - if !nodes_to_connect.contains(&qubit_last_node) { - self.dag - .add_edge(qubit_last_node, new_node, Wire::Qubit(*qubit)); - nodes_to_connect.insert(qubit_last_node); - } + self.dag + .add_edge(qubit_last_node, new_node, Wire::Qubit(*qubit)); } // Check all the clbits in this instruction. @@ -6430,11 +6425,8 @@ impl DAGCircuit { predecessor_node }; clbit_last_nodes.entry(clbit).or_insert(new_node); - if !nodes_to_connect.contains(&clbit_last_node) { - self.dag - .add_edge(clbit_last_node, new_node, Wire::Clbit(clbit)); - nodes_to_connect.insert(clbit_last_node); - } + self.dag + .add_edge(clbit_last_node, new_node, Wire::Clbit(clbit)); } // If available, check all the vars in this instruction @@ -6456,15 +6448,12 @@ impl DAGCircuit { }; vars_last_nodes.set_item(var, new_node.index())?; - if !nodes_to_connect.contains(&var_last_node) { - if var_last_node == new_node { - // TODO: Fix instances of duplicate nodes for Vars - continue; - } - self.dag - .add_edge(var_last_node, new_node, Wire::Var(var.clone_ref(py))); - nodes_to_connect.insert(var_last_node); + if var_last_node == new_node { + // TODO: Fix instances of duplicate nodes for Vars + continue; } + self.dag + .add_edge(var_last_node, new_node, Wire::Var(var.clone_ref(py))); } } From dcf674bb0c2174a39db9a62057a1f9f8dcce5f58 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Tue, 27 Aug 2024 15:29:01 -0400 Subject: [PATCH 07/12] Fix: Adapt to #13033 --- crates/circuit/src/dag_circuit.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index f9c610c574b6..09f0a2e716f6 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6363,7 +6363,6 @@ impl DAGCircuit { for instr in iter { let op_name = instr.op.name(); let (all_cbits, vars): (Vec, Option>) = { - // Check if the clbits are already included if self.may_have_additional_wires(py, &instr) { let mut clbits: HashSet = HashSet::from_iter(self.cargs_interner.get(instr.clbits).iter().copied()); From 3cb950ee5160922f07d0815fbd996f7309c619e3 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Wed, 4 Sep 2024 10:09:39 -0400 Subject: [PATCH 08/12] Refactor: `add_from_iter` is now called `extend` to stick with `Rust` nomenclature. --- crates/circuit/src/dag_circuit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 09f0a2e716f6..31ccd0eb31fa 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -4350,7 +4350,7 @@ def _format(operand): let mut new_layer = self.copy_empty_like(py, vars_mode)?; - new_layer.add_from_iter(py, op_nodes.iter().map(|(inst, _)| (*inst).clone()))?; + new_layer.extend(py, op_nodes.iter().map(|(inst, _)| (*inst).clone()))?; let new_layer_op_nodes = new_layer.op_nodes(false).filter_map(|node_index| { match new_layer.dag.node_weight(node_index) { @@ -6347,7 +6347,7 @@ impl DAGCircuit { } /// Adds valid instances of [PackedInstruction] to the back of the Circuit. - pub fn add_from_iter(&mut self, py: Python, iter: I) -> PyResult> + pub fn extend(&mut self, py: Python, iter: I) -> PyResult> where I: IntoIterator, { From ff4b6eb2776a8f3a6a43681c1afe4b330a83c936 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com> Date: Wed, 4 Sep 2024 12:57:45 -0400 Subject: [PATCH 09/12] Fix docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Caught by @ElePT Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com> --- crates/circuit/src/dag_circuit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 31ccd0eb31fa..2aaf5c450d26 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6346,7 +6346,7 @@ impl DAGCircuit { } } - /// Adds valid instances of [PackedInstruction] to the back of the Circuit. + /// Extends the DAG with valid instances of [PackedInstruction] pub fn extend(&mut self, py: Python, iter: I) -> PyResult> where I: IntoIterator, From 8011fdf547ab1a6539cc3920136fa9746cab9ba5 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Wed, 4 Sep 2024 12:58:38 -0400 Subject: [PATCH 10/12] Fix: Remove duplicate vars check --- crates/circuit/src/dag_circuit.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 2aaf5c450d26..1eb27b71d9d9 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6447,10 +6447,6 @@ impl DAGCircuit { }; vars_last_nodes.set_item(var, new_node.index())?; - if var_last_node == new_node { - // TODO: Fix instances of duplicate nodes for Vars - continue; - } self.dag .add_edge(var_last_node, new_node, Wire::Var(var.clone_ref(py))); } From 468869c8bb9fcfd508662f5e5e1c330ae255be12 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Fri, 6 Sep 2024 09:16:21 -0400 Subject: [PATCH 11/12] Fix: Corrections from code review. - Use Entry API to modify last nodes in the var. - Build new_nodes with an allocated vec. - Add comment explaining the removal of the edge between the output node and its predecessor. --- crates/circuit/src/dag_circuit.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 06233618660e..75f0c4bc8232 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6358,8 +6358,10 @@ impl DAGCircuit { // Dict [ Var: (int, VarWeight)] let vars_last_nodes: Bound = PyDict::new_bound(py); + // Consume into iterator to obtain size hint + let iter = iter.into_iter(); // Store new nodes to return - let mut new_nodes = vec![]; + let mut new_nodes = Vec::with_capacity(iter.size_hint().1.unwrap_or_default()); for instr in iter { let op_name = instr.op.name(); let (all_cbits, vars): (Vec, Option>) = { @@ -6390,9 +6392,9 @@ impl DAGCircuit { // Check all the qubits in this instruction. for qubit in self.qargs_interner.get(qubits_id) { // Retrieve each qubit's last node - let qubit_last_node = if let Some(node) = qubit_last_nodes.remove(qubit) { - node - } else { + let qubit_last_node = *qubit_last_nodes.entry(*qubit).or_insert({ + // If the qubit is not in the last nodes collection, the edge between the output node and its predecessor. + // Then, store the predecessor's NodeIndex in the last nodes collection. let output_node = self.qubit_io_map[qubit.0 as usize][1]; let (edge_id, predecessor_node) = self .dag @@ -6402,17 +6404,19 @@ impl DAGCircuit { .unwrap(); self.dag.remove_edge(edge_id); predecessor_node - }; - qubit_last_nodes.entry(*qubit).or_insert(new_node); + }); + qubit_last_nodes + .entry(*qubit) + .and_modify(|val| *val = new_node); self.dag .add_edge(qubit_last_node, new_node, Wire::Qubit(*qubit)); } // Check all the clbits in this instruction. for clbit in all_cbits { - let clbit_last_node = if let Some(node) = clbit_last_nodes.remove(&clbit) { - node - } else { + let clbit_last_node = *clbit_last_nodes.entry(clbit).or_insert({ + // If the qubit is not in the last nodes collection, the edge between the output node and its predecessor. + // Then, store the predecessor's NodeIndex in the last nodes collection. let output_node = self.clbit_io_map[clbit.0 as usize][1]; let (edge_id, predecessor_node) = self .dag @@ -6422,8 +6426,10 @@ impl DAGCircuit { .unwrap(); self.dag.remove_edge(edge_id); predecessor_node - }; - clbit_last_nodes.entry(clbit).or_insert(new_node); + }); + clbit_last_nodes + .entry(clbit) + .and_modify(|val| *val = new_node); self.dag .add_edge(clbit_last_node, new_node, Wire::Clbit(clbit)); } @@ -6435,6 +6441,8 @@ impl DAGCircuit { vars_last_nodes.del_item(var)?; NodeIndex::new(node) } else { + // If the var is not in the last nodes collection, the edge between the output node and its predecessor. + // Then, store the predecessor's NodeIndex in the last nodes collection. let output_node = self.var_output_map.get(py, var).unwrap(); let (edge_id, predecessor_node) = self .dag From d846cd0bfe5e5612a8b4a5471879770fb227c08f Mon Sep 17 00:00:00 2001 From: Raynel Sanchez Date: Fri, 6 Sep 2024 10:58:45 -0400 Subject: [PATCH 12/12] Fix: Improper use of `Entry API`. - Use `or_insert_with` instead of `or_insert` to perform actions before inserting a value. --- crates/circuit/src/dag_circuit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/circuit/src/dag_circuit.rs b/crates/circuit/src/dag_circuit.rs index 75f0c4bc8232..1f4acde9934a 100644 --- a/crates/circuit/src/dag_circuit.rs +++ b/crates/circuit/src/dag_circuit.rs @@ -6392,7 +6392,7 @@ impl DAGCircuit { // Check all the qubits in this instruction. for qubit in self.qargs_interner.get(qubits_id) { // Retrieve each qubit's last node - let qubit_last_node = *qubit_last_nodes.entry(*qubit).or_insert({ + let qubit_last_node = *qubit_last_nodes.entry(*qubit).or_insert_with(|| { // If the qubit is not in the last nodes collection, the edge between the output node and its predecessor. // Then, store the predecessor's NodeIndex in the last nodes collection. let output_node = self.qubit_io_map[qubit.0 as usize][1]; @@ -6414,7 +6414,7 @@ impl DAGCircuit { // Check all the clbits in this instruction. for clbit in all_cbits { - let clbit_last_node = *clbit_last_nodes.entry(clbit).or_insert({ + let clbit_last_node = *clbit_last_nodes.entry(clbit).or_insert_with(|| { // If the qubit is not in the last nodes collection, the edge between the output node and its predecessor. // Then, store the predecessor's NodeIndex in the last nodes collection. let output_node = self.clbit_io_map[clbit.0 as usize][1];