Qcut sampling template expansion #2399
Conversation
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
…ithub.com:PennyLaneAI/pennylane into qcut-sample-subgraphs-to-fragment-tape-conversion
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
…I/pennylane into qcut-cut-circuit-mc-transform
anthayes92
changed the base branch from
master
to
qcut-cut-circuit-mc-transform
|
Hello. You may have forgotten to update the changelog!
|
Codecov Report
@@ Coverage Diff @@
## master #2399 +/- ##
=======================================
Coverage 99.45% 99.45%
=======================================
Files 244 244
Lines 18971 18976 +5
=======================================
+ Hits 18867 18872 +5
Misses 104 104
Continue to review full report at Codecov.
|
|
[sc-16859] |
pennylane/transforms/qcut.py
Outdated
| def _qcut_expand_fn_mc( | ||
| tape: QuantumTape, | ||
| shots: Optional[int] = None, | ||
| device_wires: Optional[Wires] = None, | ||
| classical_processing_fn: Optional[callable] = None, | ||
| max_depth: int = 1, | ||
| ): | ||
| """Expansion function for sample-based circuit cutting. | ||
|
|
||
| Expands operations until reaching a depth that includes :class:`~.WireCut` operations. | ||
| """ | ||
| # pylint: disable=unused-argument | ||
| for op in tape.operations: | ||
| if isinstance(op, WireCut): | ||
| return tape | ||
|
|
||
| if max_depth > 0: | ||
| return cut_circuit_mc.expand_fn(tape.expand(), max_depth=max_depth - 1) | ||
|
|
||
| raise ValueError( | ||
| "No WireCut operations found in the circuit. Consider increasing the max_depth value if " | ||
| "operations or nested tapes contain WireCut operations." | ||
| ) | ||
|
|
||
|
|
||
| cut_circuit_mc.expand_fn = _qcut_expand_fn_mc | ||
|
|
||
|
|
There was a problem hiding this comment.
This works but I'm not sure if it makes sense to have _qcut_expand_fn and _qcut_expand_fn_mc be the same function with repeated code. How about:
def _qcut_expand_fn(
tape: QuantumTape,
max_depth: int = 1,
):
"""Expansion function for sample-based circuit cutting.
Expands operations until reaching a depth that includes :class:`~.WireCut` operations.
"""
for op in tape.operations:
if isinstance(op, WireCut):
return tape
if max_depth > 0:
return _qcut_expand_fn(tape.expand(), max_depth=max_depth - 1)
raise ValueError(
"No WireCut operations found in the circuit. Consider increasing the max_depth value if "
"operations or nested tapes contain WireCut operations."
)
def cut_circuit_expand(
tape: QuantumTape,
use_opt_einsum: bool = False,
device_wires: Optional[Wires] = None,
max_depth: int = 1,
):
# pylint: disable=unused-argument
return _qcut_expand_fn(tape, max_depth)
def cut_circuit_mc_expand(
tape: QuantumTape,
shots: Optional[int] = None,
device_wires: Optional[Wires] = None,
classical_processing_fn: Optional[callable] = None,
max_depth: int = 1,
):
# pylint: disable=unused-argument
return _qcut_expand_fn(tape, max_depth)
cut_circuit.expand_fn = cut_circuit_expand
cut_circuit_mc.expand_fn = cut_circuit_mc_expandI'm not sure it'll be much shorter, but it seems cleaner for both expansions to be using the same core function (reducing code duplication).
That being said, the current approach works well if we expect cut_circuit and cut_circuit_mc expansions to ever behave differently 🤔
There was a problem hiding this comment.
Thanks @anthayes92, looks good overall but I've just requested one change to the tests.
tests/transforms/test_qcut.py
Outdated
|
|
||
| assert spy.call_count == 2 | ||
| assert spy.call_count == 1 |
There was a problem hiding this comment.
We should still be calling the expansion functionality twice. The issue is that we'll have to have one spy for cut_transform.expand_fn and another for qcut._qcut_expand_fn, and assert that each is called once.
Before, we had
return cut_circuit.expand_fn(tape.expand(), max_depth=max_depth - 1)
in line 1914 which meant we could get away with just one spy.
There was a problem hiding this comment.
This is related to the discussion we had in the previous PR.
tests/transforms/test_qcut.py
Outdated
| @@ -3697,10 +3731,37 @@ def circuit(template_weights): | |||
|
|
|||
| spy = mocker.spy(qcut.cut_circuit, "expand_fn") | |||
| res = qnode_cut(template_weights) | |||
| assert spy.call_count == 2 | |||
| assert spy.call_count == 1 | |||
There was a problem hiding this comment.
Also here and in line 3763 of test_expansion_mc_ttn - we expand the expansion to be happening twice because we have a nested block.
There was a problem hiding this comment.
Thanks @anthayes92! Approved, but worth you taking a look through at the changes too.
Context:
A circuit containing sample measurements can be cut using
cut_circuit_mc. Here we add an expansion function to the transform that allows cutting support for circuits that contain WireCut operations nested in operations or sub-tapes.Similar to the expansion function added to
cut_circuitin #2340