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

[debug][NDTensors] In-place contraction bug #1152

Merged
merged 7 commits into from Jul 20, 2023

Conversation

kmp5VT
Copy link
Collaborator

@kmp5VT kmp5VT commented Jul 19, 2023

Description

When computing C = \alpha AB + \beta C, with \beta != 0 there was a bug found that sometimes the tensor C would be evaluated incorrectly. This bug was found to occur when the output labels of C were permuted from the output labels of AB. I.e. C(5,2,4,3) = A(-1, 2,3,4) * B(-1,5) + C(5,2,4,3). A(-1,2,3,4) * B(-1,5) => R(5,2,3,4) and was matricized to R(5,234) and added to the matricized C(5,243). The addition R(5,234) + C(5,243) has no runtime errors because the size (234) = (243) however numerical errors occurred. This was fixed by permuting C when \beta != 0
Fixes #(issue)

This bug was discovered in this thread https://itensor.discourse.group/t/inplace-multiplication-of-tensors-with-the-same-shape-but-different-indicies/900/16

@kmp5VT kmp5VT requested a review from mtfishman July 19, 2023 16:07
@mtfishman
Copy link
Member

Thanks for the quick fix!

@mtfishman
Copy link
Member

Could you add a test?

@kmp5VT
Copy link
Collaborator Author

kmp5VT commented Jul 20, 2023

@mtfishman a test has been added to base/test_contract

@codecov-commenter
Copy link

Codecov Report

Merging #1152 (7507112) into main (708220f) will decrease coverage by 31.03%.
The diff coverage is 71.42%.

❗ Current head 7507112 differs from pull request most recent head 6d57dce. Consider uploading reports for the commit 6d57dce to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##             main    #1152       +/-   ##
===========================================
- Coverage   85.29%   54.26%   -31.03%     
===========================================
  Files          87       86        -1     
  Lines        8363     8314       -49     
===========================================
- Hits         7133     4512     -2621     
- Misses       1230     3802     +2572     
Impacted Files Coverage Δ
src/itensor.jl 80.33% <63.63%> (-1.88%) ⬇️
src/tensor_operations/matrix_decomposition.jl 90.00% <100.00%> (-2.86%) ⬇️

... and 35 files with indirect coverage changes

@mtfishman mtfishman merged commit bd10851 into ITensor:main Jul 20, 2023
8 checks passed
@kmp5VT kmp5VT deleted the kmp5/debug/inplace_mult_issue branch April 15, 2024 14:28
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.

None yet

3 participants