search

PennyLaneAI / pennylane

PennyLane is a cross-platform Python library for quantum computing, quantum machine learning, and quantum chemistry. Train a quantum computer the same way as a neural network.
https://pennylane.ai
Apache License 2.0
2.27k stars 585 forks source link

[BUG] `QuantumTape.diagonalizing_gates` returns wrong result when observables include `Hamiltonian` #3082

Open dwierichs opened 1 year ago

dwierichs commented 1 year ago

Expected behavior

The property QuantumTape.diagonalizing_gates always either returns the correct diagonalizing gates or raises an error (warning) if they are unknown.

Actual behavior

When measuring, say, the expectation value or variance of a Hamiltonian, the tape always reports diagonalizing_gates=[], which is not correct whenever the Hamiltonian is not diagonal.

Additional information

This is due to the code in QuantumTape.diagonalizing_gates itself, where the observable's diagonalizing_gates are queried, and if an error is raised (due to unknown diagonalizing gates), it is simply suppressed.

I suggest that we either raise a warning, or better a DiagGatesUndefinedError. The method clearly does not return what it promises in the described case. The reason why these wrong diagonalizing gates do not cause problems in circuit evaluations is because DefaultQubit has a separate logic for Hamiltonian and SparseHamiltonian, which actually relies on the fact that we skip the rotation to the eigenbasis in QubitDevice.execute. This is an dependency between QuantumTape.diagonalizing_gates, QubitDevice.execute and DefaultQubit.expval, which is likely to cause (or have caused) trouble at some point for developers that build their device on QubitDevice instead of DefaultQubit (as they are supposed to).

More generally: I think there might have been a misunderstanding regarding the meaning of diagonalizing_gates raising the error DiagGatesUndefinedError. As far as I can tell it means "We do not know the diagonalizing gates", which most likely is because they are expensive to compute. The code in QuantumTape.diagonalizing_gates treats this error as "We know the diagonalizing gates to be the empty set". We could/should make sure that there is a uniform understanding of the DiagGatesUndefinedError, and that the misunderstanding/difference in interpretation didn't make it into the code base in any other way.

Source code

>>> o = qml.Hamiltonian([0.4], [qml.PauliX(0)])

>>> with qml.tape.QuantumTape() as tape:
...     qml.expval(o)

>>> tape.diagonalizing_gates
[]

Tracebacks

No response

System information

Name: PennyLane
Version: 0.27.0.dev0
Summary: PennyLane is a Python quantum machine learning library by Xanadu Inc.
Home-page: https://github.com/XanaduAI/pennylane
Author: 
Author-email: 
License: Apache License 2.0
Location: /home/david/venvs/xanadu3.10/lib/python3.10/site-packages/PennyLane-0.27.0.dev0-py3.10.egg
Requires: appdirs, autograd, autoray, cachetools, networkx, numpy, pennylane-lightning, retworkx, scipy, semantic-version, toml
Required-by: amazon-braket-pennylane-plugin, PennyLane-Cirq, PennyLane-Lightning, PennyLane-PQ, PennyLane-qiskit, pennylane-qulacs

Platform info:           Linux-5.4.0-125-generic-x86_64-with-glibc2.31
Python version:          3.10.7
Numpy version:           1.23.1
Scipy version:           1.9.0rc2
Installed devices:
- braket.aws.qubit (amazon-braket-pennylane-plugin-1.6.9)
- braket.local.qubit (amazon-braket-pennylane-plugin-1.6.9)
- lightning.qubit (PennyLane-Lightning-0.25.0)
- projectq.classical (PennyLane-PQ-0.17.0)
- projectq.ibm (PennyLane-PQ-0.17.0)
- projectq.simulator (PennyLane-PQ-0.17.0)
- qulacs.simulator (pennylane-qulacs-0.13.0.dev0)
- cirq.mixedsimulator (PennyLane-Cirq-0.25.0.dev0)
- cirq.pasqal (PennyLane-Cirq-0.25.0.dev0)
- cirq.qsim (PennyLane-Cirq-0.25.0.dev0)
- cirq.qsimh (PennyLane-Cirq-0.25.0.dev0)
- cirq.simulator (PennyLane-Cirq-0.25.0.dev0)
- qiskit.aer (PennyLane-qiskit-0.25.0.dev0)
- qiskit.basicaer (PennyLane-qiskit-0.25.0.dev0)
- qiskit.ibmq (PennyLane-qiskit-0.25.0.dev0)
- qiskit.ibmq.circuit_runner (PennyLane-qiskit-0.25.0.dev0)
- qiskit.ibmq.sampler (PennyLane-qiskit-0.25.0.dev0)
- quantumag.redtrap (pennylane-umz-0.2.0)
- default.gaussian (PennyLane-0.27.0.dev0)
- default.mixed (PennyLane-0.27.0.dev0)
- default.qubit (PennyLane-0.27.0.dev0)
- default.qubit.autograd (PennyLane-0.27.0.dev0)
- default.qubit.jax (PennyLane-0.27.0.dev0)
- default.qubit.tf (PennyLane-0.27.0.dev0)
- default.qubit.torch (PennyLane-0.27.0.dev0)
- default.qutrit (PennyLane-0.27.0.dev0)

Existing GitHub issues

antalszava commented 1 year ago

Devices that do not support qml.Hamiltonian natively will have such objects expanded: https://github.com/PennyLaneAI/pennylane/blob/master/pennylane/_device.py#L738

This essentially means that in such cases it's not qml.Hamiltonian.diagonalizing_gates but, the diagonalizing_gates attribute of the terms themselves will be called.

Overall, it seems that this behaviour is indeed off, but with the potential superseding of the qml.Hamiltonian class by operator arithmetic, it's questionable if we'd like to take any immediate steps here. For example, if we raised a warning/error, it would have to be suppressed anyways during device execution (or could there be another way to that? :thinking:).

dwierichs commented 1 year ago

Thanks @antalszava!

I agree, this is a bug with very low visibility. Conceptually, I'd prefer if there is an error raised and a device that supports Hamiltonian measurements suppresses it, to be on the safe side. However, this should be traded off against

  1. this being a breaking change, requiring action by all devices/plugin devices that support Hamiltonian measurement directly, i.e. not via decomposition.
  2. Hamiltonian maybe being replaced via arithmetic.
dwierichs commented 1 year ago

I think this can be left as-is due to the above reasons of low visibility. Leaving open to provide a reference for this behaviour.

timmysilv commented 1 year ago

if anyone else finds their way here, this PR is related: #3938