10000 Pauli expansion protocol by viathor · Pull Request #1390 · quantumlib/Cirq · GitHub
[go: up one dir, main page]
Skip to content

Pauli expansion protocol #1390

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
Merged
merged 14 commits into from
Feb 27, 2019
Merged

Pauli expansion protocol #1390

merged 14 commits into from
Feb 27, 2019

Conversation

viathor
Copy link
Collaborator
@viathor viathor commented Feb 22, 2019

Logic for expanding an operator in the Pauli basis extracted from #1299 into a protocol.

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Feb 22, 2019
@viathor viathor requested a review from Strilanc February 22, 2019 01:35
Strilanc
Strilanc reviewed Feb 25, 2019
View reviewed changes
cirq/linalg/operator_spaces.py Outdated Show resolved Hide resolved
cirq/linalg/operator_spaces.py Outdated
}


def reconstruct_from_expansion(expansion: Dict[str, complex],
Copy link
Contributor

Choose a reason for hiding this comment

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

This name is not clear out of context. Perhaps matrix_from_basis_coefficients?

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. BTW: Do you think we should expose these names directly under cirq. package? They're currently under cirq.linalg..

Copy link
Contributor

Choose a reason for hiding this comment

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

I consider linalg to be one of the "flattened packages", so it should be exposed globally.

@viathor
Copy link
Collaborator Author
viathor commented Feb 26, 2019

This is ready for re-review. Coverage fails because protocol consistency checks have been disabled for parametrized code. They should pass once #1399 is merged here.

@viathor viathor added the Ready for Re-Review For when reviewers take their time. label Feb 26, 2019
Strilanc
Strilanc approved these changes Feb 26, 2019
View reviewed changes
Copy link
Contributor
@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

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

LGTM! Expose the methods to the global namespace (and get them from cirq. in tests as an 8000 assertion of that) then merge it.

@Strilanc
Copy link
Contributor
Strilanc commented Feb 26, 2019 •
edited
Loading

(Blocked on #1396)

@Strilanc Strilanc removed the Ready for Re-Review For when reviewers take their time. label Feb 27, 2019
@viathor viathor merged commit 7359de2 into quantumlib:master Feb 27, 2019
@viathor viathor deleted the pauli_expansion branch February 27, 2019 03:07
@viathor viathor mentioned this pull request Mar 14, 2019
Merged
CirqBot pushed a commit that referenced this pull request Mar 15, 2019
@viathor @CirqBot
Pauli expansion for Fredkin and Toffoli gates (#1441)
67d90ea 
Missed these gates in #1390.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Strilanc Strilanc Strilanc approved these changes

Assignees
No one assigned
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@viathor @Strilanc @googlebot
0