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.34k stars 601 forks source link

[BUG] Custom gradient recipe with multiple 0 shifts for same gate fails with `param_shift` transform #3174

Closed antalszava closed 1 year ago

antalszava commented 2 years ago

Expected behavior

Any custom gradient recipe executes without errors.

Actual behavior

An indexing error is raised for a gate with the custom recipe of [[-1e7, 1, 0], [1e7, 1, 0]].

Additional information

No response

Source code

import pennylane as qml
import numpy as np

dev = qml.device("default.qubit", wires=2)
x = [0.543, -0.654]

ops_with_custom_recipe = [0]

with qml.tape.QuantumTape() as tape:
    qml.RX(x[0], wires=[0])
    qml.RX(x[1], wires=[0])
    qml.expval(qml.PauliZ(0))

gradient_recipes = tuple(
    [[-1e7, 1, 0], [1e7, 1, 0]] if i in ops_with_custom_recipe else None
    for i in range(2)
)
tapes, fn = qml.gradients.param_shift(tape, gradient_recipes=gradient_recipes)

# two tapes per parameter that doesn't use a custom recipe,
# one tape per parameter that uses custom recipe,
# plus one global call if at least one uses the custom recipe
num_ops_standard_recipe = tape.num_params - len(ops_with_custom_recipe)
assert len(tapes) == 2 * num_ops_standard_recipe + len(ops_with_custom_recipe) + 1
# Test that executing the tapes and the postprocessing function works
grad = fn(qml.execute(tapes, dev, None))
assert qml.math.allclose(grad, -np.sin(x[0] + x[1]), atol=1e-5)

Tracebacks

~/pennylane/gradients/parameter_shift.py in param_shift(tape, argnum, shifts, gradient_recipes, fallback_fn, f0, broadcast)
   1572         g_tapes, fn = var_param_shift(tape, argnum, shifts, gradient_recipes, f0, broadcast)
   1573     else:
-> 1574         g_tapes, fn = expval_param_shift(tape, argnum, shifts, gradient_recipes, f0, broadcast)
   1575 
   1576     gradient_tapes.extend(g_tapes)

~/pennylane/gradients/parameter_shift.py in expval_param_shift(tape, argnum, shifts, gradient_recipes, f0, broadcast)
    489         # If broadcast=True, g_tapes only contains one tape. If broadcast=False, all returned
    490         # tapes will have the same batch_size=None. Thus we only use g_tapes[0].batch_size here.
--> 491         gradient_data.append((len(g_tapes), coeffs, None, unshifted_coeff, g_tapes[0].batch_size))
    492 
    493     def processing_fn(results):

IndexError: tuple index out of range

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: /xanadu/pennylane
Requires: appdirs, autograd, autoray, cachetools, networkx, numpy, pennylane-lightning, retworkx, scipy, semantic-version, toml
Required-by: amazon-braket-pennylane-plugin, PennyLane-AQT, PennyLane-Cirq, PennyLane-Forest, PennyLane-Honeywell, PennyLane-IonQ, PennyLane-Lightning, PennyLane-Orquestra, PennyLane-PQ, PennyLane-Qchem, PennyLane-qiskit, PennyLane-qsharp, pennylane-qulacs, PennyLane-SF

Platform info:           Linux-5.15.0-50-generic-x86_64-with-glibc2.10
Python version:          3.8.5
Numpy version:           1.23.0
Scipy version:           1.8.1
Installed devices:
- braket.aws.qubit (amazon-braket-pennylane-plugin-1.7.0)
- braket.local.qubit (amazon-braket-pennylane-plugin-1.7.0)
- lightning.qubit (PennyLane-Lightning-0.26.0)
- forest.numpy_wavefunction (PennyLane-Forest-0.14.0)
- forest.qvm (PennyLane-Forest-0.14.0)
- forest.wavefunction (PennyLane-Forest-0.14.0)
- orquestra.forest (PennyLane-Orquestra-0.15.0)
- orquestra.ibmq (PennyLane-Orquestra-0.15.0)
- orquestra.qiskit (PennyLane-Orquestra-0.15.0)
- orquestra.qulacs (PennyLane-Orquestra-0.15.0)
- qulacs.simulator (pennylane-qulacs-0.17.0.dev0)
- aqt.noisy_sim (PennyLane-AQT-0.18.0)
- aqt.sim (PennyLane-AQT-0.18.0)
- projectq.classical (PennyLane-PQ-0.18.0.dev0)
- projectq.ibm (PennyLane-PQ-0.18.0.dev0)
- projectq.simulator (PennyLane-PQ-0.18.0.dev0)
- microsoft.QuantumSimulator (PennyLane-qsharp-0.19.0)
- strawberryfields.fock (PennyLane-SF-0.21.0.dev0)
- strawberryfields.gaussian (PennyLane-SF-0.21.0.dev0)
- strawberryfields.gbs (PennyLane-SF-0.21.0.dev0)
- strawberryfields.remote (PennyLane-SF-0.21.0.dev0)
- strawberryfields.tf (PennyLane-SF-0.21.0.dev0)
- ionq.qpu (PennyLane-IonQ-0.24.0.dev0)
- ionq.simulator (PennyLane-IonQ-0.24.0.dev0)
- honeywell.hqs (PennyLane-Honeywell-0.19.0.dev0)
- qiskit.aer (PennyLane-qiskit-0.23.0.dev0)
- qiskit.basicaer (PennyLane-qiskit-0.23.0.dev0)
- qiskit.ibmq (PennyLane-qiskit-0.23.0.dev0)
- qiskit.ibmq.circuit_runner (PennyLane-qiskit-0.23.0.dev0)
- qiskit.ibmq.sampler (PennyLane-qiskit-0.23.0.dev0)
- cirq.mixedsimulator (PennyLane-Cirq-0.23.0.dev0)
- cirq.pasqal (PennyLane-Cirq-0.23.0.dev0)
- cirq.qsim (PennyLane-Cirq-0.23.0.dev0)
- cirq.qsimh (PennyLane-Cirq-0.23.0.dev0)
- cirq.simulator (PennyLane-Cirq-0.23.0.dev0)
- 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)
- null.qubit (PennyLane-0.27.0.dev0)

Existing GitHub issues

antalszava commented 2 years ago

@dwierichs any thoughts on the reason for the issue and/or if it could be a potentially significant use case? :thinking:

dwierichs commented 2 years ago

The problem here is not that there are multiple zero-shift terms, it is that there are no nonzero-shift terms :) Terms with the same shift (and multiplier) are actually grouped together before creating the tapes.

This bug happens because the code inherently assumes that a non-zero shift will be needed to obtain the derivative (this assumption is not used for Hamiltonian parameters, just for operations). For all operations I know this makes sense, but maybe there are some for which it doesn't. It would not be hard to change this, should I do so? Or would we prefer a warning? I would not see any silent bug that would result from the former, i.e. just allowing derivatives with non-zero shifts only. In particular, this would also allow "dummy" recipes without any terms at all, for whatever reason one might want to use those :grin:

antalszava commented 2 years ago

Frankly, I think any type of handling of the case (either changing the behaviour and/or having a warning) could be nice. I suspect it's not going to be a case widely stumbled upon, though it could confuse some users because it raises an error.

dwierichs commented 2 years ago

Just making sure: The number of tapes will be 2*num_ops_standard_recipe+int(len(ops_with_custom_recipe)>0), right? 2 shifted evaluations per "normal" recipe operation and one global one if at least one op has the custom recipe.

Also: This recipe doesn't work, so the last assertion should not go through, correct? :)