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

Extend qml.cond #2275

Merged
merged 28 commits into from Mar 4, 2022
Merged

Extend qml.cond #2275

merged 28 commits into from Mar 4, 2022

Conversation

antalszava
Copy link
Contributor

@antalszava antalszava commented Mar 3, 2022

Context:
The addition of qml.cond in #2211 can be extended.

Description of the Change:

  • Implement MeasurementValue.__invert__ to allow the ~mv syntax
  • Allow passing an else case to qml.cond
  • Allow quantum functions to work with qml.cond

Note: further changes to the documentation would follow separately.

Benefits:
More powerful conditional operation structures.

Possible Drawbacks:
N/A

Related GitHub Issues:
N/A

Note: ~ was chosen to "flip" the measurement value instead of not because not involves defining __bool__ that requires a boolean return.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022

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.

@codecov
Copy link

codecov bot commented Mar 3, 2022

Codecov Report

Merging #2275 (c27f50d) into conditional-ops-alpha-sanitized (53ff922) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                       Coverage Diff                        @@
##           conditional-ops-alpha-sanitized    #2275   +/-   ##
================================================================
  Coverage                            99.31%   99.31%           
================================================================
  Files                                  239      241    +2     
  Lines                                19023    19097   +74     
================================================================
+ Hits                                 18893    18967   +74     
  Misses                                 130      130           
Impacted Files Coverage Δ
pennylane/measurements.py 99.33% <100.00%> (+0.02%) ⬆️
pennylane/transforms/condition.py 100.00% <100.00%> (ø)
pennylane/qnode.py 100.00% <0.00%> (ø)
pennylane/ops/cv.py 100.00% <0.00%> (ø)
pennylane/__init__.py 100.00% <0.00%> (ø)
pennylane/drawer/tape_text.py 100.00% <0.00%> (ø)
pennylane/ops/qubit/__init__.py 100.00% <0.00%> (ø)
pennylane/ops/qubit/qchem_ops.py 100.00% <0.00%> (ø)
pennylane/tape/cv_param_shift.py 100.00% <0.00%> (ø)
pennylane/devices/default_mixed.py 100.00% <0.00%> (ø)
... and 10 more

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 53ff922...c27f50d. Read the comment docs.

@antalszava antalszava marked this pull request as ready for review March 4, 2022 02:43
@antalszava antalszava requested a review from josh146 March 4, 2022 02:43
Comment on lines 638 to 645
def __invert__(self):
"""Inverts the value of the measurement value."""
zero = self._zero_case
one = self._one_case

self._control_value = one if self._control_value == zero else zero

return self
Copy link
Member

Choose a reason for hiding this comment

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

very clean! Minor comment: would the docstring be more accurate to say

"""Inverts the control value of the measurement."""
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, added.

Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Nice work @antalszava! Amazing that you have added important extensions so quickly

pennylane/transforms/condition.py Outdated Show resolved Hide resolved
pennylane/transforms/condition.py Outdated Show resolved Hide resolved
pennylane/transforms/condition.py Outdated Show resolved Hide resolved
tests/test_measurements.py Show resolved Hide resolved
Comment on lines +23 to +24
class TestCond:
"""Tests that verify that the cond transform works as expect."""
Copy link
Member

Choose a reason for hiding this comment

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

Minor: the name of this test class, the docstring, and the fact that it is the only test class, imply that it is not needed. You could remove it and just have test functions in this file.

Copy link
Contributor Author

@antalszava antalszava Mar 4, 2022

Choose a reason for hiding this comment

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

There is now a class TestOtherTransforms in the file too 😄 What would be the benefit of not having a class btw?

Copy link
Member

Choose a reason for hiding this comment

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

What would be the benefit of not having a class btw?

Oh it's just that test classes don't really do anything, their main use is simply categorization of multiple tests within a module.

I find that if there isn't any categorization needed, it's cleaner to just have test functions.

pennylane/transforms/condition.py Show resolved Hide resolved
tests/transforms/test_defer_measurements.py Show resolved Hide resolved
tests/transforms/test_defer_measurements.py Outdated Show resolved Hide resolved
tests/transforms/test_defer_measurements.py Show resolved Hide resolved
tests/transforms/test_defer_measurements.py Show resolved Hide resolved
@antalszava
Copy link
Contributor Author

[sc-15616]

@antalszava antalszava requested a review from josh146 March 4, 2022 15:55
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

@antalszava this is great! Super nice, very excited about this PR.

I'm happy to approve, but only have one major comment regarding the docstring example section.

Ideally, documentation should be contained within the same PR as the feature (as we do with tests), but due to the tight deadline, I understand if this PR is blocking others - as long as finishing the docstring isn't forgotten!

pennylane/transforms/condition.py Outdated Show resolved Hide resolved
assert ops[2].then_op.wires == target_wire
assert ops[2].then_op.data == [r]

assert isinstance(ops[3], qml.transforms.condition.Conditional)
Copy link
Member

Choose a reason for hiding this comment

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

💪

assert isinstance(op2.then_op, qml.RY)
assert op1.then_op.data == [r]

assert ops[2].return_type == qml.operation.Probability
Copy link
Member

Choose a reason for hiding this comment

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

Really awesome testing!

tests/transforms/test_defer_measurements.py Show resolved Hide resolved
function: A new function that applies the conditional equivalent of ``then_op``. The returned
function takes the same input arguments as ``then_op``.
function: A new function that applies the conditional equivalent of ``true_fn``. The returned
function takes the same input arguments as ``true_fn``.

**Example**
Copy link
Member

Choose a reason for hiding this comment

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

@antalszava the example here is not up to our usual standards for docstring examples (e.g., it doesn't contain any text explaining the context/code, and showing outputs).

It also seems to be missing:

  • examples with qfuncs,
  • examples with then/else, and
  • examples showing how to deal with conditional functions with different signatures

Co-authored-by: Josh Izaac <josh146@gmail.com>
@antalszava antalszava merged commit 0308d78 into conditional-ops-alpha-sanitized Mar 4, 2022
@antalszava antalszava deleted the extend_qml_cond branch March 4, 2022 19:32
antalszava added a commit that referenced this pull request Mar 4, 2022
* better

* fixed

* fxied

* fixed

* fixed

* fixed

* fxied

* revert

* fixed

* cleaner syntax

* cleaner syntax

* better naming

* better naming

* fixed code factor

* fixed black

* fixed docs

* fixed types

* fixed

* progress

* fixed docs

* better

* simplify

* simpler now

* better design

* revert

* fixed

* better design

* optimized

* progress

* now this is working

* progress

* progress

* simplify

* better naming

* progress

* added for docs

* progress

* fixed lint

* fixing docs

* fixed black

* fixed linting

* consistancy

* simplify

* progress

* simplify

* defer_measurement transform & add a measured_qubit attribute

* adjust tests

* tests

* extend reset

* refactor

* better

* progress

* dont need

* dont need

* progress

* seems to be working

* fixed

* progress

* progress

* progress

* progress

* progress

* progress

* might be a good check

* simplify

* Update pennylane/ops/mid_circuit_measure.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* fixed

* progress

* fixed

* progress

* progress

* progress

* almost there

* ok this works

* fixed

* broken for some reason

* progress

* simple

* I think simpler

* simpler

* ok

* better design

* fixed

* progress

* progress

* progress

* broken for now

* we need to go back to multiple measurements per circuit

* progress

* now we have a working op

* license

* imports

* no condition or apply mdv

* no extras nor tests

* merge function into the main func

* more tests and boilerplate

* less

* format

* format

* more tests

* license

* drawing test

* tests

* docstring

* docstring

* format

* template test

* format

* kwarg test

* basis state prep test

* CV test, format

* docstring

* angle embedding

* format

* section on measurements page

* lint format

* lint format

* lint format

* lint format

* remove str for now (unused)

* changing to qml.If and making some changes to qml.measure

* qml.measure module ref -> qml.measurements

* measure module ref -> measurements in tests

* format

* qml.cond; qml.Conditional under the hood

* lint; format

* include the wires that have already been measured in the error message

* correct example

* reorganize defs

* reorganize defs

* Added condition.py

* formatting

* Apply the defer measurements transform by default in the QNode

* add test already measured qubit error

* format

* qml.state and the defer_measurements transform

* use a MeasurementProcess class

* format

* put mid-circuit measurement processes into tape._ops for now

* format

* pennylane/measure.py -> pennylane/measurements.py

* changelog

* no need for pennylane/ops/mid_circuit_measure.py anymore

* format

* interface tests

* draft else op, __eq__ and the likes

* docstring

* renamed:    tests/test_measure.py -> tests/test_measurements.py

* MeasurementDependantValue -> MeasurementValue

* assertion tests

* then_op

* condition using measurement outcome

* format

* no else_op for now

* formatting

* update the docs part

* xfail for now

* rephrase

* test mcm and conditional via the device base class

* too many wires passed to qml.measure

* format

* linting

* lint

* lint

* format

* measure docstring

* Conditional docstring

* JAX grad test

* control on one case

* newline

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/_device.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* RY on 0

* link to def meas princ

* RY ref

* otherwise

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* amend desc

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* revert meas check condition

* add cond to operator fun

* Update tests/test_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/condition.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* measurement value

* Update pennylane/transforms/condition.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/condition.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* amend qml.cond docstring

* no need to apply transform

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* add note to def meas transform

* use qml.cond and qml.measure in the example

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* var, probs, sample tests

* Add ref to mcm doc section

* Test on lightning.qubit too

* format

* changelog

* changelog

* adjust the logic in defer_measurements to allow m == 0 case; add integration tests; add lightning to the list of tested devices

* format

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/introduction/measurements.rst

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update doc/releases/changelog-dev.md

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/condition.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* Update pennylane/transforms/defer_measurements.py

Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>

* def meas link

* TODO: move expansion onto the device

* Extend `qml.cond` (#2275)

* implement and test __invert__

* add else_op to qml.cond

* using inversion in an integration

* format

* copy measurement under the hood when else; else test

* format

* err tests

* format

* docstrings

* isort

* test_condition

* docstrings

* format

* no need for separate logic for ops

* refactor wrapper as per Josh's suggestion from code review

* Update tests/transforms/test_defer_measurements.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

* two unit tests for checking qml.cond queuing

* add queue unit tests

* format

* invert docstring

* rename arguments

* module docstring

* module docstring: note where integration tests are

* changelog

* Update pennylane/transforms/condition.py

Co-authored-by: Josh Izaac <josh146@gmail.com>

Co-authored-by: Josh Izaac <josh146@gmail.com>

Co-authored-by: Samuel Banning <samcbanning@gmail.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
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

2 participants