Skip to content

Commit

Permalink
Deprecate DAGNode.sort_key attribute
Browse files Browse the repository at this point in the history
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: Qiskit#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 Qiskit#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 Qiskit#13736
  • Loading branch information
mtreinish committed Jan 27, 2025
1 parent 02e1476 commit 06d2aa1
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 8 deletions.
64 changes: 59 additions & 5 deletions crates/circuit/src/dag_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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};
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -462,7 +480,6 @@ impl DAGOpNode {
pub struct DAGInNode {
#[pyo3(get)]
pub wire: PyObject,
#[pyo3(get)]
sort_key: PyObject,
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -535,7 +571,6 @@ impl DAGInNode {
pub struct DAGOutNode {
#[pyo3(get)]
pub wire: PyObject,
#[pyo3(get)]
sort_key: PyObject,
}

Expand Down Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion qiskit/dagcircuit/dagdependency_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions qiskit/transpiler/passes/routing/star_prerouting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 06d2aa1

Please sign in to comment.