-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement support for general (auxiliary) tensor indices #177
Conversation
767f80e
to
5dc009d
Compare
@evaleev I believe this is ready for review now. Note that this PR has a cross-relation to #180 in that if either of these PRs is merged, the other needs to be updated. /CC @ak-ustutt |
7854d43
to
7e195c8
Compare
Rebased against current master and thereby resolved the cross-dependence on the hash PR |
Merged with master (no rebase due to too many conflicts with #184 - didn't feel like doing a rebase with those) |
@Krzmbrzl thanks, this is quite a chunk of work!!! I just had a look at this ... do you recall what the issue is with the Wick engine not meshing with the new network graph? At least, what the symptoms were? |
@evaleev At first I ran into various asserts within the Wick impl due to not having resolved an implicit assumption about the graph representation. Those were about e.g. tensor core vertex ID's being usable as indices into the tensor vector of the network. Note that I did find some bugs in my new canonicalization routine after having re-introduced the old graph for the Wick program so perhaps (part of) the issue in the end was a buggy graph representation after all. My biggest problem in all of this was that the wick implementation and the graph representation are tightly coupled, but only implicitly. That means that the wick implementation access what should probably be an implementation detail of the tensor network implementation (the exact graph representation). |
64d7af3
to
7a024e2
Compare
@evaleev the new SRCC tests for CSVs are failing. One of them fails due to an unmet assertion that detected external indices are unique, but this unique-ness is based on |
This type trait allows to e.g. static_assert that all requirements of the Tensor interface are fulfilled for a given class.
This reverts commit 4c5e8ea.
Instead of hardcoding two expected LaTeX representations of a given expression, we now do an exhaustive search over all permutation of summands to see if one of them yields the desired permutation. This is acceptable as the order of summands has no practical relevance other than for matching a hard-coded order in a test case. The benefit of doing it this way is reduced maintenance cost in exchange for a runtime performance overhead when running the tests. However, the overhead is expected to be small and for test cases it shouldn't matter anyway.
7a024e2
to
5403c14
Compare
27cb596
to
1b591fa
Compare
5d1d12b
to
1b14687
Compare
1b14687
to
61b8347
Compare
183b20c
to
a6b3c5a
Compare
Superseded by #243 |
This PR
auxiliary
) to the tensor interface. These indices are (for now) assumed to never carry spin (they are ignored for e.g. spintracing) and they are assumed to be fully non-symmetric.TensorNetwork
implementation to make the implementation (hopefully) a bit more readable and also to fix some issues in the old implementation (under certain circumstances the resulting output wasn't canonical). Furthermore, the new implementation ensures that canonicalization is a single-pass step. No more need to sandwich a slow canonicalization between two fast ones (though the implementation doing the sum canonicalization was left alone and thus still uses a multi-pass procedure). Finally, the new implementation supports canonicalization of tensors with auxiliary indices.Additional notes:
WickGraph
class that will be used for Wick. At some point we should try wiring up the Wick impl to the new graph representation, but for now I'm afraid I don't have the time to keep trying.Fixes #161
Fixes #107