pennylane icon indicating copy to clipboard operation
pennylane copied to clipboard

Add decomposition to `Controlled`

Open albi3ro opened this issue 2 years ago • 1 comments

Currently, ControlledOperation created by ctrl has some very basic decomposition behaviour. But it at least has more decomposition behaviour than the non-integrated Controlled.

This PR makes Controlled reach feature-parity with ControlledOperation and will allow us to replace it.

albi3ro avatar Aug 11 '22 22:08 albi3ro

Hello. You may have forgotten to update the changelog! Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

github-actions[bot] avatar Aug 11 '22 22:08 github-actions[bot]

Codecov Report

Merging #2938 (2bce582) into master (8c15817) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2938   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files         266      266           
  Lines       22332    22364   +32     
=======================================
+ Hits        22255    22287   +32     
  Misses         77       77           
Impacted Files Coverage Δ
pennylane/ops/op_math/controlled_class.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/matrix_ops.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/non_parametric_ops.py 100.00% <100.00%> (ø)
pennylane/ops/qubit/parametric_ops.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 16 '22 21:08 codecov[bot]

I don't have much time for a full review, but noticed the following:

>>> qml.ops.Controlled(qml.PauliRot(3, "XX", [0, 4]), 2).decomposition()
[C(Hadamard)(wires=[2, 0]),
 C(Hadamard)(wires=[2, 4]),
 C(MultiRZ)(3, wires=[2, 0, 4]),
 C(Hadamard)(wires=[2, 0]),
 C(Hadamard)(wires=[2, 4])]

Shouldn't C(MultiRZ) be decomposed as well?

AlbertMitjans avatar Aug 17 '22 09:08 AlbertMitjans

@AlbertMitjans I want to add more decomposition algorithms in the future, but that would take a bit more work than just translating over what ControlledOperation already does.

albi3ro avatar Aug 17 '22 13:08 albi3ro