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

Initial draft of ConvertToSycamore #2516

Merged
merged 27 commits into from
Nov 22, 2019

Conversation

dstrain115
Copy link
Collaborator

  • Port of ConvertToSycamore from cirq_internal
  • Cleaned up format, mypy, deprecated commands
  • Refactored various functions to try to clean them up.
  • Got rid of half-pi only rotations

- Port of ConvertToSycamore from cirq_internal
- Cleaned up format, mypy, deprecated commands
- Refactored various functions to try to clean them up.
- Got rid of half-pi only rotations
@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Nov 8, 2019
yield (gate(q1) for gate in optimizers.single_qubit_matrix_to_gates(a_1))


def zztheta(theta: float, q0: ops.Qid, q1: ops.Qid) -> ops.OP_TREE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer the name rzz.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

yield ops.Rz(theta / 2).on(q1)


def swap_zztheta(theta: float, q0: ops.Qid, q1: ops.Qid) -> ops.OP_TREE:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer the name swap_rzz.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@dstrain115
Copy link
Collaborator Author

Ping. Could I get a review of this? I would like to get this committed before the next import of google3 and to move away from the need for cirq_internal. Thanks.

Copy link
Collaborator

@mpharrigan mpharrigan left a comment

Choose a reason for hiding this comment

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

Looks fine, as this is a port of code we've been using successfully internally.

I don't like the name textbook_gates_from_sycamore and think it should just be merged into convert_to_sycamore_gates. Maybe open a low-prio follow-up ticket?

@dstrain115
Copy link
Collaborator Author

I don't like the name textbook_gates_from_sycamore and think it should just be merged into convert_to_sycamore_gates. Maybe open a low-prio follow-up ticket?

Done. Also cleaned up a bunch of tests to make them parameterized and got rid of a bunch of unnecessary cruft.

Want to take another look before I submit?

@mpharrigan
Copy link
Collaborator

Sorry to be a diva, but is there any way we could keep some (most) of the functions as free functions (i.e. not methods on a class)? They're really helpful atoms for crafting one's own optimizer if one is not happy with how ConvertToSycamore messes up all the moments xref #2553 #2406 #2461

At present, the only usage of self is to call other methods which otherwise don't use self

@dstrain115
Copy link
Collaborator Author

Sorry to be a diva, but is there any way we could keep some (most) of the functions as free functions (i.e. not methods on a class)? They're really helpful atoms for crafting one's own optimizer if one is not happy with how ConvertToSycamore messes up all the moments xref #2553 #2406 #2461

I have changed them all to class methods or staticmethods. That way you can call them without instantiating the class. That to me seems better than having a bunch of loose functions hanging around. What do you think?

@dstrain115 dstrain115 merged commit 2963285 into quantumlib:master Nov 22, 2019
@dstrain115 dstrain115 deleted the convert_to_sycamore branch November 22, 2019 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants