Skip to content
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

Adds mixed-polarity multi-controlled gates #1104

Merged
merged 28 commits into from
Mar 8, 2021
Merged

Adds mixed-polarity multi-controlled gates #1104

merged 28 commits into from
Mar 8, 2021

Conversation

glassnotes
Copy link
Contributor

@glassnotes glassnotes commented Feb 22, 2021

Context: A common use case of multi-controlled operations is to control on values other than all qubits being in state |1>.

Description of the Change: This PR does two things:

  • Extends the existing ControlledQubitUnitary to accept a string of bits (or integer) that specifies the state of the control bits
  • Adds a new operation, MixedPolarityMultiControlledToffoli which is the special case of a multi-controlled NOT

Benefits: Makes multi-controlled operations more flexible without having to apply Pauli X's everywhere.

Possible Drawbacks:

  • "Mixed-polarity multi-controlled Toffoli" is the term I've encountered for these gates in the wild, but the name is quite long and may be confusing since it's a multi-controlled NOT. One option is to abbreviate this as MPMCT. An alternative is MultiControlledNOT.

Related GitHub Issues: None

pennylane/ops/qubit.py Outdated Show resolved Hide resolved
pennylane/ops/qubit.py Outdated Show resolved Hide resolved
pennylane/ops/qubit.py Outdated Show resolved Hide resolved
@glassnotes glassnotes marked this pull request as ready for review February 23, 2021 18:25
@glassnotes glassnotes changed the title [WIP] Adds mixed-polarity multi-controlled gates Adds mixed-polarity multi-controlled gates Feb 23, 2021
@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #1104 (4a301e8) into master (59e074a) will decrease coverage by 0.00%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1104      +/-   ##
==========================================
- Coverage   97.74%   97.74%   -0.01%     
==========================================
  Files         155      155              
  Lines       11760    11783      +23     
==========================================
+ Hits        11495    11517      +22     
- Misses        265      266       +1     
Impacted Files Coverage Δ
pennylane/devices/default_mixed.py 100.00% <ø> (ø)
pennylane/devices/default_qubit.py 100.00% <ø> (ø)
pennylane/ops/qubit.py 97.38% <96.15%> (-0.07%) ⬇️

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 59e074a...4a301e8. Read the comment docs.

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a really nice addition. When we have controlled templates/wires, would be nice to also think about having mixed polarity there too.

Comment on lines 53 to 57
- Added support for mixed-polarity multi-controlled operations in
`ControlledQubitUnitary` for arbitrary unitary operations, and special
case `MixedPolarityMultiControlledToffoli` for multi-controlled `NOT`
gates. [(#1104)](https://github.com/PennyLaneAI/pennylane/pull/1104)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we merge the ControlledQubitUnitary part into the bullet point above, and potentially say what mixed polarity means? There, we could have an example:

For example, we can create a multi-controlled T gate using:

T = qml.T._matrix()
qml.ControlledQubitUnitary(T, control_wires=[0, 1, 3], wires=2, control_values="110")

Here, control wires 0 and 1 must be in the 1 state, while control wire 3 must be in the zero state for the unitary to be applied to wire 2.

Then this bullet point could be short and highlight MixedPolarityMultiControlledToffoli?

pennylane/ops/qubit.py Show resolved Hide resolved
Comment on lines 1631 to 1651
Typically controlled operations apply a desired gate if the control qubits
are all in the state :math:`\vert 1\rangle`. However, there are some situations where
it is necessary to apply a gate conditioned on all qubits being in the
:math:`\vert 0\rangle` state, or some mix of the two.

The state on which to control can be changed by passing a value to
`control_values` as either a bit string, or an integer (which is converted
to binary representation in Big-Endian format).

For example, if we want to apply a single-qubit unitary to wire ``3``
conditioned on three wires where the first is in state ``0``, the second is
in state ``1``, and the third in state ``1``, we can write:

>>> qml.ControlledQubitUnitary(U, control_wires=[0, 1, 2], wires=3, control_values='011')

Or equivalently,

>>> qml.ControlledQubitUnitary(U, control_wires=[0, 1, 2], wires=3, control_values=3)

since ``011`` is 3 in binary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice example!

pennylane/ops/qubit.py Show resolved Hide resolved
pennylane/ops/qubit.py Outdated Show resolved Hide resolved
pennylane/ops/qubit.py Outdated Show resolved Hide resolved
Comment on lines 1755 to 1756
>>> qml.MixedPolarityMultiControlledToffoli(control_wires=[0, 1, 2, 3], wires=4, control_values='1110'])
>>> qml.MixedPolarityMultiControlledToffoli(control_wires=[0, 1, 2, 3], wires=4, control_values=14)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this was as helpful here. Maybe they can defer to ControlledQubitUnitary docs?

tests/ops/test_qubit_ops.py Outdated Show resolved Hide resolved
Comment on lines +1477 to +1479
# The result of applying the mixed-polarity gate should be the same as
# if we conjugated the specified control wires with Pauli X and applied the
# "regular" ControlledQubitUnitary in between.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test!

U = unitary_group.rvs(2 ** len(target_wires), random_state=3)

@qml.qnode(dev)
def circuit_mixed_polarity():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor point, but would be it interesting to initialize in a less standard state? E.g. we take the first column of another randomly generated unitary as the starting state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea -- this also helped me catch a bug in the tests, the Pauli X were being applied to the wrong control wires. It now works with arbitrary states prepared on both the control and target qubits 🚀

Copy link
Contributor

@thisac thisac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice addition @glassnotes! I really like how this works. Just some questions/clarifications, but nothing major (I didn't have time to check all the tests due to the Gnocchi cooking class starting soon, but I'll do that as soon as I can). 🙂

Comment on lines +59 to +67
```python
T = qml.T._matrix()
qml.ControlledQubitUnitary(T, control_wires=[0, 1, 3], wires=2, control_values="110")
```

Here, the T gate will be applied to wire `2` if control wires `0` and `1` are in
state `1`, and control wire `3` is in state `0`. If no value is passed to
`control_values`, the gate will be applied if all control wires are in
the `1` state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice example. Really clear! 💯

pennylane/ops/qubit.py Outdated Show resolved Hide resolved
Comment on lines 1646 to 1650
Or equivalently,

>>> qml.ControlledQubitUnitary(U, control_wires=[0, 1, 2], wires=3, control_values=3)

since ``011`` is 3 in binary.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to be able to pass integers? I don't see any obvious use case for it (but I might be missing something). Also, it's not super-clear what control_wires=3 does while control_values='011' makes more sense. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise a really nice and clear example. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specific use-case I had in mind was lookup tables (like a quantum ROM) where you have integer or binary addresses with some stored contents. Also sometimes just it's more convenient to write an integer if there is a large number of control bits in some random-like sequence of 0 and 1 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think it's fine keeping it like this, especially if it's useful in certain cases, although I'm tend to prefer only having one type of input per argument. It could be a bit confusing to see control_values=42 in the wild. That, too me, would sound like there are 42 control values rather than what it's meant to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I definitely see how it would be confusing (especially if the integer is something like 10, is that meant to be 10, or 2?). Maybe there is a better way to enable the user to input either value - having two separate keyword arguments, control_bits and control_int would be one option, I suppose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It avoids the confusion above, although now there are two arguments that basically do the same thing. Not sure if it might be better to just have string inputs allowed, and then the extra step to create it from an int would, if needed, would be on the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the extra step to create it from an int would

This is something I have to look up every time 😅 Okay I'm on board, as this will force me to remember.

@staticmethod
def _parse_control_values(control_wires, control_values):

control_int = control_values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this statement can be put inside the if isinstance(control_values, int)?

return control_int


class MultiControlledX(ControlledQubitUnitary):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better name! 👍

Comment on lines +1423 to +1424
def test_invalid_mixed_polarity_controls(
self, control_wires, wires, control_values, expected_error_message
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense having these test only test the gates, e.g. simply

target_wires = Wires(wires)

with pytest.raises(ValueError, match=expected_error_message):
    qml.ControlledQubitUnitary(
        X, control_wires=control_wires, wires=target_wires, control_values=control_values
    )

That would make them a bit more unit-test-like. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

pennylane/ops/qubit.py Outdated Show resolved Hide resolved
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @glassnotes!

.github/CHANGELOG.md Outdated Show resolved Hide resolved
.github/CHANGELOG.md Outdated Show resolved Hide resolved
tests/ops/test_qubit_ops.py Outdated Show resolved Hide resolved
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
@glassnotes
Copy link
Contributor Author

@trbromley how do we handle failure of codecov in this case?

@trbromley
Copy link
Contributor

Is it failing in error? I think so - looks like it's saying the Alternative control values must be passed as a binary string. line is not covered, but I can see it covered in the tests.

If we're sure it's a mistake by codecov, we can ask @josh146 to merge anyway.

@glassnotes
Copy link
Contributor Author

Is it failing in error? I think so - looks like it's saying the Alternative control values must be passed as a binary string. line is not covered, but I can see it covered in the tests.

If we're sure it's a mistake by codecov, we can ask @josh146 to merge anyway.

Very strange, it's definitely covered in test cases for both ControlledQubitUnitary and MultiControlledX. Anyways yes, we can go ahead and merge 👍

Copy link
Contributor

@thisac thisac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @glassnotes! Thanks for adding this. 🚀

.github/CHANGELOG.md Outdated Show resolved Hide resolved
glassnotes and others added 2 commits March 1, 2021 08:43
@josh146 josh146 merged commit 10fde20 into master Mar 8, 2021
@josh146 josh146 deleted the add-mpmct branch March 8, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants