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] lie_closure not accepting complex coefficients on PauliWords #1318

Closed
qubitzer opened this issue Feb 17, 2025 · 4 comments
Closed

[BUG] lie_closure not accepting complex coefficients on PauliWords #1318

qubitzer opened this issue Feb 17, 2025 · 4 comments

Comments

@qubitzer
Copy link

Pennylane version: 0.40.0

Observed behaviour:

The following code runs into an error:

import pennylane as qml 
from pennylane import X
qml.lie_closure([1j * X(0)])

It works if the complex coefficient 1j is omitted. In this minimal example, it does not matter to omitt the coefficient.
Though, in more involved instances, complex coefficients naturally arise and cannot be omitted without changing the DLA.
For example:

def CX(i,j) : return 0.5 * (1 + Z(i) + X(j) - Z(i) @ X(j))
generators = [CX(0,1) @ Z(1)] 
qml.lie_closure(generators)

Expected behaviour:
Generators with complex coefficients should be accepted in the same way as generators with only real coefficients

An idea for a fix:

I ajdusted the current lie_closure implementation locally into a minimal version that accepts complex coefficients:

def lie_closure(generators):
    dla = qml.pauli.PauliVSpace(generators, dtype=complex)

    old_length = 0
    new_length = initial_length = len(dla)

    while old_length < new_length:
        gens = dla.basis.copy()
        for (op1, op2) in itertools.product(gens[:initial_length], gens[old_length:]):
            res = qml.commutator(op1, op2)/2
            res = res.simplify() 

            if not (isinstance(res, SProd) and res.scalar == 0):
                dla.add(res)
        old_length = new_length
        new_length = len(dla)

    return dla

It currently works well for my use cases, though it is not very performant. Maybe this still helps for a fix.

@CatalinaAlbornoz
Copy link
Contributor

Hi @qubitzer ,

Thank you for opening this issue!
My colleague David figured out the issue. It looks like there might be a physicists convention vs mathematicians convention confusion here.

In the implementation for lie_closure it's assumed that all coefficients are purely imaginary. So to avoid having to type 1j * always, we made this factor of 1j* implicit, and assume that you mean 1j*X(0) when you say X(0).

So the real issue here is that this isn't mentioned clearly in the documentation 🙃 , sorry for that. We'll fix the documentation to make this clear. Thank you for making us aware of this and helping us improve!

Let us know if you have any other questions or find any other issues.

@qubitzer
Copy link
Author

Hi @CatalinaAlbornoz ,

I understand. Thank you for the explanation.

Enhancing the docu is great, but still I would suggest to also adjust the functionality. The mentioned example

def CX(i,j) : return 0.5 * (1 + Z(i) + X(j) - Z(i) @ X(j))
generators = [CX(0,1) @ Z(1)] 
qml.lie_closure(generators)

does not contain explicit complex coefficients, but it does not work. As I now realized with your help it does work if a global factor of 1j is introduced:

def CX(i,j) : return 0.5 * (1 + Z(i) + X(j) - Z(i) @ X(j))
generators = [1j * CX(0,1) @ Z(1)] 
qml.lie_closure(generators)

Wouldn't it be simpler if lie_closure simply ignored global phases?

@CatalinaAlbornoz
Copy link
Contributor

Hi @qubitzer,

You can find the PR for the docs enhancement here.

I believe the factor of 1j needs to be maintained, for consistency with the conventions we currently have.

@qubitzer
Copy link
Author

Thank you for the clarification :-)

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

No branches or pull requests

2 participants