-
Notifications
You must be signed in to change notification settings - Fork 586
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
Circuit cutting: Remove circuit cuts that do not result in a disconnection #2260
Conversation
…pennylane into qcut_remove_unneeded_subgraphs
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2260 +/- ##
=======================================
Coverage 99.31% 99.31%
=======================================
Files 237 237
Lines 18899 18902 +3
=======================================
+ Hits 18769 18772 +3
Misses 130 130
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Tom, had a quick look here and seems good to go! Thanks for dealing with all these edge cases. I just have a couple questions below.
pennylane/transforms/qcut.py
Outdated
# to worry about adding back an edge between the predecessor to node1 and the successor | ||
# to node2 because edge connectivity no longer matters in the subgraphs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, maybe this comment doesn't need to change but I'm personally not sure what "edge connectivity no longer matters" means here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I clarified the comment here: b358d1d
The idea is that after fragmenting we are happy to convert back to a tape for further processing. Going from graph to tape only requires looking at the nodes and the total order (which is a node attribute), so we don't care about edges in this conversion.
@@ -2095,7 +2112,7 @@ def circuit(x): | |||
res = circuit(x) | |||
assert np.allclose(res, np.cos(x)) | |||
assert len(spy.call_args[0][0]) == 1 # there should be 2 tensors for wire 0 | |||
assert spy.call_args[0][0][0].shape == (4, 4) | |||
assert spy.call_args[0][0][0].shape == () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this have to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The circuit in this test actually has a fully contained cut. Beforehand, we were going over all measurement and preparation configurations even though the cut didn't result in a disconnection. So we'd have a (4, 4)
tensor with contraction equation a,a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @angusjlowe!
pennylane/transforms/qcut.py
Outdated
# to worry about adding back an edge between the predecessor to node1 and the successor | ||
# to node2 because edge connectivity no longer matters in the subgraphs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I clarified the comment here: b358d1d
The idea is that after fragmenting we are happy to convert back to a tape for further processing. Going from graph to tape only requires looking at the nodes and the total order (which is a node attribute), so we don't care about edges in this conversion.
@@ -2095,7 +2112,7 @@ def circuit(x): | |||
res = circuit(x) | |||
assert np.allclose(res, np.cos(x)) | |||
assert len(spy.call_args[0][0]) == 1 # there should be 2 tensors for wire 0 | |||
assert spy.call_args[0][0][0].shape == (4, 4) | |||
assert spy.call_args[0][0][0].shape == () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The circuit in this test actually has a fully contained cut. Beforehand, we were going over all measurement and preparation configurations even though the cut didn't result in a disconnection. So we'd have a (4, 4)
tensor with contraction equation a,a
Context:
Not all place
WireCut
operations lead to a disconnection between a pair of subgraphs. Sometimes aWireCut
will live within a subgraph. For example, in this circuit:This PR removes the
MeasureNode
andPrepareNode
pairs resulting fromWireCut
s that do not lead to a disconnection.