Fix non-determinism in CommutativeCancellation
#14763
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The use of `HashMap` and `HashSet` for internal tracking structures in the pass, which were subsequently iterated over, caused node removals to happen in a non-deterministic order, which in turn could cause the edge structure of the `DAGCircuit` to be non-deterministic. This did not affect a topological iteration over the nodes when canonicalised by the `qargs` and `cargs`, but passes (such as Rust-space Sabre) that iterate over the edge structure themselves can observe the non-determinism. Sabre wants to iterate over the edges itself because, as a routing algorithm, it is implementing its own coroutine variant of topological ordering on a related graph.
jakelishman
added
Changelog: Bugfix
Pull Request Test Coverage Report for Build 16383505829Details
💛 - Coveralls |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of
HashMapandHashSetfor internal tracking structures in the pass, which were subsequently iterated over, caused node removals to happen in a non-deterministic order, which in turn could cause the edge structure of theDAGCircuitto be non-deterministic. This did not affect a topological iteration over the nodes when canonicalised by theqargsandcargs, but passes (such as Rust-space Sabre) that iterate over the edge structure themselves can observe the non-determinism.Sabre wants to iterate over the edges itself because, as a routing algorithm, it is implementing its own coroutine variant of topological ordering on a related graph.
Summary
Details and comments
No test currently, because how I write it depends on whether we take #14762 or not. I manually verified that this PR fixes the current determinism issue in the ASV benchmark
QUEKOTranspilerBench.track_depth_bss_optimal_depth_100.