Fix non-determinism in SabreSwap rust code (#10740)

In the SabreSwap rust module there were 2 potential sources of
non-determinism that could cause variation in results even with a fixed
seed, the ExtendedSet struct's nodes attribute and the decremented
tracking when populating the extended set. Both were caused by the same
root cause iterating over a HashMap object. Iteration order on a HashMap
(or a HashSet) is dependent on the internal hash seed of the hasher and
iteration order is explicitly not guaranteed. In the case of the
decrementer it's unlikely to have any effect even if the iteration order
is not guaranteed but it is switched to using an indexmap regarldess
just out of best practice. But for the nodes attribute in the
ExtendedSet struct there is a potential issue there because when the
total_score() method is called the nodes are iterated over, the distance
is looked up for each swap and then summed. As the distances are all
floating point values the iteration order could result in different
values being output. In both cases the `hashbrown::HashMap<K, V>` is
changed to be an `indexmap::IndexMap<K, V, ahash::RandomState>` which
will have deterministic iteration order (it uses insertion order).
This commit is contained in:
Matthew Treinish 2023-08-31 05:10:03 -04:00 committed by GitHub
parent 9514dbe046
commit 48cfab601e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 11 additions and 12 deletions

View File

@ -11,7 +11,6 @@
// that they have been altered from the originals.
use ahash;
use hashbrown::HashMap;
use indexmap::IndexMap;
use ndarray::prelude::*;
use rustworkx_core::petgraph::prelude::*;
@ -149,19 +148,18 @@ impl FrontLayer {
}
}
/// This is largely similar to the `FrontLayer` struct, but does not need to track the insertion
/// order of the nodes, and can have more than one node on each active qubit. This does not have a
/// `remove` method (and its data structures aren't optimised for fast removal), since the extended
/// set is built from scratch each time a new gate is routed.
/// This is largely similar to the `FrontLayer` struct but can have more than one node on each active
/// qubit. This does not have `remove` method (and its data structures aren't optimised for fast
/// removal), since the extended set is built from scratch each time a new gate is routed.
pub struct ExtendedSet {
nodes: HashMap<NodeIndex, [usize; 2]>,
nodes: IndexMap<NodeIndex, [usize; 2], ahash::RandomState>,
qubits: Vec<Vec<usize>>,
}
impl ExtendedSet {
pub fn new(num_qubits: usize, max_size: usize) -> Self {
ExtendedSet {
nodes: HashMap::with_capacity(max_size),
nodes: IndexMap::with_capacity_and_hasher(max_size, ahash::RandomState::default()),
qubits: vec![Vec::new(); num_qubits],
}
}
@ -212,8 +210,8 @@ impl ExtendedSet {
return 0.0;
}
self.nodes
.iter()
.map(|(_, &[l_a, l_b])| dist[[layout.logic_to_phys[l_a], layout.logic_to_phys[l_b]]])
.values()
.map(|&[l_a, l_b]| dist[[layout.logic_to_phys[l_a], layout.logic_to_phys[l_b]]])
.sum::<f64>()
/ self.nodes.len() as f64
}

View File

@ -17,9 +17,8 @@ pub mod neighbor_table;
pub mod sabre_dag;
pub mod swap_map;
use std::cmp::Ordering;
use hashbrown::HashMap;
use indexmap::IndexMap;
use ndarray::prelude::*;
use numpy::PyReadonlyArray2;
use numpy::{IntoPyArray, PyArray, ToPyArray};
@ -36,6 +35,7 @@ use rustworkx_core::petgraph::prelude::*;
use rustworkx_core::petgraph::visit::EdgeRef;
use rustworkx_core::shortest_path::dijkstra;
use rustworkx_core::token_swapper::token_swapper;
use std::cmp::Ordering;
use crate::getenv_use_multiple_threads;
use crate::nlayout::NLayout;
@ -171,7 +171,8 @@ fn populate_extended_set(
required_predecessors: &mut [u32],
) {
let mut to_visit = front_layer.iter_nodes().copied().collect::<Vec<_>>();
let mut decremented: HashMap<usize, u32> = HashMap::new();
let mut decremented: IndexMap<usize, u32, ahash::RandomState> =
IndexMap::with_hasher(ahash::RandomState::default());
let mut i = 0;
while i < to_visit.len() && extended_set.len() < EXTENDED_SET_SIZE {
for edge in dag.dag.edges_directed(to_visit[i], Direction::Outgoing) {