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

Fix bug in SymbolicHamiltonian.__mul__ #1517

Merged
merged 2 commits into from
Nov 11, 2024
Merged

Fix bug in SymbolicHamiltonian.__mul__ #1517

merged 2 commits into from
Nov 11, 2024

Conversation

renatomello
Copy link
Contributor

@renatomello renatomello commented Nov 7, 2024

Checklist:

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

Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.72%. Comparing base (05b57e9) to head (64f6e2e).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1517   +/-   ##
=======================================
  Coverage   99.72%   99.72%           
=======================================
  Files          80       80           
  Lines       11781    11782    +1     
=======================================
+ Hits        11749    11750    +1     
  Misses         32       32           
Flag Coverage Δ
unittests 99.72% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marekgluza
Copy link
Contributor

marekgluza commented Nov 7, 2024

Thanks @renatomello

  • I confirm that the code now runs
  • Can you/I push to this branch a test that will cover this case? I suggest to modify this

def test_symbolic_hamiltonian_operator_add_and_sub(
backend, nqubits, calcterms, calcdense
):
"""Test addition and subtraction between Trotter Hamiltonians."""
local_ham1 = hamiltonians.SymbolicHamiltonian(
symbolic_tfim(nqubits, h=1.0), backend=backend
)
local_ham2 = hamiltonians.SymbolicHamiltonian(
symbolic_tfim(nqubits, h=0.5), backend=backend
)
if calcterms:
_ = local_ham1.terms
_ = local_ham2.terms
if calcdense:
_ = local_ham1.dense
_ = local_ham2.dense
local_ham = local_ham1 + local_ham2
target_ham = hamiltonians.TFIM(nqubits, h=1.0, backend=backend) + hamiltonians.TFIM(
nqubits, h=0.5, backend=backend
)
dense = local_ham.dense
backend.assert_allclose(dense.matrix, target_ham.matrix)
local_ham1 = hamiltonians.SymbolicHamiltonian(
symbolic_tfim(nqubits, h=1.0), backend=backend
)
local_ham2 = hamiltonians.SymbolicHamiltonian(
symbolic_tfim(nqubits, h=0.5), backend=backend
)
if calcterms:
_ = local_ham1.terms
_ = local_ham2.terms
if calcdense:
_ = local_ham1.dense
_ = local_ham2.dense
local_ham = local_ham1 - local_ham2
target_ham = hamiltonians.TFIM(nqubits, h=1.0, backend=backend) - hamiltonians.TFIM(
nqubits, h=0.5, backend=backend
)
dense = local_ham.dense
backend.assert_allclose(dense.matrix, target_ham.matrix)

by multiplying some fiducial numbers - see 1.5 and 0.6 in the next to last lines

def test_symbolic_hamiltonian_operator_add_and_sub(
    backend, nqubits, calcterms, calcdense
):
    """Test addition and subtraction between Trotter Hamiltonians."""
    local_ham1 = hamiltonians.SymbolicHamiltonian(
        symbolic_tfim(nqubits, h=1.0), backend=backend
    )
    local_ham2 = hamiltonians.SymbolicHamiltonian(
        symbolic_tfim(nqubits, h=0.5), backend=backend
    )
    if calcterms:
        _ = local_ham1.terms
        _ = local_ham2.terms
    if calcdense:
        _ = local_ham1.dense
        _ = local_ham2.dense
    local_ham = local_ham1 + local_ham2
    target_ham = hamiltonians.TFIM(nqubits, h=1.0, backend=backend) + hamiltonians.TFIM(
        nqubits, h=0.5, backend=backend
    )
    dense = local_ham.dense
    backend.assert_allclose(dense.matrix, target_ham.matrix)

    local_ham1 = hamiltonians.SymbolicHamiltonian(
        symbolic_tfim(nqubits, h=1.0), backend=backend
    )
    local_ham2 = hamiltonians.SymbolicHamiltonian(
        symbolic_tfim(nqubits, h=0.5), backend=backend
    )
    if calcterms:
        _ = local_ham1.terms
        _ = local_ham2.terms
    if calcdense:
        _ = local_ham1.dense
        _ = local_ham2.dense
    local_ham =1.5 * local_ham1 - 0.6 * local_ham2
    target_ham = 1.5 *hamiltonians.TFIM(nqubits, h=1.0, backend=backend) - 0.6 * hamiltonians.TFIM(
        nqubits, h=0.5, backend=backend
    )
    dense = local_ham.dense
    backend.assert_allclose(dense.matrix, target_ham.matrix)

@renatomello
Copy link
Contributor Author

@marekgluza I introduced a small example that covers this situation.

nqubits, h=0.5, backend=backend
)
dense = local_ham.dense
backend.assert_allclose(dense.matrix, target_ham.matrix)

# Test multiplication and sum
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Worth also keeping in mind for tutorials even :)

@renatomello renatomello added this pull request to the merge queue Nov 11, 2024
Merged via the queue into master with commit a8e7edd Nov 11, 2024
27 checks passed
@renatomello renatomello deleted the symbolic branch November 11, 2024 12:50
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
3 participants