Skip to content

Conversation

@yinghui-hu
Copy link
Contributor

When the 3 qubits are all connected to each other, the decomposition can be further optimized #6068. Next step is to use all-to-all connectivity when decomposing circuits using IonQTargetGateset.

@yinghui-hu yinghui-hu requested review from a team, cduck and vtomole as code owners May 16, 2023 03:26
@yinghui-hu yinghui-hu requested a review from maffoo May 16, 2023 03:26
@CirqBot CirqBot added the size: M 50< lines changed <250 label May 16, 2023
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.

Please see my comment here - #6068 (comment)

Adding an alternative decomposition as done here is not enough because you would need a way to propagate the all_to_all_connect flag through cirq.decompose which is currently not supported.

@yinghui-hu yinghui-hu changed the title Add decompose for CCZ gate when qubits are all-to-all connected Add decomposition for CCZ gate and IonQTargetGateset when qubits are all-to-all connected May 23, 2023
@yinghui-hu
Copy link
Contributor Author

Done. Thanks for the guidance.

@yinghui-hu yinghui-hu requested a review from tanujkhattar May 23, 2023 05:20
@tanujkhattar tanujkhattar self-assigned this May 30, 2023
@yinghui-hu
Copy link
Contributor Author

Requested changes done. Please review again. Thanks. @tanujkhattar

@yinghui-hu
Copy link
Contributor Author

Done

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 % some final comments

)


def decompose_all_to_all_connect_ccz_gate(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this decomposition and corresponding test to cirq-ionq/cirq_ionq/ionq_gateset.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 223 to 224
"""If qubits are all-to-all connected, e.g. qubits in the same ion trap,
the decomposition will be:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Summary line in the docstring should be a single line with <= 100 characters followed by an empty newline and then a detailed description if needed. Please update here and elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Thanks for the guide.

CCXPowGate,
CCZ,
CCZPowGate,
decompose_all_to_all_connect_ccz_gate,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove from cirq/ops/__init__.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (14baaa5) 97.37% compared to head (ca5b113) 97.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6095      +/-   ##
==========================================
- Coverage   97.37%   97.37%   -0.01%     
==========================================
  Files        1116     1116              
  Lines       96042    96067      +25     
==========================================
+ Hits        93524    93545      +21     
- Misses       2518     2522       +4     
Files Changed Coverage Δ
cirq-ionq/cirq_ionq/__init__.py 100.00% <100.00%> (ø)
cirq-ionq/cirq_ionq/ionq_gateset.py 100.00% <100.00%> (ø)
cirq-ionq/cirq_ionq/ionq_gateset_test.py 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

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

@yinghui-hu
Copy link
Contributor Author

This PR is ready to merge @tanujkhattar Thanks!

@tanujkhattar tanujkhattar merged commit 081afab into quantumlib:master Aug 7, 2023
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