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

Adds cirq.num_cnots_required and cirq.to_special #2892

Merged
merged 23 commits into from
Sep 29, 2020

Conversation

balopat
Copy link
Contributor

@balopat balopat commented Apr 6, 2020

Adds cirq.num_two_qubit_gates_required and cirq.to_special.

  • cirq.num_cnots_required: Based on simple linear algebra users can calculate the minimum required two-qubit gates (CZ, CNOT) to implement a two-qubit unitary.
  • cirq.to_special: converts a unitary to a special unitary

Context: this is the first PR breaking up #2873.

For an overview of the high level design decisions of the whole project see: https://drive.google.com/open?id=1SDEtttIAaTwfV9AUs7XAxeW3AUd0qLoY.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@balopat
Copy link
Contributor Author

balopat commented Apr 6, 2020

Argh, I pushed this from my personal computer, I'll fix the CLA soon.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Makes googlebot stop complaining. and removed cla: no labels Apr 6, 2020
@balopat balopat changed the base branch from feature-dev-three-qubit-decomposition to master July 10, 2020 13:59
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Sep 11, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Sep 11, 2020

Automerge cancelled: No approved review.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Sep 11, 2020
@balopat
Copy link
Contributor Author

balopat commented Sep 11, 2020

note - I'm just testing automerge

Copy link
Collaborator

@viathor viathor left a comment

Choose a reason for hiding this comment

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

Why is this in contrib and not in cirq.optimizers? The original bug suggests this is going to be very useful.

@balopat
Copy link
Contributor Author

balopat commented Sep 28, 2020

Why is this in contrib and not in cirq.optimizers? The original bug suggests this is going to be very useful.

@viathor, thank you for the thorough review - awesome suggestions!
I moved most of these functions into cirq packages, also updated the PR description to cater for them and fixed most of your comments. Let me know what you think!

@balopat balopat changed the title new module for 3 qubit decomp with _is_three_cnot_two_qubit_unitary Adds cirq.num_two_qubit_gates_required and cirq.to_special Sep 28, 2020
Copy link
Collaborator

@viathor viathor left a comment

Choose a reason for hiding this comment

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

Nice! LGTM with a few minor comments.

cirq/linalg/decompositions.py Outdated Show resolved Hide resolved
cirq/linalg/decompositions.py Show resolved Hide resolved
cirq/linalg/decompositions.py Outdated Show resolved Hide resolved
cirq/linalg/decompositions.py Outdated Show resolved Hide resolved
cirq/linalg/decompositions_test.py Outdated Show resolved Hide resolved
@balopat balopat changed the title Adds cirq.num_two_qubit_gates_required and cirq.to_special Adds cirq.num_cnots_required and cirq.to_special Sep 28, 2020
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Sep 28, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Sep 28, 2020
@CirqBot
Copy link
Collaborator

CirqBot commented Sep 28, 2020

Automerge cancelled: A required status check is not present.

Missing statuses: ['Build docs', 'Build protos', 'Changed files test', 'Coverage check', 'Doc test', 'Format check', 'Lint check', 'Misc check', 'Pytest MacOS (3.6)', 'Pytest MacOS (3.7)', 'Pytest Ubuntu (3.6)', 'Pytest Ubuntu (3.7)', 'Pytest Windows (3.6)', 'Pytest Windows (3.7)', 'Type check']

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Sep 28, 2020
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Sep 29, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Sep 29, 2020
@CirqBot CirqBot merged commit fb164bb into quantumlib:master Sep 29, 2020
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Sep 29, 2020
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.

None yet

4 participants