Skip to content
New issue

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

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

Already on GitHub? Sign in 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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