Skip to content

Commit

Permalink
Fix topological sort ordering
Browse files Browse the repository at this point in the history
The topological sort key being used previously was not producing a
consistent ordering based on the structure of the DAG. It was using the
interner index as the tie-breaking sort key which is roughly insertion
order based. This would lead to differing sort orders between isomorphic
DAGs that were constructed differently. The sort order returned
previously was also a bit odd, while still valid it just would traverse
the DAG in an unexpected order. This commit fixes this by using the
qubit indices as the keys instead of the interner index.
  • Loading branch information
mtreinish authored and jakelishman committed Jul 25, 2024
1 parent 3c66ad0 commit 7ca3ff8
Showing 1 changed file with 13 additions and 13 deletions.
26 changes: 13 additions & 13 deletions crates/circuit/src/dag_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::imports::{
CONTROL_FLOW_OP, DAG_TO_CIRCUIT, EXPR, FOR_LOOP_OP_CHECK, ITER_VARS, LEGACY_CONDITION_CHECK,
STORE_OP, SWITCH_CASE_OP, SWITCH_CASE_OP_CHECK, VARIABLE_MAPPER,
};
use crate::interner::{Index, IndexedInterner, Interner};
use crate::interner::{IndexedInterner, Interner};
use crate::operations::{Operation, OperationRef, Param, PyInstruction};
use crate::packed_instruction::PackedInstruction;
use crate::rustworkx_core_vnext::isomorphism;
Expand Down Expand Up @@ -79,16 +79,6 @@ pub(crate) enum NodeType {
Operation(PackedInstruction),
}

impl NodeType {
#[inline]
pub fn key(&self) -> (Option<Index>, Option<Index>) {
match self {
NodeType::Operation(packed) => (Some(packed.qubits), Some(packed.clbits)),
_ => (None, None),
}
}
}

#[derive(Clone, Debug)]
pub(crate) enum Wire {
Qubit(Qubit),
Expand Down Expand Up @@ -5128,8 +5118,18 @@ impl DAGCircuit {
}

fn topological_nodes(&self) -> PyResult<impl Iterator<Item = NodeIndex>> {
let key = |node: NodeIndex| -> Result<(Option<Index>, Option<Index>), Infallible> {
Ok(self.dag.node_weight(node).unwrap().key())
let key = |node: NodeIndex| -> Result<(Option<&[Qubit]>, Option<&[Clbit]>), Infallible> {
Ok(match &self.dag[node] {
NodeType::Operation(packed) => (
Some(self.qargs_cache.intern(packed.qubits).as_slice()),
Some(self.cargs_cache.intern(packed.clbits).as_slice()),
),
NodeType::QubitIn(qubit) => (Some(std::slice::from_ref(qubit)), None),
NodeType::ClbitIn(clbit) => (None, Some(std::slice::from_ref(clbit))),
NodeType::QubitOut(qubit) => (Some(std::slice::from_ref(qubit)), None),
NodeType::ClbitOut(clbit) => (None, Some(std::slice::from_ref(clbit))),
_ => (None, None),
})
};
let nodes =
rustworkx_core::dag_algo::lexicographical_topological_sort(&self.dag, key, false, None)
Expand Down

0 comments on commit 7ca3ff8

Please sign in to comment.