[RFC] Improve handling of nested control structures

Looking for feedback on the following:

After merging #599, Lightning obtained support for the BlockEncode operator. However, even though Lightning in principle supports arbitrary control wires on all supported operators, it didn't automatically gain support for C(BlockEncode). The reason is that this is treated as a separate gate altogether, and many operators are additionally added in their controlled form to the list of supported ops, which seems redundant. To make matters worse, operations of the form C(C(...)) are still not supported.

The following patch is just one way we could handle controlled operations in a more generic fashion. qml.simplify is applied to any Controlled instance before processing it in order to flatten any nested control structures. Additionally, the supports_operation method is overridden to strip any C( prefixes from gate names to determine support status.

Note that nested control structures can easily arise when users write functions that call other functions while adding qml.ctrl into the mix.

Benefits:

• no redundant C(...) style specifications in the list of operations
• much faster processing of certain operations: For instance, C(C(SWAP)) would previously need to be decomposed into many gates in order to simulate it, when it could just be applied in a single step. See the following benchmark for how this affects simulation time: -> more than 10x improvement
• supported gates that cannot be decomposed (such as BlockEncode) are automatically also supported in their controlled version

[sc-55549]

Jan 26 '24 00:01 dime10

Hello. You may have forgotten to update the changelog! Please edit .github/CHANGELOG.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.

Jan 26 '24 00:01 github-actions[bot]

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (2716864) 98.52% compared to head (b8d91c4) 96.71%.

@@            Coverage Diff             @@
##           master     #602      +/-   ##
==========================================
- Coverage   98.52%   96.71%   -1.82%     
==========================================
  Files         168      169       +1     
  Lines       24566    24321     -245     
==========================================
- Hits        24204    23521     -683     
- Misses        362      800     +438     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jan 26 '24 01:01 codecov[bot]

I really like the direction this is taking :) Also, wouldn't it be nice if, for example, CNOT, Toffoli, MultiControlledX were just aliases for C(PauliX)? This way we would not need special treatment for all these gates.

Jan 26 '24 13:01 vincentmr

Also, wouldn't it be nice if, for example, CNOT, Toffoli, MultiControlledX were just aliases for C(PauliX)? This way we would not need special treatment for all these gates.

100%, luckily I think @astralcai is already working on ensuring all controlled-x type operations have a canonical representation.

Jan 26 '24 14:01 dime10

In the current master of pennylane, all controlled operations inherit from Controlled, so you should be able to replace all string comparisons here with isinstance checks. For CNOT, Toffoli, and MultiControlledX, you do not need special handling anymore, because they are all instances of Controlled, and they all have a base property that is a PauliX.

Feb 21 '24 14:02 astralcai