Add internal direct 2q synthesis interface to UnitarySynthesis#15845
Open
mtreinish wants to merge 1 commit intoQiskit:mainfrom
Open
Add internal direct 2q synthesis interface to UnitarySynthesis#15845mtreinish wants to merge 1 commit intoQiskit:mainfrom
mtreinish wants to merge 1 commit intoQiskit:mainfrom
Conversation
This commit adds an internal dedicated function to the UnitarySynthesis function to enable other passes that are synthesizes unitaries to reuse the logic from UnitarySynthesis. We're looking at adding new passes that will do 2q synthesized to a target, specifically the two qubit peephole optimization pass being added in Qiskit#13419. The new interface in this PR lets that pass (and potential future passes) reuse the logic from unitary synthesis without the rest of the infrastructure. This was originally part of Qiskit#13419 but this opts to split it out since it's logically independent from the PR adding the new pass.
mtreinish
added
Changelog: None
Collaborator
|
One or more of the following people are relevant to this code:
|
Member
alexanderivrii
left a comment
alexanderivrii
left a comment
There was a problem hiding this comment.
LGTM, just a clarification question about return values.
Comment on lines
+527
to
+532
| pub fn synthesize_2q_matrix( | ||
| mut unitary: CowArray<Complex64, Ix2>, | ||
| qargs_phys: [PhysicalQubit; 2], | ||
| qargs_virt: [Qubit; 2], | ||
| state: &mut UnitarySynthesisState, | ||
| constraint: QpuConstraint, | ||
| ) -> PyResult<bool> { | ||
| ) -> PyResult<Option<TwoQSynthesisResult>> { |
Member
There was a problem hiding this comment.
Since this function is now meant to be used in other files, could you pease add a short docstring or a comment in which case this returns Ok(None) vs Err.
Comment on lines
+618
to
+620
| let Some(result) = synthesize_2q_matrix(unitary, qargs_phys, state, constraint)? else { | ||
| return Ok(false); | ||
| }; |
Member
There was a problem hiding this comment.
Why is this Ok(false) and not Err?
| } | ||
|
|
||
| #[inline] | ||
| pub fn fidelity_2q_sequence( |
Member
There was a problem hiding this comment.
is it possible to add a docstring to this function?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary
This commit adds an internal dedicated function to the UnitarySynthesis function to enable other passes that are synthesizes unitaries to reuse the logic from UnitarySynthesis. We're looking at adding new passes that will do 2q synthesized to a target, specifically the two qubit peephole optimization pass being added in #13419. The new interface in this PR lets that pass (and potential future passes) reuse the logic from unitary synthesis without the rest of the infrastructure. This was originally part of #13419 but this opts to split it out since it's logically independent from the PR adding the new pass.
Details and comments