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

DBI class 3rd order GC and reduced 3rd math error #67

Open
Sam-XiaoyueLi opened this issue Jul 15, 2024 · 8 comments
Open

DBI class 3rd order GC and reduced 3rd math error #67

Sam-XiaoyueLi opened this issue Jul 15, 2024 · 8 comments
Assignees
Labels

Comments

@Sam-XiaoyueLi
Copy link
Contributor

Sam-XiaoyueLi commented Jul 15, 2024

Pulled from main branch. In double_bracket.py, within function eval_dbr_unitary, the operator for when the mode is DoubleBracketGeneratorType.group_commutator_3 and DoubleBracketGeneratorType.group_commutator_3_reduced has errors:

  1. step not using sqrt_step
  2. signs are wrong

Before modification, plotting the off-diagonal norm w.r.t DBR duration s gives
image

The suggested changes are

elif mode is DoubleBracketGeneratorType.group_commutator_3:
            if d is None:
                d = self.diagonal_h_matrix
            sqrt_step = np.sqrt(step)
            operator = (
                self.h.exp(sqrt_step * (np.sqrt(5) - 1) / 2)
                @ self.backend.calculate_matrix_exp(-sqrt_step * (np.sqrt(5) - 1) / 2, d)
                @ self.h.exp(-sqrt_step)
                @ self.backend.calculate_matrix_exp(sqrt_step * (np.sqrt(5) + 1) / 2, d)
                @ self.h.exp(sqrt_step * (3 - np.sqrt(5)) / 2)
                @ self.backend.calculate_matrix_exp(-sqrt_step, d)
            )
        elif mode is DoubleBracketGeneratorType.group_commutator_3_reduced:
            if d is None:
                d = self.diagonal_h_matrix
            sqrt_step = np.sqrt(step)
            operator = (
                 self.backend.calculate_matrix_exp(-sqrt_step * (np.sqrt(5) - 1) / 2, d)
                @ self.h.exp(-sqrt_step)
                @ self.backend.calculate_matrix_exp(sqrt_step * (np.sqrt(5) + 1) / 2, d)
                @ self.h.exp(sqrt_step * (3 - np.sqrt(5)) / 2)
                @ self.backend.calculate_matrix_exp(-sqrt_step, d)
            )
        return operator

After correcting:
image

@Sam-XiaoyueLi Sam-XiaoyueLi changed the title DBI class 3rd order GC and reduced 3rd DBI class 3rd order GC and reduced 3rd math error Jul 15, 2024
@marekgluza
Copy link
Contributor

@andrea-pasquale
Copy link
Collaborator

I think that it should not be an issue because with @MatteoRobbiati we are using directly the DoubleBracketIteration class in qibo and not the one available here. It could very well contains bugs because I don't know exactly when you guys branched out from the qibo version, and this version is not maintained.

@andrea-pasquale
Copy link
Collaborator

@Sam-XiaoyueLi can you please check if the version available in qibo is correct?

@Sam-XiaoyueLi
Copy link
Contributor Author

@Sam-XiaoyueLi can you please check if the version available in qibo is correct?

I'm now running DBI on the master branch of qibo, if my setup is correct, I think there might be an issue.

  1. for group commutators, the step we are using should be np.sqrt(step), but if you set it in the input then should be OK.
  2. signs are not correct.

Without modifying anything:
image
Set input step=np.sqrt(step):
image
Changing the sign and step:
image

My test code:

import numpy as np
from qibo import hamiltonians, set_backend
from qibo.models.dbi.double_bracket import (
    DoubleBracketGeneratorType,
    DoubleBracketIteration,
)
from qibo.models.dbi.utils import *
from qibo.models.dbi.utils_scheduling import *
from qibo.models.dbi.utils_dbr_strategies import *

from qibo import hamiltonians

import matplotlib.pyplot as plt

def initialize_dbi(nqubits, model, param):
    if model == "XXZ":
        hamiltonian = hamiltonians.XXZ(nqubits=nqubits, delta=param)
    if model == "TFIM":
        hamiltonian = hamiltonians.TFIM(nqubits=nqubits, h=param)
    dbi = DoubleBracketIteration(hamiltonian=hamiltonian)
    return dbi

def plot_sigma_time(dbi, d, mode, s_space):
    dbi.mode = mode
    return [dbi.loss(step=s, d=d) for s in s_space]

dbi = initialize_dbi(5, "TFIM", 3)
dbi.cost = DoubleBracketCostFunction.off_diagonal_norm
s_space = np.linspace(1e-4,0.1,100)
modes = [DoubleBracketGeneratorType.single_commutator,
         DoubleBracketGeneratorType.group_commutator,
         DoubleBracketGeneratorType.group_commutator_third_order]
plots = []
for mode in modes:
    plots.append(plot_sigma_time(dbi, dbi.diagonal_h_matrix, mode, s_space))

mode_names = [r'$e^{-s\hat W_k}$',
              r'$\hat V^{\text{GC}}_k$',
              r'$\hat V^{\text{3rd order GC}}_k$']
s_min = []
for i,mode in enumerate(modes):
    plt.plot(s_space, plots[i], label=mode_names[i])
    s_min.append(s_space[np.argmin(plots[i])])
# plt.xticks(s_min)
plt.ylabel(r'Off-diagonal norm $||\sigma(\hat H_{k+1})||$')
plt.xlabel(r'DBR duration $s$')
plt.legend()
plt.savefig('group_commutator.pdf')

@andrea-pasquale
Copy link
Collaborator

Thanks @Sam-XiaoyueLi for opening the issue.
The current version of DBI in qibo doesn't perform the sqrt of the step for group commutators, this is something that we could add back, it is just a matter of conventions.
At least in my version of Qibo I don't see any errors with the sign. By applying just the square root to the step I'm able to reproduce your last plot. Make sure to do git pull before checking which version you are using.
For applying the square root to the step feel free to open a PR in Qibo.

@Sam-XiaoyueLi
Copy link
Contributor Author

Thanks @Sam-XiaoyueLi for opening the issue. The current version of DBI in qibo doesn't perform the sqrt of the step for group commutators, this is something that we could add back, it is just a matter of conventions. At least in my version of Qibo I don't see any errors with the sign. By applying just the square root to the step I'm able to reproduce your last plot. Make sure to do git pull before checking which version you are using. For applying the square root to the step feel free to open a PR in Qibo.

Thanks @andrea-pasquale for the reply. I have double checked by git pull and looking at the master branch on github, and I see what the issue is. For group_commutator_third_order, the operator is defined twice. The first definition has the wrong signs (that I used for plotting) the wrong curves and the second the correct ones. https://github.com/qiboteam/qibo/blob/59af33c7533b90ad572543b99f690ce42c07337e/src/qibo/models/dbi/double_bracket.py#L164C3-L182C14

Thanks for helping me check! Maybe deleting the wrong replicate would help clarify the code?

@andrea-pasquale
Copy link
Collaborator

Oh yes, probably while fixing conflicts we have duplicated the definition by accident.
In any case the second definition (the correct one) will overwrite the first one, this is why I don't see any error related to the sign.
Feel free to remove the first wrong definition and add the sqrt for the step.

@marekgluza
Copy link
Contributor

waiting for #92

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants