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

Add functionalty for determining whether pairs of moments commute #6679

Closed

Conversation

cdbf1
Copy link

@cdbf1 cdbf1 commented Jul 26, 2024

Currently it is not possible to determine if pairs of moments can commute which may be useful for when collections of operations commute as a group but not individually

For instance a pair of single qubit Z gates commute with a two qubit RXX gate together but not separately.

This PR fixes this challenge by decomposing two moments into operations that act on disjoint sets of qubits and then testing whether their unitary representations on these sets of qubits commute.

Closes #6659

Copy link

google-cla bot commented Jul 26, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@CirqBot CirqBot added the size: M 50< lines changed <250 label Jul 26, 2024
@cdbf1 cdbf1 marked this pull request as ready for review July 26, 2024 11:12
@cdbf1 cdbf1 requested review from vtomole and a team as code owners July 26, 2024 11:12
@cdbf1 cdbf1 requested a review from dabacon July 26, 2024 11:12
Copy link

codecov bot commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.82%. Comparing base (6e19d0d) to head (acebb77).
Report is 13 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6679   +/-   ##
=======================================
  Coverage   97.82%   97.82%           
=======================================
  Files        1072     1072           
  Lines       92047    92075   +28     
=======================================
+ Hits        90045    90074   +29     
+ Misses       2002     2001    -1     

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

@NoureldinYosri NoureldinYosri self-assigned this Aug 15, 2024
Copy link
Collaborator

@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.

sorry of the late review

# Decompose both moments onto each disjoint set of qubits and
# check for commutation using the unitary representation
if all(
definitely_commutes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

would this code raise an error if the operations are not unitary?

@@ -669,25 +676,92 @@ def _commutes_(self, other: Any, *, atol: float = 1e-8) -> Union[bool, NotImplem

Returns:
True: The Moment and Operation commute OR they don't have shared
quibits.
qubits.
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you fix the formatting?

# Decompose into disjoint overlapping sets of qubits
qubit_subsets = [list(op.qubits) for op in self.operations + other.operations]
disjoint_set = DisjointSet(itertools.chain.from_iterable(qubit_subsets))
for subset in qubit_subsets:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if a qubit appears in both moments in operations with different qubits?
so in m1: op1 acting on (q1, q0) in m2 op2 acting on q3, q4, q0)

if isinstance(other, ops.Operation):
# If an Operation is provided, convert this to a Moment consisting only
# of the given Operation
return self._commutes_(Moment(other), atol=atol)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not preserve the old code? ... perhaps in a def _commutes_with_op

@@ -693,6 +693,18 @@ def test_commutes():
assert not cirq.commutes(moment, cirq.X(c))


def test_commutes_multiqubit_gates():
a = cirq.NamedQubit('a')
Copy link
Collaborator

Choose a reason for hiding this comment

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

this test case is very simple and doesn't cover the more complex cases ... see above

@github-actions github-actions bot added the Stale label Sep 15, 2024
@github-actions github-actions bot closed this Oct 16, 2024
@mhucka mhucka added the status/stale This has been closed due to inactivity for an extended period of time. label Nov 18, 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 status/stale This has been closed due to inactivity for an extended period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue when commuting moments
4 participants