Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reduce non-determinism in UnitarySynthesis pass #13667

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

ElePT
Copy link
Contributor

@ElePT ElePT commented Jan 15, 2025

Summary

This PR proposes the use of an IndexMap in the Rust implementation of UnitarySynthesis to allow for a more deterministic output of the pass, and to ensure that the Python and Rust codes produce the same outputs when facing the same inputs. While this is not a guarantee we make, I believe it is good to have, makes testing much easier, and will smoothen the path for the transition to always using the target as a transpilation constraint "vehicle" (this PR fixes an issue found in a test in #12850).

Details and comments

@ElePT ElePT requested a review from a team as a code owner January 15, 2025 09:00
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@ElePT ElePT changed the title Remove non-determinism in UnitarySythesis pass Remove non-determinism in UnitarySynthesis pass Jan 15, 2025
@coveralls
Copy link

coveralls commented Jan 15, 2025

Pull Request Test Coverage Report for Build 12785721816

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 88.93%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 92.23%
Totals Coverage Status
Change from base Build 12771449472: 0.02%
Covered Lines: 79448
Relevant Lines: 89338

💛 - Coveralls

@ElePT ElePT changed the title Remove non-determinism in UnitarySynthesis pass Reduce non-determinism in UnitarySynthesis pass Jan 15, 2025
@ShellyGarion ShellyGarion added the Changelog: None Do not include in changelog label Jan 15, 2025
@ShellyGarion ShellyGarion added this pull request to the merge queue Jan 15, 2025
Merged via the queue into Qiskit:main with commit 994328a Jan 15, 2025
18 checks passed
@@ -627,8 +627,8 @@ fn get_2q_decomposers_from_target(
}

let target_basis_set = get_target_basis_set(target, qubits[0]);
let available_1q_basis: HashSet<&str> =
HashSet::from_iter(target_basis_set.get_bases().map(|basis| basis.as_str()));
let available_1q_basis: IndexSet<&str> =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use use the ahash hasher here otherwise this will cause a slowdown on lookups in the hash set.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants