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

Question about SymbolicHamiltonian.circuit #1357

Closed
marekgluza opened this issue Jun 11, 2024 · 2 comments · Fixed by #1371
Closed

Question about SymbolicHamiltonian.circuit #1357

marekgluza opened this issue Jun 11, 2024 · 2 comments · Fixed by #1371
Labels
bug Something isn't working

Comments

@marekgluza
Copy link
Contributor

marekgluza commented Jun 11, 2024

I assume it's a bug because it has cost me 2x a couple of days of work to trouble shoot the following issue appearing deep in the dbi module code:

import numpy as np
from copy import deepcopy

from qibo import symbols, hamiltonians

d = hamiltonians.SymbolicHamiltonian( symbols.Z(0))
 
a = d.circuit(0.1)
b = d.circuit(0.1)
print(np.linalg.norm( d.circuit(0.2).unitary() - (a+b).unitary()))

a = deepcopy(d).circuit(0.1)
b = deepcopy(d).circuit(0.1)
print(np.linalg.norm( d.circuit(0.2).unitary() - (a+b).unitary()))

Output:

0.28237154359997657
0.0

TLDR: If you assign the return value of SymbolicHamiltonian.circuit and then want to use the Circuit instance e.g. for further addition then noise comes out. Correct result can be obtained by running on deepcopies.

@marekgluza marekgluza added the bug Something isn't working label Jun 11, 2024
@alecandido
Copy link
Member

alecandido commented Jun 19, 2024

I agree there is a bug, because there is a missing copy that links the dt stored in a and b to the one in the original Hamiltonian.

In particular, the following also works:

d = hamiltonians.SymbolicHamiltonian(symbols.Z(0))
 
a = d.circuit(0.1)
b = d.circuit(0.1)
c = (a+b).unitary()
print(np.linalg.norm(d.circuit(0.2).unitary() - c))

even without the deepcopy.

I didn't dig into the code, but it's pretty clear that, for as long as a and b are circuits, they are a sort of view of the original Hamiltonian. At least, they store a reference to it, through which the dt is later modified when d.circuit(0.2) is invoked.
However, computing the .unitary() turns it into a regular NumPy array, independent of any Qibo object.

The bug should be definitely investigated and solved. The one above is just a workaround, but should not be used for long.

@alecandido
Copy link
Member

Btw, I'm pretty sure the problem to be related to Sympy: the dt is going to be stored into a gate, containing a Sympy symbol (in general an expression, essentially a DAG).

This symbol is the one derived from the original Hamiltonian, and it is not deepcopied, since all the occurrences of the symbol should agree with each other.
However, there are certainly some cases in which this is not desired, as the current setup.

So, I'm not yet so sure if this is a bug, or a limitation we have to accept for using symbols. @stavros11, we should dedicate some more time to rethink the usage of symbols in general (we don't have to drop them, but we should be careful).

@marekgluza, for the time being, whenever you want to be certain about the result, and you don't want it to be intentionally linked through a symbol, keep copying.

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.

2 participants