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

Extend Clifford protocol for 2+ qubit operations through analytical check and falling back to decompose #6332

Merged
merged 9 commits into from
Oct 30, 2023

Conversation

NoureldinYosri
Copy link
Collaborator

@NoureldinYosri NoureldinYosri commented Oct 30, 2023

This implements the check from https://quantumcomputing.stackexchange.com/a/13158 when the unitary is small enough (acting on 3 or less qubits)

Otherwise it falls back to decompose

part of #5906

@CirqBot CirqBot added the size: M 50< lines changed <250 label Oct 30, 2023
@NoureldinYosri NoureldinYosri marked this pull request as ready for review October 30, 2023 21:45
@NoureldinYosri NoureldinYosri requested review from vtomole, cduck and a team as code owners October 30, 2023 21:45
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.

Looking good overall. Left a couple of nits. Thanks!

if qid_shape is None or len(qid_shape) != 1 or not protocols.has_unitary(val):
# qubits is greater than 3 since that would be expensive.
qid_shape = qid_shape_protocol.qid_shape(val, default=None)
if qid_shape is None or len(qid_shape) > 3 or not has_unitary_protocol.has_unitary(val):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we insead check that qid_shape is either (2,) or (2, 2) ? For example, what happens if the operation is defined on 2 or less qudits with dimension greater than 2?

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

res = has_stabilizer_effect(op)
if not res:
return res
return res
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we return True here instead of res ? res is defined inside the loop and used outside, which is confusing for the reader.

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. good catch

@@ -125,9 +129,20 @@ def test_via_unitary():
op3 = OpWithUnitary(np.array([[1, 0], [0, np.sqrt(1j)]]))
assert not cirq.has_stabilizer_effect(op3)

# 2+ qubit cliffords
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# 2+ qubit cliffords
# 2 qubit cliffords

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it works up to 3 qubit operations like CCNOTs

Copy link
Collaborator

Choose a reason for hiding this comment

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

The 3 asserts for cliffords are only for 2 qubit operations, but lgtm

Copy link
Collaborator Author

@NoureldinYosri NoureldinYosri left a comment

Choose a reason for hiding this comment

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

@tanujkhattar PTAL

now that has_stabilizer_effect is more general we need to limit is use in t_complexity protocol by calling only on single and two qubit operations

@@ -125,9 +129,20 @@ def test_via_unitary():
op3 = OpWithUnitary(np.array([[1, 0], [0, np.sqrt(1j)]]))
assert not cirq.has_stabilizer_effect(op3)

# 2+ qubit cliffords
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it works up to 3 qubit operations like CCNOTs

res = has_stabilizer_effect(op)
if not res:
return res
return res
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. good catch

if qid_shape is None or len(qid_shape) != 1 or not protocols.has_unitary(val):
# qubits is greater than 3 since that would be expensive.
qid_shape = qid_shape_protocol.qid_shape(val, default=None)
if qid_shape is None or len(qid_shape) > 3 or not has_unitary_protocol.has_unitary(val):
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

@NoureldinYosri NoureldinYosri enabled auto-merge (squash) October 30, 2023 22:21
Comment on lines +84 to 86
if cirq.num_qubits(stc) <= 2 and cirq.has_stabilizer_effect(stc):
# Clifford operation.
return TComplexity(clifford=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this change; it also makes reporting the TComplexity more accurate since we won't count large cliffords as a single clifford operation.

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!

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1eced48) 97.89% compared to head (d804974) 97.83%.
Report is 2 commits behind head on master.

❗ Current head d804974 differs from pull request most recent head b38ca39. Consider uploading reports for the commit b38ca39 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6332      +/-   ##
==========================================
- Coverage   97.89%   97.83%   -0.07%     
==========================================
  Files        1108     1108              
  Lines       96204    96240      +36     
==========================================
- Hits        94182    94158      -24     
- Misses       2022     2082      +60     
Files Coverage Δ
...e/cirq/protocols/has_stabilizer_effect_protocol.py 100.00% <100.00%> (ø)
...q/protocols/has_stabilizer_effect_protocol_test.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/algos/qrom_test.py 100.00% <ø> (ø)
...t/cirq_ft/algos/qubitization_walk_operator_test.py 69.62% <100.00%> (-30.38%) ⬇️
...-ft/cirq_ft/algos/reflection_using_prepare_test.py 61.64% <100.00%> (-38.36%) ⬇️
cirq-ft/cirq_ft/algos/select_swap_qrom_test.py 100.00% <100.00%> (ø)
...ft/cirq_ft/algos/selected_majorana_fermion_test.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/algos/state_preparation_test.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/algos/unary_iteration_gate_test.py 100.00% <100.00%> (ø)
cirq-ft/cirq_ft/infra/t_complexity_protocol.py 97.77% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NoureldinYosri NoureldinYosri merged commit 34e8dab into master Oct 30, 2023
@NoureldinYosri NoureldinYosri deleted the clifford_check branch October 30, 2023 23:22
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants