-
Notifications
You must be signed in to change notification settings - Fork 706
feat: add MCM-based decomposition for Adjoint(TemporaryAND)
#8633
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
base: decomp-mcm
Are you sure you want to change the base?
feat: add MCM-based decomposition for Adjoint(TemporaryAND)
#8633
Conversation
|
Thank you for your contribution! As you've noticed, MCMs are still not fully supported in decompositions in master, but maybe you can rebase your PR onto #8079 in which I've added support for MCMs and conditionals in decompositions. We're hopefully getting that PR merged some time this or next week, and your PR would be able to follow. |
|
|
||
|
|
||
| def _adjoint_temporary_and_resources(*, base_class=None, base_params=None, **__): | ||
| return {ops.Hadamard: 1, ops.MidMeasure: 1, ops.Conditional: 1} |
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.
When you have a conditional operator, you would only declare the actual operator being conditionally applied (in this case, the CZ), not the Conditional.
comp-phys-marc
left a comment
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 for the interesting PR! It is great that you have followed our patterns. Just some initial feedback.
| assert qml.math.allclose( | ||
| probs[idx], 1.0 | ||
| ), f"Failed for a={a}, b={b}, cv={control_values}" | ||
| mask = qml.math.ones_like(probs, dtype=bool) |
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.
Clever way to assert that all the other probabilities are zero. 👍
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.
Do we need to do that, given that we already have identified one entry of probs with probability 1? 😅
Could the probs possibly not be normalized?
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 think based on the docs that the probabilities will be normalized.
| # TODO: add add_decomps("Adjoint(TemporaryAND)", _adjoint_TemporaryAND) when MCMs supported by the pipeline | ||
|
|
||
|
|
||
| def _adjoint_temporary_and_resources(*, base_class=None, base_params=None, **__): |
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.
| def _adjoint_temporary_and_resources(*, base_class=None, base_params=None, **__): | |
| def _adjoint_temporary_and_resources(*, base_class=None, base_params=None, **__): # pylint: disable=unused-argument |
|
|
||
|
|
||
| @register_resources(_adjoint_temporary_and_resources) | ||
| def _adjoint_TemporaryAND(wires: WiresLike, **kwargs): |
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.
| def _adjoint_TemporaryAND(wires: WiresLike, **kwargs): | |
| def _adjoint_TemporaryAND(wires: WiresLike, **kwargs): # pylint: disable=unused-argument |
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 decomposition rule _temporary_and has these keywords arguments, so I followed the pattern. If not necessary this can be removed.
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.
Yes, I noticed you were following the pattern. That is perfectly fine. In that case however, we need these pylint directives.
dwierichs
left a comment
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 for flooding you with reviews, @LiukDiihMieu , and thanks a lot for this contribution 🎉 :)
Just leaving three small comments.
| Validate the MCM-based decomposition of Adjoint(TemporaryAND). | ||
| """ | ||
| sys_wires = [0, 1, 2] | ||
| work_wires = [3] # ancilla for deferred measure |
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.
| work_wires = [3] # ancilla for deferred measure | |
| work_wires = [3] # auxiliary qubit for deferred measure |
| assert qml.math.allclose( | ||
| probs[idx], 1.0 | ||
| ), f"Failed for a={a}, b={b}, cv={control_values}" | ||
| mask = qml.math.ones_like(probs, dtype=bool) |
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.
Do we need to do that, given that we already have identified one entry of probs with probability 1? 😅
Could the probs possibly not be normalized?
| rules = qml.list_decomps("Adjoint(TemporaryAND)") | ||
| assert rules, "No decomposition rules registered for Adjoint(TemporaryAND)" | ||
| rule = rules[0] |
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 think it might be more stable to import the decomposition function itself in the test file and use it here directly.
Unfortunately, we currently don't have a neat way to point at a specific decomposition rule, as you probably noticed 😬
The current approach could accidentally switch to testing something other than the intended decomposition, if that other decomposition is added to the decomps first 🙃
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.
Could you please let me know how to import the decompose function properly?
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 think the following should work :)
from pennylane.templates.subroutines.arithmetic.temporary_and import _adjoint_TemporaryANDand they you can use it in place of rule :)
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.
Alternatively, you could also iterate over all the rules, as the test is not super specific to the decomposition rule, but any decomposition of Adjoint(TemporaryAND) must pass this test :)
7125fea to
6a008b9
Compare
|
Thank you all for the comments. I have rebased this branch and have made changes according to the comments. It's my first time to contribute to a project which is not mine, so it may not be perfect. |
| assert qml.math.allclose( | ||
| probs[idx], 1.0 | ||
| ), f"Failed for a={a}, b={b}, cv={control_values}" | ||
|
|
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.
Can you also create an integration test where this decomposition rule is applied within the decompose transform? What you would do is first define a custom operator like
class CustomOp(Operation):
...and then a decomposition rule for CustomOp that uses TemporaryAND:
@register_resources(...)
def _custom_op_decomp(...):
...
qml.TemporaryAnd(...)
...
qml.adjoint(qml.TemporaryAnd)(...)
...Finally create a circuit that contains a CustomOp and try to decompose it:
from functools import partial
@partial(qml.transforms.decompose, gate_set={...}, fixed_decomps={CustomOp: _custom_op_decomp})
@qml.qnode(...)
def circuit():
...
CustomOp(...)
...
decomposed_tape = qml.workflow.construct_tape(circuit, level='user')()Finally you should verify that the sequence of operations within the decomposed tape is as you would expect.
A couple of things to note:
MidMeasuremust be in the gate set for this to work- Add a
@pytest.mark.usefixtures("enable_graph_decomposition")on the test case so that the new system is enabled for this test.
Let me know if you have questions about any part of this.
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.
My motivation of adding this decomposition rule is exactly the need to use it in qml.Select, so I think we can use qml.Select to serve as the CustomOp you mentioned?
wires = [0, 1, 'aux0', 2, 3]
qml.decomposition.enable_graph()
@qml.set_shots(1)
@qml.qnode(qml.device("default.qubit", wires=wires), interface=None)
@partial(qml.transforms.decompose, fixed_decomps={qml.Select: qml.templates.subroutines.select._select_decomp_unary})
def circuit():
# query |11> state
qml.X(0)
qml.X(1)
ops = [qml.X(2 + _ % 2) for _ in range(4)] # four arbitrary operators to be selected
qml.Select(ops, control=[0, 1], work_wires=['aux0'], partial=True)
return qml.sample(wires=wires)
print(qml.draw(circuit)())result:
0: ──X──X───────────────╭●──X────────────────────────────────╭○────╭●───────╭●────── ···
1: ──X────────────╭●────│─────────────╭●──X──────────────────│─────│──╭●────│─────── ···
aux0: ──H──Rϕ(-0.79)─╰X──T─╰X──Rϕ(-0.79)─╰X──T──H──Rϕ(-1.57)─╭●─╰X─╭●─╰X─╰X─╭●─╰X─╭●──H ···
2: ───────────────────────────────────────────────────────╰X────│────────╰X────│──── ···
3: ─────────────────────────────────────────────────────────────╰X─────────────╰X─── ···
0: ··· ───────────╭●─┤ ╭Sample
1: ··· ───────────╰Z─┤ ├Sample
aux0: ··· ──┤↗│ │0⟩──║─┤ ├Sample
2: ··· ───║────────║─┤ ├Sample
3: ··· ───║────────║─┤ ╰Sample
╚════════╝
Please let me know how to "verify that the sequence of operations" and wrap this into a proper test function.
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.
Currently I keep getting warning when running the circuit above:
UserWarning: Operator MidMeasureMP does not define a decomposition to the target gate set and was not found in the target gate set. To remove this warning, add the operator name (MidMeasureMP) or type (<class 'pennylane.ops.mid_measure.mid_measure.MidMeasure'>) to the gate set.
This will be addressed once the support for MCMs in decomposition is added?
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.
If you do not provide a gate set to decompose, it will use a default gate set that does not have MidMeasureMP in it.
Optimally, you could provide a gate set that includes all needed gates and MidMeasureMP to remove the warning, as @astralcai suggested :)
Something like gate_set={"X", "T", "Adjoint(T)", "Hadamard", "C(X)", "CZ", "MidMeasureMP", "Adjoint(S)"} could work (have not tested it exactly, but you get the idea :)
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.
Please let me know how to "verify that the sequence of operations" and wrap this into a proper test function.
You can do
tape = qml.workflow.construct_tape(circuit)()
expected_ops = [the, ops, you, expect]
for op, exp_op in zip(tape.operations, expected_ops, strict=True):
qml.assert_equal(op, exp_op)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 still have some problem with this qml.assert_equal.
wires = [0, 1, 'aux0', 2]
qml.decomposition.enable_graph()
gate_set = {"X", "T", "Adjoint(T)", "Hadamard", "CX", "CZ", "MidMeasureMP", "Adjoint(S)"}
@qml.set_shots(1)
@qml.qnode(qml.device("default.qubit", wires=wires), interface=None)
@partial(qml.transforms.decompose, gate_set=gate_set, fixed_decomps={qml.Select: qml.templates.subroutines.select._select_decomp_unary})
def circuit():
ops = [qml.Z(2) for _ in range(4)]
qml.Select(ops, control=[0, 1], work_wires=['aux0'], partial=True)
return qml.sample(wires=wires)
print(qml.draw(circuit)()) 0: ──X────────────────────╭●──X──────────────────────────────╭●─────────────╭●─────────────── ···
1: ──X────────╭●──────────│────────────╭●──X─────────────────│──────────────│────────╭●────── ···
aux0: ──H──T†──H─╰Z──H──T──H─╰Z──H──T†──H─╰Z──H──T──H──S†─╭●──H─╰Z──H──X─╭●──H─╰Z──H──H─╰Z──H─╭● ···
2: ────────────────────────────────────────────────────╰Z─────────────╰Z───────────────────╰Z ···
0: ··· ────╭●────────────────────╭●─┤ ╭Sample
1: ··· ────│─────────────────────╰Z─┤ ├Sample
aux0: ··· ──H─╰Z──H─╭●──H──┤↗│ │0⟩──║─┤ ├Sample
2: ··· ──────────╰Z──────║────────║─┤ ╰Sample
╚════════╝
tape = qml.workflow.construct_tape(circuit)()
expected_operators = [qml.X(0), qml.X(1), qml.H('aux0'), qml.adjoint(qml.T('aux0')), qml.H('aux0'),
qml.CZ(wires=[1, 'aux0']), qml.H('aux0'), qml.T('aux0'), qml.H('aux0'),
qml.CZ(wires=[0, 'aux0']), qml.H('aux0'), qml.adjoint(qml.T('aux0')),
qml.H('aux0'), qml.CZ(wires=[1, 'aux0']), qml.H('aux0'), qml.T('aux0'),
qml.H('aux0'), qml.adjoint(qml.S('aux0')), qml.X(0), qml.X(1),
qml.CZ(wires=['aux0', 2]), qml.H('aux0'), qml.CZ(wires=[0, 'aux0']),
qml.H('aux0'), qml.X('aux0'), qml.CZ(wires=['aux0', 2]), qml.H('aux0'),
qml.CZ(wires=[0, 'aux0']), qml.H('aux0'), qml.H('aux0'),
qml.CZ(wires=[1, 'aux0']), qml.H('aux0'), qml.CZ(wires=['aux0', 2]),
qml.H('aux0'), qml.CZ(wires=[0, 'aux0']), qml.H('aux0'),
qml.CZ(wires=['aux0', 2]), qml.H('aux0'),
qml.measurements.MidMeasureMP(wires=['aux0'], postselect=None, reset=True), qml.cond(qml.CZ)]
for op, exp_op in zip(tape.operations, expected_operators):
qml.assert_equal(op, exp_op)---------------------------------------------------------------------------
AssertionError Traceback (most recent call last)
Cell In[34], line 2
1 for op, exp_op in zip(tape.operations, expected_operators):
----> 2 qml.assert_equal(op, exp_op)
File [~/Lab/pennylane/pennylane/ops/functions/equal.py:217](http://localhost:8888/lab/tree/Lab/quemix/codes/Lab/pennylane/pennylane/ops/functions/equal.py#line=216), in assert_equal(op1, op2, check_interface, check_trainability, rtol, atol)
208 dispatch_result = _equal(
209 op1,
210 op2,
(...) 214 rtol=rtol,
215 )
216 if isinstance(dispatch_result, str):
--> 217 raise AssertionError(dispatch_result)
AssertionError: MidMeasure(wires=['aux0'], postselect=None, reset=True) and MidMeasure(wires=['aux0'], postselect=None, reset=True) are not equal for an unspecified reason.
Co-authored-by: Astral Cai <[email protected]>
You are doing great for your first contribution! Welcome to open source! |
| for rule in qml.list_decomps(qml.TemporaryAND): | ||
| _test_decomposition_rule(qml.TemporaryAND([0, 1, 2], control_values=(0, 0)), rule) | ||
|
|
||
| @pytest.mark.parametrize("control_values", [(0, 0)]) |
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.
Are we going to test other combinations of control values?
Context:
The class
TemporaryANDcurrently has a decomposition rule that requires 4 T gates. However, its adjoint — which can be implemented using mid-circuit measurement (MCM) and reset with zero T gates — was not yet defined. For a reference, see Fig. 4 in arXiv:1805.03662.Description of the Change:
Added a new decomposition rule for
Adjoint(TemporaryAND)that uses mid-circuit measurement.Benefits:
Enables decomposition using fewer T gates in subroutines like
qml.Select(unary iterator decomposition).Possible Drawbacks:
Mid-circuit measurements are still not fully supported by all transformation pipelines. Since
qml.matrixdoes not support MCMs, the test avoids_test_decomposition_ruleand instead validates equivalence at the circuit level using a QNode.Related GitHub Issues: