Use toposort instead of topological_op_nodes for DAG reconstruction

This commit switches the topological sort function we use in transpiler
passes when reconstructing a dag from scratch. In several passes where
we typically replace or remove a large numbers of gates we iterate over
the input dag in topological order and construct a copy of it making the
alterations as we go. Right now when we do this we rely on
`DAGCircuit::topological_op_nodes` which makes sense because it's or
built-in method for iterating over a dag's op node in topological
ordering. Internally this uses rustworkx's lexicographical topological
sort function with our custom sort function that maintains our desired
tie breaker using the bits of a node. However, since Qiskit#14762 where we're
asserting structural equality in passes we don't need to use that sort
anymore for these reconstruction cases. We just need a consistent
topological sort. In optimizing Qiskit#13419 one thing that showed in profiles
was that for very large circuits the overhead of the lexicographical
topological sort for the iteration. The toposort function in petgraph
is lower overhead because it doesn't need to work about the
lexicographical tie breaker. By switching to use this instead we can
reduce the overhead of the final sort in all these passes. In asv
benchmarking this commit speeds up transpiler benchmarks are 2-5%
faster (although without asv flagging it as significant).

Author: mtreinish

crates/transpiler/src/passes/basis_translator/mod.rs

@@ -20,6 +20,7 @@ use errors::BasisTranslatorError;
 use hashbrown::{HashMap, HashSet};
 use indexmap::{IndexMap, IndexSet};
 use pyo3::prelude::*;
+use rustworkx_core::petgraph::algo::toposort;
 
 mod basis_search;
 mod compose_transforms;
@@ -36,7 +37,7 @@ use qiskit_circuit::parameter::symbol_expr::SymbolExpr;
 use qiskit_circuit::parameter::symbol_expr::Value;
 use qiskit_circuit::{BlocksMode, Clbit, PhysicalQubit, Qubit, VarsMode};
 use qiskit_circuit::{
-    dag_circuit::DAGCircuit,
+    dag_circuit::{DAGCircuit, NodeType},
     operations::{Operation, OperationRef, PauliBased, PyOperationTypes, PythonOperation},
 };
 use smallvec::SmallVec;
@@ -343,8 +344,10 @@ fn apply_translation(
             )
         })?;
     let mut out_dag_builder = out_dag.into_builder();
-    for node in dag.topological_op_nodes(false) {
-        let node_obj = dag[node].unwrap_operation();
+    for node in toposort(dag.dag(), None).unwrap() {
+        let NodeType::Operation(ref node_obj) = dag.dag()[node] else {
+            continue;
+        };
         let node_qarg = dag.get_qargs(node_obj.qubits);
         let node_carg = dag.get_cargs(node_obj.clbits);
         let qubit_set: AhashIndexSet<Qubit> = AhashIndexSet::from_iter(node_qarg.iter().copied());
@@ -497,8 +500,10 @@ fn replace_node(
         });
     }
     if params_view.is_empty() {
-        for inner_index in target_dag.topological_op_nodes(false) {
-            let inner_node = &target_dag[inner_index].unwrap_operation();
+        for inner_index in toposort(target_dag.dag(), None).unwrap() {
+            let NodeType::Operation(ref inner_node) = target_dag[inner_index] else {
+                continue;
+            };
             let old_qargs = dag.qargs_interner().get(node.qubits);
             let old_cargs = dag.cargs_interner().get(node.clbits);
             let new_qubits: Vec<Qubit> = target_dag
@@ -574,8 +579,10 @@ fn replace_node(
                     _ => None,
                 }),
         );
-        for inner_index in target_dag.topological_op_nodes(false) {
-            let inner_node = &target_dag[inner_index].unwrap_operation();
+        for inner_index in toposort(target_dag.dag(), None).unwrap() {
+            let NodeType::Operation(ref inner_node) = target_dag[inner_index] else {
+                continue;
+            };
             let old_qargs = dag.qargs_interner().get(node.qubits);
             let old_cargs = dag.cargs_interner().get(node.clbits);
             let new_qubits: Vec<Qubit> = target_dag

crates/transpiler/src/passes/disjoint_layout.rs

@@ -20,14 +20,15 @@ use pyo3::prelude::*;
 use pyo3::types::PyList;
 use rustworkx_core::connectivity::connected_components;
 use rustworkx_core::petgraph::EdgeType;
+use rustworkx_core::petgraph::algo::toposort;
 use rustworkx_core::petgraph::prelude::*;
 use rustworkx_core::petgraph::visit::{IntoEdgeReferences, IntoNodeReferences, NodeFiltered};
 use uuid::Uuid;
 
 use crate::TranspilerError;
 use crate::target::{Qargs, Target};
 use qiskit_circuit::bit::ShareableQubit;
-use qiskit_circuit::dag_circuit::DAGCircuit;
+use qiskit_circuit::dag_circuit::{DAGCircuit, NodeType};
 use qiskit_circuit::operations::{Operation, OperationRef, StandardInstruction};
 use qiskit_circuit::packed_instruction::PackedOperation;
 use qiskit_circuit::{
@@ -474,8 +475,10 @@ fn separate_dag(dag: &mut DAGCircuit) -> PyResult<Vec<DAGCircuit>> {
             new_dag.set_global_phase_f64(0.);
             let old_qubits = dag.qubits();
             let mut block_map = BlockMapper::new();
-            for index in dag.topological_op_nodes(false) {
-                let node = dag[index].unwrap_operation();
+            for index in toposort(dag.dag(), None).unwrap() {
+                let NodeType::Operation(ref node) = dag.dag()[index] else {
+                    continue;
+                };
                 let qargs: HashSet<Qubit> = dag.get_qargs(node.qubits).iter().copied().collect();
                 if dag_qubits.is_superset(&qargs) {
                     let qargs = dag.get_qargs(node.qubits);

crates/transpiler/src/passes/split_2q_unitaries.rs

@@ -15,6 +15,7 @@ const PI4: f64 = PI / 4.;
 use nalgebra::Matrix2;
 use num_complex::Complex64;
 use pyo3::prelude::*;
+use rustworkx_core::petgraph::algo::toposort;
 use rustworkx_core::petgraph::stable_graph::NodeIndex;
 use smallvec::{SmallVec, smallvec};
 
@@ -101,9 +102,9 @@ pub fn run_split_2q_unitaries(
     let mut mapping: Vec<usize> = (0..dag.num_qubits()).collect();
     let new_dag = dag.copy_empty_like(VarsMode::Alike, BlocksMode::Keep)?;
     let mut new_dag = new_dag.into_builder();
-    for node in dag.topological_op_nodes(false) {
-        let NodeType::Operation(inst) = &dag.dag()[node] else {
-            unreachable!("Op nodes contain a non-operation");
+    for node in toposort(dag.dag(), None).unwrap() {
+        let NodeType::Operation(ref inst) = dag.dag()[node] else {
+            continue;
         };
         if let OperationRef::Unitary(unitary_gate) = inst.op.view() {
             if unitary_gate.num_qubits() == 2 {

crates/transpiler/src/passes/unitary_synthesis/mod.rs

@@ -22,12 +22,13 @@ use std::hash;
 use numpy::PyReadonlyArray2;
 use pyo3::prelude::*;
 use pyo3::{intern, wrap_pyfunction};
+use rustworkx_core::petgraph::algo::toposort;
 
 use self::decomposers::{Decomposer2q, DecomposerCache, Direction2q, FlipDirection};
 use crate::QiskitError;
 use crate::target::Target;
 use qiskit_circuit::bit::QuantumRegister;
-use qiskit_circuit::dag_circuit::{DAGCircuit, DAGCircuitBuilder};
+use qiskit_circuit::dag_circuit::{DAGCircuit, DAGCircuitBuilder, NodeType};
 use qiskit_circuit::instruction::Parameters;
 use qiskit_circuit::operations::{
     Operation, OperationRef, Param, PyOperationTypes, PythonOperation, StandardGate,
@@ -336,8 +337,10 @@ pub fn run_unitary_synthesis(
     let mut out = dag
         .copy_empty_like(VarsMode::Alike, BlocksMode::Drop)?
         .into_builder();
-    for node in dag.topological_op_nodes(false) {
-        let inst = dag[node].unwrap_operation();
+    for node in toposort(dag.dag(), None).unwrap() {
+        let NodeType::Operation(ref inst) = dag.dag()[node] else {
+            continue;
+        };
         let Some(cf) = dag.try_view_control_flow(inst) else {
             // Handle regular instructions - this path is where we end up most of the time.
             if !synthesize_onto(&mut out, state, inst)? {