-
Notifications
You must be signed in to change notification settings - Fork 575
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
Fix metric tensor for circuits with inverted gates #1987
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1987 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 227 227
Lines 17291 17293 +2
=======================================
+ Hits 17086 17088 +2
Misses 205 205
Continue to review full report at Codecov.
|
if op.inverse: | ||
s = -s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dwierichs, I'm wondering: should we fix this behaviour instead by negating the scaling factor on the operation level?
I.e., a potential solution would be to make generator
attribute a property:
For qml.RX
, we'd go from
generator = [PauliX, -1 / 2]
to
@property
def generator(self):
scaling_factor = -1 / 2 if not self.inverse 1 / 2
return [PauliX, scaling_factor]
I think this would be the intuitive thing for a user, given the docstring of the generator
attribute.
Example
If a user would like to naively apply the formula from the docstring, they could do so by gathering the gate parameter and the generator.
op = qml.RY(0.3, wires=0).inv()
print('Theta: ', op.data, '\nInverse? ', op.inverse, '\nGenerator: ', op.generator)
Theta: [0.3]
Inverse? True
Generator: [<class 'pennylane.ops.qubit.non_parametric_ops.PauliY'>, -0.5]
This is where the issue already arises: we store the information about taking the inverse of the operation in the op.inverse
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this change would:
- Require changing the
generator
attribute on a case by case basis for several operations; - Would likely affect the operation derivative capability (see here) (and maybe other places too?);
- Potentially be revisited: having both
.inv()
and theadjoint
method of an operation act separately seems to be causing strange situations (e.g.,inv()
only flips a flag, whereasadjoint()
may negate the gate parameter).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antalszava the generator attribute is due to be refactored next week as part of the operator overhaul! So a hotfix for this release should be okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm all in for deprecating the inverse
flag 😅 I found it a bit counter-intuitive in all scenarios in which I came across it so far...
But I think it might be unnecessary to fix this now, given the changes that soon will be made to the Operator class. @mariaschuld what do you think about this? Would it pay off to address this issue now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍 The changes here looked good as a hotfix.
* Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com>
…1979) * check supported operations in more places * add PauliX to measurment qnodes * Incrementing the version number to `0.21.0-dev` (#1988) * changelogs * changelog * version number bumps * clean changelog file * RC to master (#1995) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * eliminate empty circuits, minor rewriting * black, changelog, fix probs * fix merging with rc branch * state non empty circuit Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com>
* Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests * Resolve error from texted-based circuit drawer for tapes (#1994) * Add support for drawing tapes to textdrawer Tapes appearing in the operations list of circuits (or other tapes) are now drawn as an opaque tape object, and its contents are appended to the circuit drawing as a subcircuit. * Fix charset of tape subcircuits * Add persistent indices to ops in nested circuits When a circuit to be printed contains tapes nested within, the index printed to identify each tape, matrix argument, etc. is now passed to lower levels of nesting (and passed back up) to retain unique names for circuit elements. * Add tests for drawing nested tapes * Changelog * Review: docstrings, arg name, generator * Make multi_dispatch treat scipy same as numpy (#2001) * Make multi_dispatch treat scipy same as numpy * Changelog * Add test to assert warning is no longer raised * Use pytest.mark instead of warnings module for filter Adding a warning filter using the warnings module seems to persist across unit tests. * Remove empty circuits and device tests to check supported operations (#1979) * check supported operations in more places * add PauliX to measurment qnodes * Incrementing the version number to `0.21.0-dev` (#1988) * changelogs * changelog * version number bumps * clean changelog file * RC to master (#1995) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * eliminate empty circuits, minor rewriting * black, changelog, fix probs * fix merging with rc branch * state non empty circuit Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * Update doc/releases/changelog-dev.md * Update pennylane/drawer/representation_resolver.py * Update pennylane/drawer/representation_resolver.py * have a single self.label_offsets dictionary for linting purposes * organize tape drawing into its own method * no too many return statements check * format Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: antalszava <antalszava@gmail.com>
* Merge release candidate into master (#2003) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests * Resolve error from texted-based circuit drawer for tapes (#1994) * Add support for drawing tapes to textdrawer Tapes appearing in the operations list of circuits (or other tapes) are now drawn as an opaque tape object, and its contents are appended to the circuit drawing as a subcircuit. * Fix charset of tape subcircuits * Add persistent indices to ops in nested circuits When a circuit to be printed contains tapes nested within, the index printed to identify each tape, matrix argument, etc. is now passed to lower levels of nesting (and passed back up) to retain unique names for circuit elements. * Add tests for drawing nested tapes * Changelog * Review: docstrings, arg name, generator * Make multi_dispatch treat scipy same as numpy (#2001) * Make multi_dispatch treat scipy same as numpy * Changelog * Add test to assert warning is no longer raised * Use pytest.mark instead of warnings module for filter Adding a warning filter using the warnings module seems to persist across unit tests. * Remove empty circuits and device tests to check supported operations (#1979) * check supported operations in more places * add PauliX to measurment qnodes * Incrementing the version number to `0.21.0-dev` (#1988) * changelogs * changelog * version number bumps * clean changelog file * RC to master (#1995) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * eliminate empty circuits, minor rewriting * black, changelog, fix probs * fix merging with rc branch * state non empty circuit Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * Update doc/releases/changelog-dev.md * Update pennylane/drawer/representation_resolver.py * Update pennylane/drawer/representation_resolver.py * have a single self.label_offsets dictionary for linting purposes * organize tape drawing into its own method * no too many return statements check * format Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: antalszava <antalszava@gmail.com> * Style2 Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: antalszava <antalszava@gmail.com>
* Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests * Resolve error from texted-based circuit drawer for tapes (#1994) * Add support for drawing tapes to textdrawer Tapes appearing in the operations list of circuits (or other tapes) are now drawn as an opaque tape object, and its contents are appended to the circuit drawing as a subcircuit. * Fix charset of tape subcircuits * Add persistent indices to ops in nested circuits When a circuit to be printed contains tapes nested within, the index printed to identify each tape, matrix argument, etc. is now passed to lower levels of nesting (and passed back up) to retain unique names for circuit elements. * Add tests for drawing nested tapes * Changelog * Review: docstrings, arg name, generator * Make multi_dispatch treat scipy same as numpy (#2001) * Make multi_dispatch treat scipy same as numpy * Changelog * Add test to assert warning is no longer raised * Use pytest.mark instead of warnings module for filter Adding a warning filter using the warnings module seems to persist across unit tests. * Remove empty circuits and device tests to check supported operations (#1979) * check supported operations in more places * add PauliX to measurment qnodes * Incrementing the version number to `0.21.0-dev` (#1988) * changelogs * changelog * version number bumps * clean changelog file * RC to master (#1995) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * eliminate empty circuits, minor rewriting * black, changelog, fix probs * fix merging with rc branch * state non empty circuit Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * Update doc/releases/changelog-dev.md * Update pennylane/drawer/representation_resolver.py * Update pennylane/drawer/representation_resolver.py * have a single self.label_offsets dictionary for linting purposes * organize tape drawing into its own method * no too many return statements check * format * [WIP] Jit probs bug (#1998) * trying to use jax unique function to allow for compilation * testing * Overrided the estimate_probability method in the default_qubit_jax device to allow for jit-ing when working with qml.probs * Apply suggestions from code review * missed comment * lint * Added tests * fixed bug with fill value * cleaning up * Fix missing function call in metric tensor example (#2008) * Update docs of `qml.fourier.reconstruct` and `qml.QubitDensityMatrix` (#2005) * reconstruct fixes * QubitDensityMatrix fix * revert merge master commit * minor doc fixes (#2009) * fix typo in CVNeuralNetLayers UsageDetails * Fix some minor doc example for drawer (#2010) * Merge release candidate into master (#2003) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests * Resolve error from texted-based circuit drawer for tapes (#1994) * Add support for drawing tapes to textdrawer Tapes appearing in the operations list of circuits (or other tapes) are now drawn as an opaque tape object, and its contents are appended to the circuit drawing as a subcircuit. * Fix charset of tape subcircuits * Add persistent indices to ops in nested circuits When a circuit to be printed contains tapes nested within, the index printed to identify each tape, matrix argument, etc. is now passed to lower levels of nesting (and passed back up) to retain unique names for circuit elements. * Add tests for drawing nested tapes * Changelog * Review: docstrings, arg name, generator * Make multi_dispatch treat scipy same as numpy (#2001) * Make multi_dispatch treat scipy same as numpy * Changelog * Add test to assert warning is no longer raised * Use pytest.mark instead of warnings module for filter Adding a warning filter using the warnings module seems to persist across unit tests. * Remove empty circuits and device tests to check supported operations (#1979) * check supported operations in more places * add PauliX to measurment qnodes * Incrementing the version number to `0.21.0-dev` (#1988) * changelogs * changelog * version number bumps * clean changelog file * RC to master (#1995) * Fix metric tensor for circuits with inverted gates (#1987) * fix metric tensor for inverse gates * changelog * modify test for block diagonal * undo tmp change * AmplitudeEmbedding: Fix warning from implicit cast of complex to real (#1990) * Fix warning from implicit cast of complex to real * Changelog * Fix warning without failing other tests * V0.20.0: remove deprecated features (#1981) * remove the decorator * remove the decorator tests * remove the remaining template decorator use * Remove the default.tensor devices * no mention of default.tensor.tf * remove fourier.spectrum * no default.tensor entry points * no beta devices ref in docs * remove the diag_approx keyword argument of qml.metric_tensor and qml.QNGOptimizer * changelog * changelog formatting * Remove the metric_tensor method of the old QNode; remove tests and adjust an example for QNG * remove unused variable * Fix error mitigation tests Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * eliminate empty circuits, minor rewriting * black, changelog, fix probs * fix merging with rc branch * state non empty circuit Co-authored-by: antalszava <antalszava@gmail.com> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> * Update doc/releases/changelog-dev.md * Update pennylane/drawer/representation_resolver.py * Update pennylane/drawer/representation_resolver.py * have a single self.label_offsets dictionary for linting purposes * organize tape drawing into its own method * no too many return statements check * format Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: antalszava <antalszava@gmail.com> * Style2 Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: antalszava <antalszava@gmail.com> * Tape validation for the JAX interface (batch execute) (#2011) * add tape validation to the JAX interface to provide users with a descriptive error message * formatting * changelog * Fix behaviour of `qml.gradients.gradient_transform` for multiple array arguments (#1989) * Fix gradient_transform for complex QNode shapes Previously, the gradient transform did not account for the shape of classical jacobians produced by `qml.jacobian` for complex QNode argument scenarious, leading to a shape mismatch error during tensor contraction of the classical and quantum jacobian. This is resolved by distinguishing between the case of a single classical Jacobian for a single QNode argument, and multiple classical Jacobians stacked together when all QNode arguments have the same shape. The two cases require different tensor manipulations as `qml.jacobian` behaves differently in each. Two test cases are added that include multiple vector arguments to a QNode and non-trainable QNode arguments mixed with trainable ones. * Format * Changelog * Fix coverage by removing some "fallback" branches and adding dummy test. * Update doc/releases/changelog-dev.md Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> * Review: small corrections * Proposal: leave old behaviour as fallback branch * Review: remove tf & torch logic, comment * Review: raise warning for unknown cjac shape Co-authored-by: Antal Szava <antalszava@gmail.com> Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> * Fix docstrings for `CommutingEvolution` (#2012) * Example and docstrings * Undo. * Use the correct GPU device internally with `default.qubit.torch` (#1982) * changes * format * docstring * lint * Update pennylane/devices/default_qubit_torch.py Co-authored-by: Christina Lee <christina@xanadu.ai> * Update pennylane/devices/default_qubit_torch.py Co-authored-by: Christina Lee <christina@xanadu.ai> * no cover; extend docstring * format; correct typo * revert * no cover for GPU part * format Co-authored-by: Christina Lee <christina@xanadu.ai> * updated docstring example (#2019) * Fix Barrier example * `v0.20.0` release notes (#1977) * current release * rename latest changelog * reorder the entries, create sections * version, release notes file * Update doc/releases/changelog-0.20.0.md * Update doc/releases/changelog-0.20.0.md Co-authored-by: Josh Izaac <josh146@gmail.com> * Update doc/releases/changelog-0.20.0.md Co-authored-by: Josh Izaac <josh146@gmail.com> * init module: breaking * Apply suggested changes * Apply suggested changes * rendering issues * mutable kwarg * apply suggestions * move to breaking changes * Lie opt example * Update doc/releases/changelog-0.20.0.md Co-authored-by: Josh Izaac <josh146@gmail.com> * transformation example in the notes * updates * Fix examples * fix refs * contributors * More PRs listed * PauliError * PauliError * custom ops as attribute * Update changelog-0.20.0.md * Update changelog-0.20.0.md Co-authored-by: Josh Izaac <josh146@gmail.com> * readd changelog item * correct version numbers * nl * disallow qml.probs with default.qubit.jax when using a shot vector * no else Co-authored-by: David Wierichs <davidwierichs@gmail.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: Christina Lee <christina@xanadu.ai> Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca> Co-authored-by: Romain <rmoyard@gmail.com> Co-authored-by: Josh Izaac <josh146@gmail.com>
Context:
The generator and coefficient stored in
op.generator
are not modified when an operation is inverted. Therefore,using this information in the metric tensor led to the wrong result for inverted gates. This is now fixed and three tests were modified to include this szenario.
Note that this bug affects both, the new Hadamard test-based metric tensor introduced in #1725 and the block-diagonal original version.
Description of the Change:
The coefficient is inverted with a minus sign if the operation in question has the
inverse
flag set toTrue
. The documentation clearly states now that this assumes the generator of the operations for which the metric tensor is computed to be Hermitian.Benefits:
bug fix and better documentation as well as test coverage.
Possible Drawbacks: n/a
Related GitHub Issues: n/a