Implement transform to push commuting operations through controlled gates #1464

glassnotes · 2021-07-20T16:30:28Z

Context: Addition of compilation transforms to PennyLane.

Description of the Change: Adds a new transform, commute_controlled that pushes commuting gates through controls/targets of controlled operations. All diagonal gates are pushed ahead (or behind, if specified) of control qubits, and X/Y/Z-type gates are pushed through targets when appropriate. Additional compilation attributes and properties are also added to support the transform.

Benefits: Moving as many gates as possible through controlled operations will enable more single-qubit gate simplifications to be performed, as well as fusion and cancellation of two-qubit controlled rotations once those in-between gates are out of the way.

Possible Drawbacks: Rather than adding attributes, it would be nice if we could instead resolve #1447 and have things inherit from ControlledOperation. However after a preliminary look, it seems that would be a pretty extensive refactor.

Related GitHub Issues: Uses some of the code and ideas from #1446 🎉

github-actions · 2021-07-20T16:30:46Z

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.

glassnotes · 2021-07-20T19:19:35Z

pennylane/operation.py

+    """
+
+    @property
+    def comp_control_wires(self):


I originally named this property control_wires, however this caused issues with variables of the same name used in operations like MultiControlledX. I prefixed with comp_ to indicate compilation, but we should figure out a better way of incorporating this information for all gates (e.g., inheritance from a class similar to ControlledOperation)

Hmm, I would suggest renaming MultiControlledX.control_wires -> MultiControlledX._control_wires.

Then, you could have this property do two things:

By default, Operation.control_wires could simply return self._control_wires if it exists, otherwise raising NotImplementedError

Inheriting devices can defined this class property directly.

codecov · 2021-07-20T19:20:59Z

Codecov Report

Merging #1464 (455560c) into master (c9b78a5) will increase coverage by 0.00%.
The diff coverage is 99.20%.

@@           Coverage Diff            @@
##           master    #1464    +/-   ##
========================================
  Coverage   98.29%   98.30%            
========================================
  Files         169      170     +1     
  Lines       12360    12483   +123     
========================================
+ Hits        12149    12271   +122     
- Misses        211      212     +1

Impacted Files	Coverage Δ
...nnylane/transforms/optimization/cancel_inverses.py	`100.00% <ø> (ø)`
...nnylane/transforms/optimization/merge_rotations.py	`100.00% <ø> (ø)`
...ane/transforms/optimization/single_qubit_fusion.py	`100.00% <ø> (ø)`
pennylane/ops/qubit.py	`98.67% <97.82%> (-0.04%)`	⬇️
pennylane/operation.py	`95.78% <100.00%> (+0.01%)`	⬆️
pennylane/transforms/__init__.py	`100.00% <100.00%> (ø)`
pennylane/transforms/control.py	`100.00% <100.00%> (ø)`
pennylane/transforms/optimization/__init__.py	`100.00% <100.00%> (ø)`
...lane/transforms/optimization/commute_controlled.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9b78a5...455560c. Read the comment docs.

pennylane/operation.py

glassnotes · 2021-07-20T20:01:54Z

pennylane/ops/qubit.py

+    @property
+    def comp_control_wires(self):
+        return Wires(self.wires[0])
+


Not sure how to handle testing for this one, it seems redundant to do it for every gate.

Agree 🤔 What you could do is add # pragma: no cover to this property, to indicate it is intentionally not covered?

I've added the # pragma (L595 of operation.py) but the warning still occurs. Was it added incorrectly?

Looks okay to me 🤔

Huh :( Well... since it is not causing any issues with the CI and doesn't require an override, I am going to just merge 👍

glassnotes · 2021-07-20T20:02:54Z

tests/transforms/test_optimization/utils.py

+            if op.wires == qml.wires.Wires(wire_order):
+                mat = qml.math.dot(op.matrix, mat)
+            else:
+                mat = qml.math.dot(qml.SWAP(wires=[0, 1]).matrix, qml.math.dot(op.matrix, mat))


I should have caught this when I wrote it originally. Emphasizes the need for that get_unitary_matrix transform.

josh146

Nice @glassnotes! I left some suggestions and thoughts regarding the structure. Looking forward to having all these transforms in!

josh146 · 2021-07-21T08:55:12Z

pennylane/operation.py

+    """
+
+    @property
+    def comp_control_wires(self):


Hmm, I would suggest renaming MultiControlledX.control_wires -> MultiControlledX._control_wires.

Then, you could have this property do two things:

By default, Operation.control_wires could simply return self._control_wires if it exists, otherwise raising NotImplementedError

Inheriting devices can defined this class property directly.

josh146 · 2021-07-21T08:56:35Z

pennylane/ops/qubit.py

+    @property
+    def comp_control_wires(self):
+        return Wires(self.wires[0])
+


Agree 🤔 What you could do is add # pragma: no cover to this property, to indicate it is intentionally not covered?

josh146 · 2021-07-21T08:57:58Z

.github/CHANGELOG.md

+  single-qubit gates ahead of controlled operations.
+  [(#1464)](https://github.com/PennyLaneAI/pennylane/pull/1464)
+
+  The `commute_through_controls_targets` transform works as follows:


Relatively minor, but is there more succinct name we could use for this transform? I tend to dislike the verbosity/typing of functions with more than 2 or so words!

commute_controlled?

josh146 · 2021-07-21T09:00:52Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+# Gates that commute among each particular basis
+commuting_gates = {
+    "X": ["PauliX", "RX", "SX"],
+    "Y": ["PauliY", "RY"],
+    "Z": ["PauliZ", "RZ", "PhaseShift", "S", "T"],
+}


Out of curiosity: could we not have a basis property on gates instead? e.g.:

class RX(Operation): basis = "X" ... class CRX(Operation): basis = "X" ...

This could replace the dictionary here as well as the target_basis property?

Alternatively, you could instead define the controlled target basis as a dictionary of gates here?

Not really suggesting one over the other, I think both approaches have their place. Just gathering my thoughts: I guess I'm trying to determine why commuting_gates is transform-specific data, while target_basis is a Operation-specific data.

Is commuting_gates explicitly linked to the compilation transform?

Awesome, I like the idea of specifying a basis; then we can just check that they match, and there is no need to have any of this info here. Nothing is compilation-specific, the gates either commute or don't, it's just we aren't right now making use of that information anywhere else.

josh146 · 2021-07-21T09:03:58Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+    Args:
+        qfunc (function): A quantum function.


I can't recall if other compilation transforms have this, but does it make sense to have

Returns: function: the transformed quantum function

They don't, but I think that's a good idea. Will update!

josh146 · 2021-07-21T09:04:46Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+    with ``CRY``). We can use the transform to push single-qubit gates as
+    far as possible through the controlled operations:


Question: could we have

@commute_through_controls_target(direction="left"|"right")

in case you want to move the single-qubit operations in a particular direction? :D

I love this!! After I wrote the transform I was still debating whether I should not rewrite it the other way (in fact, the precursors to it worked by pushing the controlled operations to the right and thus the single-qubit gates to the left, but didn't work nearly as well).

Also, this implicitly solves the long name problem 😁

josh146 · 2021-07-21T09:05:07Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+    # We will go through the list backwards; whenever we find a single-qubit
+    # gate, we will extract it and push it through 2-qubit gates as far as
+    # possible to the right.


Your comments are always super helpful to understand the logical flow!

josh146 · 2021-07-21T09:06:36Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+        # Find the next gate that contains an overlapping wire
+        next_gate_idx = find_next_gate(current_gate.wires, list_copy[current_location + 1 :])
+
+        where_should_we_put_it = current_location


Perhaps new_location?

josh146 · 2021-07-21T09:08:05Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+                except KeyError:
+                    break


When does this happen? Shouldn't this be AttributeError? 🤔

josh146 · 2021-07-21T09:08:40Z

tests/transforms/test_optimization/utils.py

+            if op.wires == qml.wires.Wires(wire_order):
+                mat = qml.math.dot(op.matrix, mat)
+            else:
+                mat = qml.math.dot(qml.SWAP(wires=[0, 1]).matrix, qml.math.dot(op.matrix, mat))


anthayes92 · 2021-07-21T10:59:04Z

tests/transforms/test_optimization/test_commute_through_controls_targets.py

+    compare_operation_lists,
+    compute_matrix_from_ops_two_qubit,
+    check_matrix_equivalence,


Really useful to see these functions being used that I'd otherwise remain unaware of!

anthayes92 · 2021-07-21T11:24:19Z

tests/transforms/test_optimization/test_commute_through_controls_targets.py

+    def test_commute_through_controls_targets_autograd(self):
+        """Test QNode and gradient in autograd interface."""
+
+        original_qnode = qml.QNode(qfunc, dev)


might be worth explicitly passing interface="autograd" in case the default interface changes in future

anthayes92 · 2021-07-21T11:32:16Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+
+    Diagonal gates on either side of control qubits do not affect the outcome
+    of controlled gates; thus we can push all the single-qubit gates on the
+    first qubit together on the right (and fuse them if desired). Similarly, X


Is there any plan to combine this and fusion in a single function?

Not at the moment - for now, trying to keep the transforms as modular as possible so they can be chained in different ways, but this might change in the future.

anthayes92 · 2021-07-21T11:32:54Z

pennylane/transforms/optimization/commute_through_controls_targets.py

+    # We will go through the list backwards; whenever we find a single-qubit
+    # gate, we will extract it and push it through 2-qubit gates as far as
+    # possible to the right.


… transform.

.github/CHANGELOG.md

pennylane/transforms/optimization/commute_controlled.py

glassnotes · 2021-07-21T15:09:23Z

pennylane/transforms/optimization/commute_controlled.py

+    return op_list
+
+
+def _commute_controlled_left(op_list):


Since this is so close to the _right version, I have not added a full set of comments in order to keep things more streamlined. I can add them back if preferred though 🙂

tests/transforms/test_optimization/test_commute_controlled.py

glassnotes · 2021-07-21T15:29:07Z

@josh146 @anthayes92 thank you both for the feedback, updated version is cleaner and more flexible now 😁

josh146

💯 Happy to approve this!

Only blocker it looks like there are a few places where the left function is not being tested?

josh146 · 2021-07-22T12:13:53Z

pennylane/transforms/optimization/commute_controlled.py

+            # Only go ahead if information is available
+            if next_gate.basis is None:
+                break


I guess this never happens in the test suite?

Coverage has been added for this and the instances in _commute_controlled_left 👍

…g-of-commuting

.github/CHANGELOG.md

tests/transforms/test_optimization/test_commute_controlled.py

Initial commit of transform to push stuff behind controlled gates.

bb54e6e

Redo transform backwards by pushing gates forwards.

feacce7

glassnotes changed the title ~~Implement transform to push commuting operations behind controlled gates~~ Implement transform to push commuting operations through controlled gates Jul 20, 2021

glassnotes added 5 commits July 20, 2021 14:39

Update docs and CHANGELOG.

6c0d005

Fix argument type in docs.

bd7538c

Remove print statements.

1283fe8

Add interface tests.

703005b

Rename control/target wire attributes due to degeneracy.

f37a88e

glassnotes commented Jul 20, 2021

View reviewed changes

glassnotes added 2 commits July 20, 2021 15:37

Streamline attribute; remove unused target wires. Add try/except.

873304a

Remove unnecessary check.

2a0c1c9

glassnotes commented Jul 20, 2021

View reviewed changes

pennylane/operation.py Outdated Show resolved Hide resolved

Update pennylane/operation.py

07a6f6f

glassnotes commented Jul 20, 2021

View reviewed changes

glassnotes requested review from josh146 and anthayes92 July 20, 2021 20:03

josh146 requested changes Jul 21, 2021

View reviewed changes

anthayes92 approved these changes Jul 21, 2021

View reviewed changes

glassnotes added 9 commits July 21, 2021 08:55

Rename target_basis to basis. Remove unnecessary data structures from…

04d5440

… transform.

Rename transform.

ade883c

Specify interface.

4524ef8

Specify return type of other transforms.

ba408b7

Set up for bi-directionality of transform.

a9eb8c5

Rename tests.

cff25a4

Add method to push gates left through controlled operations.

7cb1ba3

Rename control wires and remove coverage from attribute.

9c657b3

Remove unused import

6b82be2

Add new property to ControlledOperation.

dd934d1

glassnotes commented Jul 21, 2021

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

glassnotes commented Jul 21, 2021

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

glassnotes commented Jul 21, 2021

View reviewed changes

pennylane/transforms/optimization/commute_controlled.py Show resolved Hide resolved

glassnotes commented Jul 21, 2021

View reviewed changes

glassnotes and others added 2 commits July 21, 2021 11:14

Add test for invalid direction.

cb2e4c4

Apply suggestions from code review

bd52d8e

glassnotes commented Jul 21, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

glassnotes commented Jul 21, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

Apply suggestions from code review

2afba23

glassnotes requested a review from josh146 July 21, 2021 15:29

josh146 approved these changes Jul 22, 2021

View reviewed changes

glassnotes and others added 3 commits July 23, 2021 09:42

Improve test coverage.

bcd58c6

Merge branch 'master' into ch7108-implement-transforms-for-re-orderin…

4bd45a5

…g-of-commuting

Add missing backticks.

132d2ac

glassnotes commented Jul 23, 2021

View reviewed changes

.github/CHANGELOG.md Outdated Show resolved Hide resolved

Update .github/CHANGELOG.md

6263197

glassnotes commented Jul 23, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

Update tests/transforms/test_optimization/test_commute_controlled.py

2accc47

glassnotes commented Jul 23, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

glassnotes commented Jul 23, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

glassnotes commented Jul 23, 2021

View reviewed changes

tests/transforms/test_optimization/test_commute_controlled.py Outdated Show resolved Hide resolved

Fix inconsistencies in test docstrings.

455560c

glassnotes merged commit 347d06e into master Jul 23, 2021

glassnotes deleted the ch7108-implement-transforms-for-re-ordering-of-commuting branch July 23, 2021 16:51

		with ``CRY``). We can use the transform to push single-qubit gates as
		far as possible through the controlled operations:

Implement transform to push commuting operations through controlled gates #1464

Implement transform to push commuting operations through controlled gates #1464

Conversation

glassnotes commented Jul 20, 2021 • edited Loading

github-actions bot commented Jul 20, 2021

glassnotes Jul 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 20, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh146 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glassnotes commented Jul 21, 2021

josh146 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glassnotes commented Jul 20, 2021 •

edited

Loading

glassnotes Jul 20, 2021 •

edited

Loading

codecov bot commented Jul 20, 2021 •

edited

Loading