Deprecate DAGNode.sort_key attribute (#13744)

Prior to having the DAGCircuit in rust the sort_key attribute was added
as a cache to speed up the lexicographical topological sort. Prior to
having this attribute the topological sort method would compute the sort
key as a string on the fly which ended up being a large bottleneck in
transpilation (see: #4040 for more details). However, since migrating the
DAGCircuit implementation to Rust this sort_key attribute isn't needed
anymore because we call the rustworkx-core function with a tuple of bit
objects instead of a string. The sort_key atribute was left on in place
for backwards compatibility (it shouldn't have been public, this was a
mistake in #4040) and when we create a python DAGNode object that will
be returned to Python the creation of the sort key is unnecessary
overhead (it takes roughly 1% of profile time to format the sort_key
string during transpilation). Since nothing in DAGCircuit is actually
using it this commit removes it to save the CPU cycles creating the
string on each dag creation.

This attribute is removed in Qiskit 2.0.0 in #13736

Author: mtreinish

crates/circuit/src/dag_node.rs

@@ -15,7 +15,7 @@ use std::cell::OnceCell;
 use std::hash::Hasher;
 
 use crate::circuit_instruction::{CircuitInstruction, OperationFromPython};
-use crate::imports::QUANTUM_CIRCUIT;
+use crate::imports::{QUANTUM_CIRCUIT, WARNINGS_WARN};
 use crate::operations::{Operation, Param};
 use crate::TupleLikeArg;
 
@@ -24,7 +24,7 @@ use approx::relative_eq;
 use rustworkx_core::petgraph::stable_graph::NodeIndex;
 
 use numpy::IntoPyArray;
-use pyo3::exceptions::PyValueError;
+use pyo3::exceptions::{PyDeprecationWarning, PyValueError};
 use pyo3::prelude::*;
 use pyo3::types::{PyString, PyTuple};
 use pyo3::{intern, IntoPy, PyObject, PyResult, ToPyObject};
@@ -111,7 +111,6 @@ impl DAGNode {
 #[pyclass(module = "qiskit._accelerate.circuit", extends=DAGNode)]
 pub struct DAGOpNode {
     pub instruction: CircuitInstruction,
-    #[pyo3(get)]
     pub sort_key: PyObject,
 }
 
@@ -152,6 +151,25 @@ impl DAGOpNode {
         )
     }
 
+    #[getter]
+    fn sort_key(&self, py: Python) -> PyResult<PyObject> {
+        WARNINGS_WARN.get_bound(py).call1((
+            intern!(
+                py,
+                concat!(
+                    "The sort_key attribute of DAGOpNode has been deprecated ",
+                    "and will be removed in the Qiskit 2.0.0 release. ",
+                    "Instead you can create this by looking at the `qargs` and ",
+                    "`cargs` attributes to create an equivalent string for a ",
+                    "given DAGOpNode instance.",
+                )
+            ),
+            py.get_type_bound::<PyDeprecationWarning>(),
+            1,
+        ))?;
+        Ok(self.sort_key.clone_ref(py))
+    }
+
     fn __hash__(slf: PyRef<'_, Self>) -> PyResult<u64> {
         let super_ = slf.as_ref();
         let mut hasher = AHasher::default();
@@ -462,7 +480,6 @@ impl DAGOpNode {
 pub struct DAGInNode {
     #[pyo3(get)]
     pub wire: PyObject,
-    #[pyo3(get)]
     sort_key: PyObject,
 }
 
@@ -491,6 +508,25 @@ impl DAGInNode {
         ))
     }
 
+    #[getter]
+    fn sort_key(&self, py: Python) -> PyResult<PyObject> {
+        WARNINGS_WARN.get_bound(py).call1((
+            intern!(
+                py,
+                concat!(
+                    "The sort_key attribute of DAGInNode has been deprecated ",
+                    "and will be removed in the Qiskit 2.0.0 release. ",
+                    "Instead you can create this by looking at the `wire` ",
+                    "attribute to create an equivalent string for a given ",
+                    "DAGInNode instance."
+                )
+            ),
+            py.get_type_bound::<PyDeprecationWarning>(),
+            1,
+        ))?;
+        Ok(self.sort_key.clone_ref(py))
+    }
+
     fn __reduce__(slf: PyRef<Self>, py: Python) -> PyObject {
         let state = (slf.as_ref().node.map(|node| node.index()), &slf.sort_key);
         (py.get_type_bound::<Self>(), (&slf.wire,), state).into_py(py)
@@ -535,7 +571,6 @@ impl DAGInNode {
 pub struct DAGOutNode {
     #[pyo3(get)]
     pub wire: PyObject,
-    #[pyo3(get)]
     sort_key: PyObject,
 }
 
@@ -564,6 +599,25 @@ impl DAGOutNode {
         ))
     }
 
+    #[getter]
+    fn sort_key(&self, py: Python) -> PyResult<PyObject> {
+        WARNINGS_WARN.get_bound(py).call1((
+            intern!(
+                py,
+                concat!(
+                    "The sort_key attribute of DAGOutNode has been deprecated ",
+                    "and will be removed in the Qiskit 2.0.0 release. ",
+                    "Instead you can create this by looking at the `wire` ",
+                    "attribute to create an equivalent string for a given ",
+                    "DAGInNode instance."
+                )
+            ),
+            py.get_type_bound::<PyDeprecationWarning>(),
+            1,
+        ))?;
+        Ok(self.sort_key.clone_ref(py))
+    }
+
     fn __reduce__(slf: PyRef<Self>, py: Python) -> PyObject {
         let state = (slf.as_ref().node.map(|node| node.index()), &slf.sort_key);
         (py.get_type_bound::<Self>(), (&slf.wire,), state).into_py(py)

qiskit/dagcircuit/dagdependency_v2.py

@@ -13,6 +13,7 @@
 """_DAGDependencyV2 class for representing non-commutativity in a circuit.
 """
 
+import itertools
 import math
 from collections import OrderedDict, defaultdict, namedtuple
 from typing import Dict, List, Generator, Any
@@ -459,7 +460,9 @@ def topological_nodes(self, key=None) -> Generator[DAGOpNode, Any, Any]:
         """
 
         def _key(x):
-            return x.sort_key
+            return ",".join(
+                f"{self.find_bit(q).index:04d}" for q in itertools.chain(x.qargs, x.cargs)
+            )
 
         if key is None:
             key = _key

qiskit/transpiler/passes/routing/star_prerouting.py

@@ -11,12 +11,20 @@
 # that they have been altered from the originals.
 
 """Search for star connectivity patterns and replace them with."""
+import itertools
 from typing import Iterable, Union, Optional, List, Tuple
 from math import floor, log10
 
 from qiskit.circuit import SwitchCaseOp, Clbit, ClassicalRegister, Barrier
 from qiskit.circuit.controlflow import condition_resources, node_resources
-from qiskit.dagcircuit import DAGOpNode, DAGDepNode, DAGDependency, DAGCircuit
+from qiskit.dagcircuit import (
+    DAGOpNode,
+    DAGDepNode,
+    DAGDependency,
+    DAGCircuit,
+    DAGOutNode,
+    DAGInNode,
+)
 from qiskit.transpiler.basepasses import TransformationPass
 from qiskit.transpiler.layout import Layout
 from qiskit.transpiler.passes.routing.sabre_swap import _build_sabre_dag, _apply_sabre_result
@@ -330,7 +338,14 @@ def star_preroute(self, dag, blocks, processing_order):
         }
 
         def tie_breaker_key(node):
-            return processing_order_index_map.get(node, node.sort_key)
+            processing_order = processing_order_index_map.get(node, None)
+            if processing_order is not None:
+                return processing_order
+            if isinstance(node, (DAGInNode, DAGOutNode)):
+                return str(node.wire)
+            return ",".join(
+                f"{dag.find_bit(q).index:04d}" for q in itertools.chain(node.qargs, node.cargs)
+            )
 
         rust_processing_order = _extract_nodes(dag.topological_op_nodes(key=tie_breaker_key), dag)
 

releasenotes/notes/deprecate-DAGNode-sort-key-3c6ed71133875872.yaml

@@ -0,0 +1,20 @@
+---
+deprecations_transpiler:
+  - |
+    The :attr:`.DAGOpNode.sort_key`, :attr:`.DAGOutNode.sort_key`, and
+    :attr:`.DAGInNode.sort_key` attributes have been deprecated and will be
+    removed in the Qiskit 2.0.0 release. This attribute was originally used
+    as a lexicographical key for topological sorting nodes in
+    a :class:`.DAGCircuit`. However, the key is no longer used for this
+    as the sorting is done internally in Rust code now. If you're using this
+    attribute for anything you can recreate the key from the other attributes
+    of a node. For example, you can use a function like::
+
+        def get_sort_key(node: DAGNode):
+            if isinstance(node, (DAGInNode, DAGOutNode)):
+                return str(node.wire)
+            return ",".join(
+                f"{dag.find_bit(q).index:04d}" for q in itertools.chain(node.qargs, node.cargs)
+            )
+
+    which will generate a string like the sort key does.