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.

Sign up for GitHub

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

Jump to bottom

Bring GateTabulation out of cirq-google to cirq-core #4461

Closed
Ashalynd opened this issue Aug 24, 2021 · 6 comments · Fixed by #4602
Closed

Bring GateTabulation out of cirq-google to cirq-core #4461

Ashalynd opened this issue Aug 24, 2021 · 6 comments · Fixed by #4602
Assignees
@Ashalynd
Labels
area/gate-compilation good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/task A task that's part of a larger effort triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on triage/discuss Needs decision / discussion, bring these up during Cirq Cynque

Comments

@Ashalynd
Copy link
Collaborator

As discussed with @tanujkhattar in #4362, creating a separate issue to track GateTabulation refactoring.
@Ashalynd Ashalynd added the kind/feature-request Describes new functionality label Aug 24, 2021
@Ashalynd Ashalynd mentioned this issue Aug 24, 2021
Open
@Ashalynd Ashalynd self-assigned this Aug 24, 2021
@tanujkhattar tanujkhattar added area/gate-compilation good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. labels Aug 24, 2021
@viathor viathor added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Aug 25, 2021
@MichaelBroughton MichaelBroughton added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Aug 25, 2021
@tanujkhattar tanujkhattar added kind/task A task that's part of a larger effort and removed kind/feature-request Describes new functionality labels Aug 25, 2021
@tanujkhattar
Copy link
Collaborator

@Ashalynd As part of tracking the progress towards our Cirq 1.0 roadmap items, I wanted to check in on the progress on this issue. Could you start working on it yet?

@Ashalynd
Copy link
Collaborator Author

Ashalynd commented Oct 4, 2021

Hi Tanuj, I had to put it on hold for a few weeks due to 80% work. Expect to create a PR this week.

@Ashalynd
Copy link
Collaborator Author

Hey, a quick update: started working on it, fighting with a couple tests :)

@Ashalynd
Copy link
Collaborator Author

Question: does it make sense to also move the code that is using GateTabulation now?

@tanujkhattar
Copy link
Collaborator

Optimizers like ConvertToSycamoreGates should stay in cirq_google and use cirq.GateTabulation object. So no, the google specific code using GateTabulation shouldn't be moved to cirq-core.

Ashalynd added a commit to Ashalynd/Cirq that referenced this issue Oct 27, 2021
@Ashalynd
Moved GateTabulation and friends to cirq-core as requested in quantum…
8d7f5b6 
…lib#4461
@tanujkhattar
Copy link
Collaborator

There are some nuances in moving the code to cirq-core, mainly around dealing with two_qubit_gates/math_utils.py. Ideally, we'd like to move / re-implement the methods present in math_utils.py to more appropriate locations in cirq-core. For example,

  • random_qubit_unitary : Replace usages with cirq.testing.linalg_utils.random_unitary
  • kak_vector_infidelity, in_weyl_chamber, weyl_chamber_mesh: Move to linalg.decompositions.py ?
  • unitary_entanglement_fidelity : Move to qis.measures.py ?

One difference in methods implemented in math_utils.py vs similar / existing methods in cirq-core is that most of the methods in math_utils.py are vectorized for speed. We should discuss the tradeoffs between consistency of the API with other similar methods and importance of vectorization for this use case. There are 3 options:

  • Move math_utils.py as it is and rename to gate_tabulation_math_utils.py.
  • Move out the vectorized methods out to different locations as it is maintaining the existing vectorized API.
  • Move out the methods out to different locations with APIs consistent with existing cirq code and adopt the GateTabulation code according to the new API.

Thoughts? cc @viathor @dabacon @maffoo
xref : #4722

@tanujkhattar tanujkhattar added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Dec 9, 2021
This was referenced Dec 13, 2021
Closed
Merged
tanujkhattar pushed a commit that referenced this issue Jan 10, 2022
@Ashalynd
Move GateTabulation and friends to cirq-core as requested in #4461 (#…
778d317 
…4602)

- Adds `cirq. TwoQubitGateTabulation`, `cirq.TwoQubitGateTabulationResult`, and `cirq.two_qubit_gate_product_tabulation` public top level objects to Cirq. 
- TwoQubitGateTabulation is heuristic compilation technique for decomposing any 2q unitary to a given target 2q gate + single qubit rotations.
MichaelBroughton pushed a commit to MichaelBroughton/Cirq that referenced this issue Jan 22, 2022
@Ashalynd @MichaelBroughton
Move GateTabulation and friends to cirq-core as requested in quantuml…
70e492a 
…ib#4461 (quantumlib#4602)

- Adds `cirq. TwoQubitGateTabulation`, `cirq.TwoQubitGateTabulationResult`, and `cirq.two_qubit_gate_product_tabulation` public top level objects to Cirq. 
- TwoQubitGateTabulation is heuristic compilation technique for decomposing any 2q unitary to a given target 2q gate + single qubit rotations.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
@Ashalynd @rht
Move GateTabulation and friends to cirq-core as requested in quantuml…
3997175 
…ib#4461 (quantumlib#4602)

- Adds `cirq. TwoQubitGateTabulation`, `cirq.TwoQubitGateTabulationResult`, and `cirq.two_qubit_gate_product_tabulation` public top level objects to Cirq. 
- TwoQubitGateTabulation is heuristic compilation technique for decomposing any 2q unitary to a given target 2q gate + single qubit rotations.
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this issue Oct 31, 2024
@Ashalynd
Move GateTabulation and friends to cirq-core as requested in quantuml…
1f3b502 
…ib#4461 (quantumlib#4602)

- Adds `cirq. TwoQubitGateTabulation`, `cirq.TwoQubitGateTabulationResult`, and `cirq.two_qubit_gate_product_tabulation` public top level objects to Cirq. 
- TwoQubitGateTabulation is heuristic compilation technique for decomposing any 2q unitary to a given target 2q gate + single qubit rotations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Ashalynd Ashalynd

Labels
area/gate-compilation good first issue This issue can be resolved by someone who is not familiar with the codebase. A good starting issue. kind/task A task that's part of a larger effort triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on triage/discuss Needs decision / discussion, bring these up during Cirq Cynque
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Move GateTabulation and friends to cirq-core as requested in #4461 Ashalynd/Cirq
4 participants
@Ashalynd @MichaelBroughton @tanujkhattar @viathor