-
Notifications
You must be signed in to change notification settings - Fork 5
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
Potentially wrong symmetry deduction for intermediates in eval nodes #178
Comments
Indeed there might be some issues here: this code was written a while back and we did not actually exploit symmetry of the intermediates during evaluation, so incorrect deductions might have gone unnoticed. The symmetry deduction rules are taken from this paper, Table 1. I invite you to take a look at it since you are already in the loop of symmetry deduction. Regarding the |
Thanks for the link to the paper - haven't seen that one before. I cross-referenced the rules given in table 1 of that paper with the implementation that I see in SeQuant/SeQuant/core/eval_expr.cpp Lines 310 to 321 in 42558e9
This seems to be searching for an outer product of a tensor with itself - with exactly the same indices (contrary to what I wrote in my initial post). I take it that this is essentially a special case of rule 2.5 of the paper. If my interpretation of what the hash-check does is correct, then the deduced antisymmetry should be correct. However, that would also mean that every index appears twice on the intermediate tensor, which seems odd (not more odd than having an outer product of a tensor with itself though). Should the hash-check allow for different indices on the same tensor, then this would be exactly rule 2.5, in which case the deduced antisymmetry would be wrong. We would only create column-symmetry (particle symmetry) in the intermediate that way, but (in the general case) only between indices originating from different tensors. The permutational symmetry of indices coming from the same tensor would entirely depend on the symmetry properties of that tensor. SeQuant/SeQuant/core/eval_expr.cpp Lines 333 to 339 in 42558e9
This seems to implement rule 2.2 and generalize that to two fully antisymmetric tensors, which seems correct 🤔 If someone could double-check my interpretation of the outer-produce special case handling, I think the deduction is probably fine after all 👀 |
The outer product of a tensor with itself is symmetric as given in the referenced table (rule 2.5). Could it be antisymmetric for when the tensor itself is antisymmetric (in terms of bra indices and ket indices considered independently? The answer is no -- as far as I could generalize. So the bug fix should be to return the tensor symmetry as |
For tensor contractions, the intermediate tensor that is constructed gets deduced symmetries via
SeQuant/SeQuant/core/eval_expr.cpp
Lines 379 to 382 in 42558e9
I believe that this can lead to incorrect results.
Assumption that outer products produce antisymmetric tensors
Inside
tensor_symmetry_prod
we findSeQuant/SeQuant/core/eval_expr.cpp
Lines 310 to 321 in 42558e9
which checks whether the product of the two given tensors is an outer product (no common indices between the tensors -> no contractions involved). If so, the function returns that the symmetry of the product is antisymmetric.
To my understanding this is wrong for two reasons:
Symmetry of fully contracted expressions
In the same function we also find
SeQuant/SeQuant/core/eval_expr.cpp
Lines 325 to 339 in 42558e9
that assigns a symmetry to the result of a full contraction. While technically never wrong (regardless of what symmetry is chosen) as there aren't any indices on that intermediate tensor, I am wondering why the effort is taken at all? Shouldn't a full contraction simply yield to a
Variable
instead of aTensor
as a tensor without indices is effectively a variable, no?P.S.: That's already done via
SeQuant/SeQuant/core/eval_expr.cpp
Lines 374 to 376 in 42558e9
so maybe the code snippet linked above is superfluous?
Braket symmetry
SeQuant/SeQuant/core/eval_expr.cpp
Line 381 in 42558e9
This is rather surprising to me. Isn't bra-ket symmetry rather exceptional and essentially only appears for the two-electron integrals? Under what circumstances can we declare the result of any arbitrary tensor product as bra-ket symmetric? 🤔
The text was updated successfully, but these errors were encountered: