qiboteam / qibo

A framework for quantum computing
https://qibo.science
Apache License 2.0
287 stars 58 forks source link

Question about `SymbolicHamiltonian.circuit` #1357

Closed marekgluza closed 3 months ago

marekgluza commented 3 months ago

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.

alecandido commented 3 months ago

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 commented 3 months ago

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.