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

Add warnings for the gradient transforms when there are no trainable parameters #2156

Merged
merged 11 commits into from
Feb 7, 2022

Conversation

dime10
Copy link
Contributor

@dime10 dime10 commented Feb 2, 2022

Similar to the previous changes in #2148 and #2139, we'd like to provide a warning or useful error message when a user attempts to compute metric tensor even though there are no trainable parameters, such as in the following example:

dev = qml.device("default.qubit", wires=3)

@qml.qnode(dev)
def circuit(weights):
    qml.RX(weights[0], wires=0)
    qml.RY(weights[1], wires=0)
    return qml.expval(qml.PauliZ(0) @ qml.PauliZ(1))

weights = [0.1, 0.2]
qml.metric_tensor(circuit)(weights)

Most or all gradient transforms actually throw an error at the moment when this is attempted, unless the hybrid parameter is set to False:

ValueError: need at least one array to stack

so this change will also resolve this bug.

With the proposed changes, a warning is raised and a default value returned instead, in both the QNode and tape execution case:

UserWarning: Attempted to compute the metric tensor of a QNode with no trainable parameters. If this is unintended, please add trainable parameters via the 'requires_grad' attribute.
  warnings.warn(
()

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 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 Feb 2, 2022

Codecov Report

❗ No coverage uploaded for pull request base (v0.21.0-rc0@e50c386). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             v0.21.0-rc0    #2156   +/-   ##
==============================================
  Coverage               ?   99.20%           
==============================================
  Files                  ?      230           
  Lines                  ?    17695           
  Branches               ?        0           
==============================================
  Hits                   ?    17554           
  Misses                 ?      141           
  Partials               ?        0           

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 e50c386...88147f5. Read the comment docs.

@dime10
Copy link
Contributor Author

dime10 commented Feb 3, 2022

One scenario (although not a useful one) where this warning might be superfluous is for empty circuits.

@josh146
Copy link
Member

josh146 commented Feb 3, 2022

I guess technically still true though! Or perhaps you could edit the warning:

Attempted to compute the metric tensor of a tape with no trainable parameters
or no operations ...

Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Thank you @dime10! Looking great overall. 💯 Left a couple of minor suggestions. In addition: could we have tests (e.g., those using QNode) be parametrized based on the auto differentiation frameworks? Checking multiple of them would help us make sure that we warn in the correct cases.

pennylane/gradients/gradient_transform.py Outdated Show resolved Hide resolved
@@ -657,8 +665,7 @@ def _update(data):
# functionality before deprecation.
diff_methods = tape._grad_method_validation("analytic" if fallback_fn is None else "best")
all_params_grad_method_zero = all(g == "0" for g in diff_methods)

if not tape.trainable_params or all_params_grad_method_zero:
Copy link
Contributor

Choose a reason for hiding this comment

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

How come not tape.trainable_params was removed?

The previous change in this file added a case for if argnum is None and not tape.trainable_params.

It seems, however, that we could still have argnum is not None and not tape.trainable_params, correct? If so,

  • The first if that has been added on line 649 would not evaluate to True;
  • Due to omitting not tape.trainable_params on this line (line 661 originally), not evaluating to True here either.

Is that considered with the change?

Copy link
Contributor Author

@dime10 dime10 Feb 7, 2022

Choose a reason for hiding this comment

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

Hmm, in which scenario would argnum be not None but trainable_params be True, argnum = [] ?
It's true I didn't consider such cases, because parameter_shift was setup the same way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, in which scenario would argnum be not None but trainable_params be True, argnum = [] ?

Frankly, I'm not sure. Just noticed that that case is not being covered 🤔

It's true I didn't consider such cases, because parameter_shift is setup the same way.

Interesting. Would be great to know why argnum is None was added there in the first place and why not tape.trainable_params did not suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like here the version I'm using now was introduced to parameter_shift 4 months ago, while the version that was present in parameter_shift_cv is 6 month old according to the blame. Before that they had the same conditionals, so I'm guessing parameter_shift_cv was just overlooked regarding this change.

tests/gradients/test_gradient_transform.py Show resolved Hide resolved

x = np.array([0.1, 0.2, 0.3], requires_grad=False)
# TODO: remove once #2155 is resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Once merged, these lines are worth noting in #2155.

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 :)

tests/transforms/test_metric_tensor.py Outdated Show resolved Hide resolved
tests/gradients/test_param_shift_hessian.py Outdated Show resolved Hide resolved
Co-authored-by: antalszava <antalszava@gmail.com>
pennylane/gradients/hessian_transform.py Outdated Show resolved Hide resolved
pennylane/gradients/param_shift_hessian.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift_cv.py Outdated Show resolved Hide resolved
pennylane/transforms/metric_tensor.py Outdated Show resolved Hide resolved
pennylane/transforms/metric_tensor.py Outdated Show resolved Hide resolved
Copy link
Contributor

@antalszava antalszava left a comment

Choose a reason for hiding this comment

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

Looks good! 💯 Thank you 😊 🚀

@dime10
Copy link
Contributor Author

dime10 commented Feb 7, 2022

[sc-14244]

@dime10 dime10 changed the title Add warnings for the metric tensor when there are no trainable parameters Add warnings for the gradient transforms when there are no trainable parameters Feb 7, 2022
@dime10 dime10 merged commit ce941f3 into v0.21.0-rc0 Feb 7, 2022
@dime10 dime10 deleted the useful_error_metrict branch February 7, 2022 21:50
josh146 added a commit that referenced this pull request Feb 8, 2022
* Run updated version of black on repo (#2140)

* Run updated black on repo

* Also reformat tests

* Dont convert variables to numpy if on interface-specific device (#2136)

* dont unwrap interface devices if diffmethod None

* tests and black

* dont expect warning on test, update changelog

* readability update and test fix

* lint

* Update doc/releases/changelog-dev.md

Co-authored-by: antalszava <antalszava@gmail.com>

* Modify changelog for psi4 bug fix (#2144)

* modify changelog for psi4 bug fix

* move the entry to the QChem changelog

Co-authored-by: Antal Szava <antalszava@gmail.com>

* Add useful error message on empty batch transform (#2139)

* Add error on empty batch transform

When a batch transform collects an empty `params` list, it currently
raises an IndexError. This fix adds a check to raise a useful error
message in cases were there are no trainable parameters and
`all_operations` was not set to True.

Also fix template argument types in docstrings.

* Add test case

* Update changelog-dev.md

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

* Add warning when using `qml.grad`/`qml.jacobian` without any trainable parameters (#2148)

* Add warning for gradients with no trainable params

* Fix tests expecting different warning

* Fix many tests with no trainable params

Catch expected warnings

* Formatting

* Add changelog

* Update doc/releases/changelog-dev.md

* Use quotes instead of backticks

Co-authored-by: antalszava <antalszava@gmail.com>

* changelog typos

* Add `qml.hf.hamiltonian.simplify` to docs (#2162)

* Allow qml.hf.simplify

* dummy change to trigger CI

* PL version bump

* add 0.21.0 file

* Update (#2167)

* create sections

* org

* update

* correct examples

* Num params fix2 (#2135)

* made num_params a class property in all remaining Operations as it indeed is independen of the instance in all currently defined Operations
Changed Operation class to allow sub-classes to define num_params as instance or class property as desired

* made num_params of Identity static

* added a simple test to check that for some operations whose num_params can be a static class property this is actually the case

* updated changelog

* linting

* linting

* take both static and instance num_params property into account when checking number of provided parameters

* black

* Update pennylane/operation.py

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* Update pennylane/operation.py

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* communting evolution cannot (and thus should not) declare num_params on the class level

* moved num_params test to later in Operation.__init__()
added comments to provide more context
fixed typo

* reverting change in operation.py becaseu they are not needed

* test all ways of declaring num_params

* Update doc/releases/changelog-dev.md

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* removed optional test

Co-authored-by: Maria Schuld <maria@xanadu.ai>

* black

* Update doc/releases/changelog-dev.md

Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: Maria Schuld <maria@xanadu.ai>
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
Co-authored-by: antalszava <antalszava@gmail.com>

* Extended list of contributors

* QChem

* minor

* section names

* Updated non_parametric ops adjoint method and added test (#2133)

* Updated non_parametric ops adjoint method and added test

* changelog

* lint

* fixed testing bug

* lint

* moving updated adjoint logic into the Operation class and override this method in Observable class

* updated black (#2141)

* Add tape to graph conversion for circuit cutting (#2107)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* Apply suggestions from code review

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

* add code review suggestions

* add unit tests

* format

* Apply suggestions from code review

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

* add review suggestions

* Update pennylane/transforms/qcut.py

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

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

* add fix and tests (#2145)

* Incrementing the version number to `0.22.0-dev` (#2142)

* clean changelog-dev

* version bump

* mark current and dev release in the changelog

* Add Qchem v0.21.0 release notes

* Over wrote adjoint method for these templates as they inherit from Operations

* implemented adjoint method for a few more templates

* `WireCut` nodes can be replaced with `MeasureNode` and `PrepareNode` (#2124)

* add WireCut operator, add qcut package, update docs

* add unit test

* update changelog

* updates

* add tape to graph conversion and unit test

* add unit tests

* add obs to node tests

* restructure qcut module

* add tests for observable and measurement conversion to nodes, update stateprep conversion logic

* fix pylint

* add method to replace Wirecut ops with Measure and Prepare ops

* add unit tests

* add unit tests, update changelog

* format

* set measure and prepare node order to be equal (remove 0.5)

* Apply suggestions from code review

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

* add code review suggestions

* add unit tests

* format

* format

* Apply suggestions from code review

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

* add review suggestions

* Update pennylane/transforms/qcut.py

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

* Apply suggestions from code review

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

* add review suggestions

* add unit test

* update changelog

* update changelog-0.21.0

* Apply suggestions from code review

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

* apply review suggestions

* Apply suggestions from code review

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

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

* Update changelog-0.21.0.md (#2149)

Co-authored-by: antalszava <antalszava@gmail.com>

* reset to previous soln

* clean

* more cleaning

* addressing code-review comments

* lint

* remove qcut feature

* reset changelogs

* reset logs

* updated changelog and qchem version

Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: antalszava <antalszava@gmail.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.com>

* section name and remove imports

* add paper ref

* tapering example

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Breaking changes

* Improvements order

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* no well known imports

* apply latest suggestion to the tapering example

* JAX

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* trainable weights

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Roto move up

* PR link move up

* Apply suggested headlines

* Apply Rotosolve rephrasing

* emojis

* Fix `dev.num_executions` bug with QNode caching (#2171)

* fix & test

* format

* changelog

* use _tape_cached private attribute on the QNode

* format

* PR number

* changelog

* remove unnecessary comment

* Update doc/releases/changelog-dev.md

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

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

* fixed bug when trying to insert around an observable (#2172)

* fixed bug when trying to insert around an observable (or any operation that also inherits from other classes), added tests

* fixed bug in test

* fixing bug in test again

* quick typo fix

* changelog

Co-authored-by: antalszava <antalszava@gmail.com>

* Roto description

* Update doc/releases/changelog-0.21.0.md

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

* black examples

* Update doc/releases/changelog-0.21.0.md

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

* remove new lines and suggested sentence

* apply suggestion

* full stop

* Add warnings for the gradient transforms when there are no trainable parameters (#2156)

* Add warnings (tape/qnode) with no trainable params

* Generalize warning + add tests

* Expand warning to all gradient transforms

Also fixes one test warning about shots.

* Add changelog

* code review: warning wording, separate asserts

Co-authored-by: antalszava <antalszava@gmail.com>

* code review: warning wording

* Formatting

* Parametrize qnode tests by interface

* Include finite_difference grad transform

use seperate asserts across tests

* Add import skips for interfaces

Co-authored-by: antalszava <antalszava@gmail.com>

* Deprecate `QubitDevice`'s caching (#2154)

* Add warning and test

* no print

* format

* format

* changelog

* add preferred way of caching

* mark current release

* change an example

* require Lightning v0.21.0 or higher in setup.py

* re-add dev changelog

* `v0.21.0` release notes (#2159)

* changelog typos

* PL version bump

* add 0.21.0 file

* create sections

* org

* update

* correct examples

* Extended list of contributors

* QChem

* minor

* section names

* section name and remove imports

* add paper ref

* tapering example

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Breaking changes

* Improvements order

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* no well known imports

* apply latest suggestion to the tapering example

* JAX

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* trainable weights

* Update doc/releases/changelog-0.21.0.md

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

* Update doc/releases/changelog-0.21.0.md

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

* Roto move up

* PR link move up

* Apply suggested headlines

* Apply Rotosolve rephrasing

* emojis

* Roto description

* Update doc/releases/changelog-0.21.0.md

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

* black examples

* Update doc/releases/changelog-0.21.0.md

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

* remove new lines and suggested sentence

* apply suggestion

* full stop

* mark current release

* change an example

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

* Bump the requirement of PennyLane-Lightning to v0.21.0 in `setup.py` (#2177)

* Bump the requirement of PennyLane-Lightning in setup.py

* Update setup.py

* Update setup.py

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

* merge

* more

Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Co-authored-by: Christina Lee <christina@xanadu.ai>
Co-authored-by: soranjh <40344468+soranjh@users.noreply.github.com>
Co-authored-by: Romain <rmoyard@gmail.com>
Co-authored-by: Josh Izaac <josh146@gmail.com>
Co-authored-by: cvjjm <60615188+cvjjm@users.noreply.github.com>
Co-authored-by: Maria Schuld <maria@xanadu.ai>
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
Co-authored-by: anthayes92 <34694788+anthayes92@users.noreply.github.com>
Co-authored-by: Tom Bromley <49409390+trbromley@users.noreply.github.com>
Co-authored-by: Nathan Killoran <co9olguy@users.noreply.github.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

3 participants