[WIP] Rework of controlled operations

[astralcai]

Context:
All controlled operations should inherit from the general Controlled class, and the decomposition of controlled operations is not consistent for custom and non-custom controlled operations. This is a continuation of #5069

Description of the Change:

• Make CNOT, CCZ, CH, Toffoli, MultiControlledX, CSWAP inherit from ControlledOp.
• qml.ctrl called on operators with custom controlled versions will return instances of the custom class.
• Special handling of PauliX based controlled operations (PauliX, CNOT, Toffoli, MultiControlledX)
  • Calling qml.ctrl on one of these operators will always resolve to the best option in CNOT, Toffoli, or MultiControlledX depending on the number of control wires and control values.
• qml.ctrl will flatten nested controlled operators to a single multi-controlled operation.
• Controlled operators with a custom controlled version decomposes like how their controlled counterpart decomposes, as opposed to decomposing into their controlled version.
  • Special handling of PauliX based controlled operations: e.g., Controlled(CNOT([0, 1]), [2, 3]) will have the same decomposition behaviour as a MultiControlledX([2, 3, 0, 1])

Benefits:
Cleaner code and more consistent decomposition behaviour of controlled operations.

Possible Drawbacks:
Change of decomposition behaviour may cause issues.

Related GitHub Issues:
#5069
#1447

Related Shortcut Stories
[sc-37951]
[sc-55131]

[github-actions]

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.

[dime10]

[sc-55358]

[dime10]

I have a question about this aspect:

• Controlled operators with a custom controlled version decomposes like how their controlled counterpart decomposes, as opposed to decomposing into their controlled version.
  • Special handling of PauliX based controlled operations: e.g., Controlled(CNOT([0, 1]), [2, 3]) will have the same decomposition behaviour as a MultiControlledX([2, 3, 0, 1])

Should those operators even exist to begin with? As in, why allow Controlled(CNOT([0, 1]), [2, 3]) to exist when it already has a representation as MultiControlledX([2, 3, 0, 1])?

I think there is a danger that low-level code could create Controlled(CNOT([0, 1]), [2, 3]) anyways and devices would still need to handle both regardless. The unification of the decomposition behaviour is nice, but wouldn't it be even better if they were identical in all behaviours? Unless of course there is a good reason for Controlled(CNOT([0, 1]), [2, 3]) to exist.

(posting here for threaded conversation)

[astralcai]

So external users should only be using ctrl, and that would always give you the MultiControlledX as opposed to the Controlled(CNOT([0, 1]), [2, 3]). I suppose we could have a separate PR after this to clean up all internal use of the Controlled constructor and make them call ctrl instead?

[dime10]

The only "problem" I see with that approach is that the potential for internal "misuse" of the Controlled class remains present into the future, even if we clean up all uses now.

[astralcai]

The only "problem" I see with that approach is that the potential for internal "misuse" of the Controlled class remains present into the future, even if we clean up all uses now.

This is true. @albi3ro What do you feel about this?

[albi3ro]

I think we should expect for things like Controlled(CNOT((2,3)), (0,1)) to occur have have sensible defaults for when such a case occurs, such as decomposing to like a MultiControlledX.

I'm not sure it would even be considered a "misuse", as these wrappers are all about "you can control anything you feel like and we will handle it the best we can".

[dime10]

I think we should expect for things like Controlled(CNOT((2,3)), (0,1)) to occur have have sensible defaults for when such a case occurs, such as decomposing to like a MultiControlledX.

I don't necessarily want to disagree, but can we actually think of a reason why Controlled(CNOT((2,3)), (0,1)) should exist?

I'm not sure it would even be considered a "misuse"

I just labelled it "misuse" in response to the sentiment that only qml.ctrl should ever be used in order to avoid any issues.

[timmysilv]

the only reason I can think of is user expectations about class constructors vs. helper functions (that we ourselves have defined). For this example, if you call Controlled(some_base), I expect the returned value to be an instance of Controlled. This means it should have eg. a base property, and whatever else Controlled will provide. This might be a by-product of an OOP brain, but when I use a class constructor, I expect a class instance back. I'll admit, we have some sneaky __new__ overrides in our code, and they all scare me.

Another example is qml.prod vs. the Prod class:

>>> type(qml.prod(qml.PauliX(0)))
<class 'pennylane.ops.qubit.non_parametric_ops.PauliX'>
>>> type(qml.ops.Prod(qml.PauliX(0)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/matthews/src/github.com/PennyLaneAI/pennylane/pennylane/ops/op_math/composite.py", line 61, in __init__
    raise ValueError(f"Require at least two operators to combine; got {len(operands)}")
ValueError: Require at least two operators to combine; got 1

The prod helper does some sneaky stuff, and doesn't even return a Prod instance if it doesn't make sense to! That said.. it does fail if you give it something that (in your words) shouldn't exist. We could be consistent in this design, and make Controlled(base) fail if the base itself is also Controlled. This would encourage what Astral is proposing - to use qml.ctrl if you're getting up to some funny business. I don't mean to make a vote in any direction because I don't necessarily love or hate any choice, but I would like to strive for consistency above all else if possible.

[astralcai]

That said.. it does fail if you give it something that (in your words) shouldn't exist. We could be consistent in this design, and make Controlled(base) fail if the base itself is also Controlled.

A big portion of this PR is to make sure that the operation you get when you call ctrl vs. when you call Controlled on a base that is already a controlled operation should have the same decomposition behaviour. But other behaviour will not be the same, such as with Controlled(CNOT([0, 1]), 2).control_wires would be [2], but ctrl(CNOT([0, 1]), 2).control_wires would be [2, 0], including the control wire in CNOT. Whether or not we raise an error in this case depends on whether we consider one of these results to be objectively wrong, or do we consider both of them valid, depending on your perspective.

[dime10]

This might be a by-product of an OOP brain, but when I use a class constructor, I expect a class instance back. I'll admit, we have some sneaky new overrides in our code, and they all scare me.

Yeah that's a good point 😅 The constructor probably shouldn't return a class that is not itself.

Outright disallowing the construction in certain cases would be an alternative.

Either way though, this PR is a huge improvement, but avoiding the same thing to be represented in multiple ways (i.e. striving for a canonical representation) is a huge help for downstream code, see for example a relevant issue in lightning.

[dime10]

I don't know if you've already taken this into account, but what I would ideally like to see (and test) is that this behaviour:

• qml.ctrl will flatten nested controlled operators to a single multi-controlled operation.

be applied not only during first construction of the operator, but also in later stages. Consider an operator:

class MyOp:
  def decomposition(self):
    return [qml.CNOT(self.wires)]

If the user writes qml.ctrl(MyOp([0,1]), control=[1,2]), the construction logic cannot initially generate a MultiControlledX gate. However, it would be great to ensure that during the decomposition process, when MyOp turns into CNOT, we generate a MultiControlledX rather than a C(CNOT).

[astralcai]

However, it would be great to ensure that during the decomposition process, when MyOp turns into CNOT, we generate a MultiControlledX rather than a C(CNOT).

I believe this is indeed the new behaviour, but I can add test cases to make sure.

[albi3ro]

Suggested change

	def _get_special_ops():
	@functools.lru_cache
	def _get_special_ops():

Potentially we can just construct the data once.

[astralcai]

This PR will be split into two, here is part 1: #5125