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

use diagonal matrix with group commutator #1154

Merged
merged 1 commit into from
Jan 17, 2024
Merged

Conversation

Edoardo-Pedicillo
Copy link
Contributor

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

Copy link
Contributor

@MatteoRobbiati MatteoRobbiati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
The tests errors are due to the dbf_migrate branch, right?

@andrea-pasquale
Copy link
Contributor

Thanks. The tests errors are due to the dbf_migrate branch, right?

Not really, I think that it is better if we fix them in this PR. @Edoardo-Pedicillo feel free to do it whenever you have time otherwise I can do it.

@MatteoRobbiati MatteoRobbiati merged commit 759ae7b into dbf_migrate Jan 17, 2024
10 of 19 checks passed
@Sam-XiaoyueLi
Copy link
Contributor

@MatteoRobbiati Hi Matteo, I think this PR broke test_models_dbi.py by removing the line raise_error(ValueError, f"Cannot use group_commutator with matrix {d}"). I'm not sure if we should update the test to reflect the changes or should I try to fix it in my branch (or this?). Please advise. Thanks!

/cc @Edoardo-Pedicillo @andrea-pasquale

@MatteoRobbiati
Copy link
Contributor

@Sam-XiaoyueLi you are right, because now we don't need to raise an error, since we have a default matrix d. I guess the solution is to remove the lines:

    with pytest.raises(ValueError):
        dbf(mode=DoubleBracketGeneratorType.single_commutator, step=0.01)

from the tests.

What do you think @Edoardo-Pedicillo, @andrea-pasquale?

@marekgluza
Copy link
Contributor

@Sam-XiaoyueLi you are right, because now we don't need to raise an error, since we have a default matrix d. I guess the solution is to remove the lines:

    with pytest.raises(ValueError):
        dbf(mode=DoubleBracketGeneratorType.single_commutator, step=0.01)

from the tests.

What do you think @Edoardo-Pedicillo, @andrea-pasquale?

@Sam-XiaoyueLi One comment, maybe others can help to understand the dependancies: I believe d is a numpy array, shouldn't it be Hamiltonian? That way other backends eg hardware won't have an issue?

For now DBI is not deployable because we don't unfold so it's a classical simulator model but I'm preparing to do the transpiler into Hamiltonian simulation and then we will have to switch.

Possible solution: add transpiled_double_bracket.py which inherits and works with appropriate evolution oracles which here would derive from Circuit

@andrea-pasquale
Copy link
Contributor

andrea-pasquale commented Jan 24, 2024

@Sam-XiaoyueLi you are right, because now we don't need to raise an error, since we have a default matrix d. I guess the solution is to remove the lines:

    with pytest.raises(ValueError):
        dbf(mode=DoubleBracketGeneratorType.single_commutator, step=0.01)

from the tests.

What do you think @Edoardo-Pedicillo, @andrea-pasquale?

Hi all, actually the correct lines to remove should be

    with pytest.raises(ValueError):
        dbf(mode=DoubleBracketGeneratorType.group_commutator, step=0.01)

given that this PR changes only the group_commutator and not the single_commutator.
I've seen that @Sam-XiaoyueLi already performed some fixes in #1143 we can continue the discussion there.

@andrea-pasquale andrea-pasquale mentioned this pull request Jan 24, 2024
4 tasks
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

Successfully merging this pull request may close these issues.

5 participants