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

[BUG] Split non commuting fails with autograd requires_grad=False coefficients #5924

Closed
josh146 opened this issue Jul 2, 2024 · 9 comments · Fixed by #6022
Closed

[BUG] Split non commuting fails with autograd requires_grad=False coefficients #5924

josh146 opened this issue Jul 2, 2024 · 9 comments · Fixed by #6022
Labels
bug 🐛 Something isn't working

Comments

@josh146
Copy link
Member

josh146 commented Jul 2, 2024

The following code, which uses the append_gate transformation present within qml.optimize.AdaptiveOptimizer, works fine with default.qubit:

import pennylane as qml
from pennylane.optimize.adaptive import append_gate
from pennylane import numpy as np

H, qubits = qml.qchem.molecular_hamiltonian(
    ["Li", "H"],
    np.array([0.0, 0.0, 0.0, 0.0, 0.0, 2.969280527]),
    active_electrons=active_electrons,
    active_orbitals=5
)

operator_pool = [qml.CRX(0.0, [0, 1]), qml.CRY(0.0, [1, 2])]

dev = qml.device("qulacs.simulator", wires=10)

@qml.qnode(dev, diff_method="parameter-shift")
def circuit():
    for i in range(3):
        qml.RX(0.5, i)

    return qml.expval(H)
>>> params = np.array([0.1, 0.2], requires_grad=True)
>> qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)
array([-0.00393102, -0.00063065])

However, change the device to qulacs.simulator (which uses the old device API), and an error is raised:

>>> qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)
NonDifferentiableError: -6.917117171256823 is non-differentiable. Set the requires_grad attribute to True.

This means that qml.AdapativeOptimizer cannot be used with devices like qulacs.simulator.

Note that this error appears to be peculiar to the following situation:

  • A Hamiltonian is generated using H = qml.qchem.molecular_hamiltonian(...)
  • The QNode returns qml.expval(H).

If the Hamiltonian is generated manually, the above code works correctly on both devices.

@josh146 josh146 added the bug 🐛 Something isn't working label Jul 2, 2024
@albi3ro
Copy link
Contributor

albi3ro commented Jul 2, 2024

Note that the issue is not particularily with the qulacs device, but with split_non_commuting.

Sticking qml.transforms.split_non_commuting onto the qnode causes the same error.

from pennylane.optimize.adaptive import append_gate
from pennylane import numpy as np

H, qubits = qml.qchem.molecular_hamiltonian(
    ["Li", "H"],
    np.array([0.0, 0.0, 0.0, 0.0, 0.0, 2.969280527]),
    active_electrons=2,
    active_orbitals=5
)

operator_pool = [qml.CRX(0.0, [0, 1]), qml.CRY(0.0, [1, 2])]

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

@qml.transforms.split_non_commuting
@qml.qnode(dev, diff_method="parameter-shift")
def circuit():
    for i in range(3):
        qml.RX(0.5, i)

    return qml.expval(H)

params = np.array([0.1, 0.2], requires_grad=True)
qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)

@albi3ro
Copy link
Contributor

albi3ro commented Jul 2, 2024

This may also be related to #5824

@josh146
Copy link
Member Author

josh146 commented Jul 2, 2024

Thanks @albi3ro! I suspected it was something to do with the Hamiltonian, but didn't realize it was because H had non-commuting terms.

Is the bug present with just split_non_commuting and non-H-commuting-supporting-devices + grad? In which case, we could remove the append_gate transform from the minimal error example

@albi3ro
Copy link
Contributor

albi3ro commented Jul 2, 2024

It has less to do with the qulacs simulator, and more about a device that doesn't natively support hamiltonians.

Why append_gate causes this, i have no idea.

@josh146
Copy link
Member Author

josh146 commented Jul 2, 2024

oops, typo in my message, have updated it!

@albi3ro
Copy link
Contributor

albi3ro commented Jul 3, 2024

Never mind. This has nothing to do with append_gate.

The issue can also be replicated with:

H = pnp.array(0.5, requires_grad=False) * qml.Y(0)

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

@qml.transforms.split_non_commuting
@qml.qnode(dev, diff_method="adjoint")
def circuit(x):
    qml.RX(x, 0)
    return qml.expval(H)
params = pnp.array(0.1, requires_grad=True)
qml.grad(circuit)(params)

@albi3ro albi3ro changed the title [BUG] [BUG] Split non commuting fails with autograd requires_grad=False coefficients Jul 3, 2024
@josh146
Copy link
Member Author

josh146 commented Jul 3, 2024

Thanks, that's great! Much more minimal now

astralcai added a commit that referenced this issue Jul 5, 2024
…5945)

Honestly, I don't really understand what the root issue is, and I don't
really understand why this fixes it.

But it does 🤷

Fixes #5924
[sc-67508]
@austingmhuang austingmhuang reopened this Jul 18, 2024
@austingmhuang
Copy link
Contributor

austingmhuang commented Jul 18, 2024

Sorry for re-opening this issue. The original code block (w/ minor changes) still raises a NonDifferentiableError:

import pennylane as qml
from pennylane.optimize.adaptive import append_gate
from pennylane import numpy as np

H, qubits = qml.qchem.molecular_hamiltonian(
    ["Li", "H"],
    np.array([0.0, 0.0, 0.0, 0.0, 0.0, 2.969280527]),
    active_electrons=2,
    active_orbitals=5
)

operator_pool = [qml.CRX(0.0, [0, 1]), qml.CRY(0.0, [1, 2])]

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

@qml.transforms.split_non_commuting
@qml.qnode(dev, diff_method="adjoint")
def circuit():
    for i in range(3):
        qml.RX(0.5, i)

    return qml.expval(H)

params = np.array([0.1, 0.2], requires_grad=True)
qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)

Error:

>>> qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)
NonDifferentiableError: -6.917117171256823 is non-differentiable. Set the requires_grad attribute to True.

@austingmhuang
Copy link
Contributor

Update:

After some investigation with the help of @soranjh, it appears that the error is raised if the given Hamiltonian has an identity term with a non-zero coefficient.

E.g.

import pennylane as qml
from pennylane.optimize.adaptive import append_gate
from pennylane import numpy as pnp

coeffs = pnp.array([0.2, -0.543, 1], requires_grad=False)
obs = [qml.X(0) @ qml.Z(1), qml.Z(0) @ qml.X(1), qml.I(0)]
H = qml.ops.LinearCombination(coeffs, obs)

operator_pool = [qml.CRX(0.0, [0, 1]), qml.CRY(0.0, [1, 2])]

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

@qml.transforms.split_non_commuting
@qml.qnode(dev, diff_method="adjoint")
def circuit():
    for i in range(3):
        qml.RX(0.5, i)

    return qml.expval(H)

circuit()
params = pnp.array([0.1, 0.2], requires_grad=True)
qml.grad(lambda p: append_gate(circuit, p, operator_pool)())(params)

A proposed solution is in #6022

mudit2812 pushed a commit that referenced this issue Sep 10, 2024
**Context:**
Described in #5924, but
to summarize, `split_non_commuting` does not handle Identity terms with
non-trainable coefficients well, leading to an unexpected
`NonDifferentiableError`.

Although the coefficients of Identity terms (also known as `offset`) is
assumed to be a float given the type-hinting of `_sum_terms()`, it seems
that this is not the case. It is also not the case for `coeffs` which is
assumed to be a list of floats but can actually be a list of tensors.

**Description of the Change:**
Fixes #5924 by setting
`requires_grad` to `True` for offsets when autograd is in use.

**Benefits:**
Fixes #5924

**Possible Drawbacks:**
May have unintended side effects, or perhaps we would like to coerce
`offset` and `coeffs` to be the types in the type hint.

**Related GitHub Issues:**
[sc-67508]

---------

Co-authored-by: Christina Lee <[email protected]>
Co-authored-by: Astral Cai <[email protected]>
mudit2812 pushed a commit that referenced this issue Sep 12, 2024
**Context:**
Described in #5924, but
to summarize, `split_non_commuting` does not handle Identity terms with
non-trainable coefficients well, leading to an unexpected
`NonDifferentiableError`.

Although the coefficients of Identity terms (also known as `offset`) is
assumed to be a float given the type-hinting of `_sum_terms()`, it seems
that this is not the case. It is also not the case for `coeffs` which is
assumed to be a list of floats but can actually be a list of tensors.

**Description of the Change:**
Fixes #5924 by setting
`requires_grad` to `True` for offsets when autograd is in use.

**Benefits:**
Fixes #5924

**Possible Drawbacks:**
May have unintended side effects, or perhaps we would like to coerce
`offset` and `coeffs` to be the types in the type hint.

**Related GitHub Issues:**
[sc-67508]

---------

Co-authored-by: Christina Lee <[email protected]>
Co-authored-by: Astral Cai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants