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

move SU2RotationGate to basic_gates/ #802

Merged
merged 20 commits into from
Apr 1, 2024

Conversation

anurudhp
Copy link
Contributor

@anurudhp anurudhp commented Mar 18, 2024

#669 (comment)

  • Move SU2RotationGate to basic_gates/ directory
  • Add a builder method .from_matrix so you can derive the parameters of the gate given an arbitrary 2-qubit unitary.
  • override wire_symbol
  • test that bloq_autotester works.
  • Add a docstring (maybe just move the docstring from rotation_matrix method to the class)
  • Add a BloqExample
  • Update and run dev_tools/autogenerate-bloqs-notebooks.py to generate a doc
  • Make sure theta, phi and lambd can be symbolics.

Skipped (due to global phase mismatch)

  • override as_cirq_op to return a PhasedXZGate
  • override CIRQ_GATE_TO_BLOQ_MAP in _cirq_gate_to_bloq to convert a cirq.PhasedXZGate to SU2RotationGate
  • test that assert_decompose_is_consistent_with_t_complexity works
  • Override build_call_graph

@anurudhp anurudhp force-pushed the refactor-SU2-gate branch from ce193f2 to bc7519d Compare March 18, 2024 23:29
@anurudhp
Copy link
Contributor Author

@tanujkhattar one question: cirq.PhasedXZGate and SU2RotationGate differ by a global phase, i.e. $SU_2(\theta, \phi, \lambda) = e^{i(\phi+\lambda-\theta)} \text{PhasedXZ}(x=\frac{2\theta}{\pi}, z=1-\frac{\lambda+\phi}\pi, a=\frac{\lambda}\pi - 0.5)$. (this only matters when using controlled GSQP inside a larger algorithm).

Should we still map them ignoring global phase? Another idea could be to just used PhasedXZ in GQSP rotations, and apply a single global phase correction at the end.

@tanujkhattar tanujkhattar marked this pull request as ready for review March 19, 2024 16:59
@anurudhp anurudhp force-pushed the refactor-SU2-gate branch from bc7519d to c681d85 Compare March 21, 2024 16:41
@mpharrigan
Copy link
Collaborator

What was the conclusion from the meeting? Were we going to remove the decomposition and leave this as an "atomic" bloq? And add its tensor. Also: re: global phase: we said that it needs to match, so if cirq.PhasedXZ isn't up to the task we can leave that out.

@anurudhp anurudhp force-pushed the refactor-SU2-gate branch from c681d85 to bd1d5c9 Compare March 22, 2024 15:25
@anurudhp
Copy link
Contributor Author

@mpharrigan I dropped the PhasedXZ conversion.

About the decomposition, I am fine with either case. Perhaps its still useful to have a decomposition in terms of Pauli rotations in case someone wants to generate circuits?

@anurudhp anurudhp force-pushed the refactor-SU2-gate branch 2 times, most recently from bf98db0 to e895a93 Compare March 23, 2024 18:04
@anurudhp
Copy link
Contributor Author

@tanujkhattar @mpharrigan could one of you review this PR? thanks!

I also have two questions:

  • Should I make the bloq atomic?
  • Is there anything specific to be done to support symbolic params?

Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

Answered all your questions and requested a few changes. Looking great overall.

Should I make the bloq atomic?

No

Is there anything specific to be done to support symbolic params?

Suggested minor improvements

qualtran/bloqs/basic_gates/su2_rotation.py Show resolved Hide resolved
qualtran/bloqs/basic_gates/su2_rotation.ipynb Show resolved Hide resolved
qualtran/bloqs/basic_gates/su2_rotation.py Outdated Show resolved Hide resolved
qualtran/bloqs/basic_gates/su2_rotation.py Show resolved Hide resolved
qualtran/bloqs/basic_gates/su2_rotation.py Outdated Show resolved Hide resolved
@anurudhp anurudhp force-pushed the refactor-SU2-gate branch 3 times, most recently from 402acc5 to 47d2ba5 Compare March 28, 2024 17:48
@anurudhp anurudhp force-pushed the refactor-SU2-gate branch from 47d2ba5 to edd3f14 Compare March 28, 2024 21:18
@anurudhp anurudhp requested a review from tanujkhattar March 31, 2024 11:07
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % one final suggestion to update the _cirq_gate_to_bloq to return GlobalPhase bloq when it sees a cirq.GlobalPhaseGate.

Feel free to merge after that.

@anurudhp anurudhp force-pushed the refactor-SU2-gate branch from 5e9a40b to 3a0d092 Compare April 1, 2024 09:51
@anurudhp
Copy link
Contributor Author

anurudhp commented Apr 1, 2024

_cirq_gate_to_bloq already converts to GlobalPhase (merged with #828)

if isinstance(gate, cirq.GlobalPhaseGate):
return GlobalPhase(coefficient=gate.coefficient)

@tanujkhattar tanujkhattar merged commit c77e59d into quantumlib:main Apr 1, 2024
6 checks passed
@anurudhp anurudhp deleted the refactor-SU2-gate branch April 1, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants