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

support for non-vector-space (aux) modes in tensorials #243

Open
wants to merge 75 commits into
base: master
Choose a base branch
from

Conversation

evaleev
Copy link
Member

@evaleev evaleev commented Nov 18, 2024

revamp of #177

largely cosmetic touchup (auxiliary -> aux) + merge with master + support for both old TensorNetwork and new TensorNetworkV2 implementations (old TN was made aux-aware and is the temporary default until TNV2 is tested as its replacement).

N.B. since in master bra and ket are reserved for strong typing, the tensor concept drops bra(t), ket(t) and ``aux(t)` requirements.

Krzmbrzl added 30 commits March 26, 2024 11:44
This type trait allows to e.g. static_assert that all requirements of
the Tensor interface are fulfilled for a given class.
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.
@evaleev evaleev requested a review from Krzmbrzl November 19, 2024 14:02
Comment on lines +8 to +9
#include "abstract_tensor.hpp"
#include "expr.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include "abstract_tensor.hpp"
#include "expr.hpp"
#include <SeQuant/core/abstract_tensor.hpp>
#include <SeQuant/core/expr.hpp>

.def_property_readonly("braket", [](const Tensor &t) {
auto braket = t.braket();
return std::vector<Index>(braket.begin(), braket.end());
.def_property_readonly("auxikiary", &Tensor::auxiliary)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.def_property_readonly("auxikiary", &Tensor::auxiliary)
.def_property_readonly("auxiliary", &Tensor::auxiliary)

@Krzmbrzl
Copy link
Collaborator

P.S. I started out by deprecating API elements that included word auxiliary but then just dropped them to make maintenance easier. Hopefully this is not a dealbreaker for your existing code.

Sure, that shouldn't cause issues. I'll just keep replacing auxiliary with aux until the compiler no longer yells at me :D

@Krzmbrzl
Copy link
Collaborator

@bimalgaudel imo we shouldn't be hard-coding tensor names. Instead, I would make the CSV/DF functions take the respective tensor names as arguments (that might default to what you have currently hard-coded). This should allow for more flexibility in the future.

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.

4 participants