evaluate expressions with non-aux-only hyperindices#495
Conversation
There was a problem hiding this comment.
Pull request overview
This PR relaxes the restriction from #462 that hyperindices must be aux-only, enabling evaluation of expressions with cluster-specific reduced density matrices (RDMs) and allowing more flexible bra-ket contractions. The changes introduce a new StrictBraKetSymmetry context setting and convert compile-time assertion checks to runtime checks throughout the test suite.
Changes:
- Added
StrictBraKetSymmetryenum and context support to control whether bra/ket slots enforce strict vector space semantics - Refactored evaluation expression handling to work with all indices (bra, ket, aux) instead of just aux indices for hyperindices
- Converted compile-time preprocessor assertion checks to runtime checks using new
assert_behavior()function - Added comprehensive tests for evaluating expressions with non-covariant indices and multiple bra/ket hyperindices
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SeQuant/core/attr.hpp | Adds StrictBraKetSymmetry enum with Yes/No values |
| SeQuant/core/context.hpp | Adds StrictBraKetSymmetry to Context class with getter/setter |
| SeQuant/core/context.cpp | Implements StrictBraKetSymmetry support in Context |
| SeQuant/core/utility/macros.hpp | Declares assert_enabled() and assert_behavior() functions |
| SeQuant/core/utility/macros.cpp | Implements runtime assert behavior query functions |
| SeQuant/core/tensor_network/v3.hpp | Removes strict bra-to-bra/ket-to-ket connection prohibition from Edge |
| SeQuant/core/tensor_network/v3.cpp | Adds conditional strict bra-ket checks during graph creation with new include |
| SeQuant/core/eval/eval_expr.cpp | Changes from aux_indices to all_indices, adds logic to route hyperindices to correct slots |
| SeQuant/core/io/serialization/serialization.hpp | Uses explicit std::nullopt instead of {} for optional initialization |
| tests/unit/test_utilities.cpp | Converts compile-time assertions to runtime checks |
| tests/unit/test_tensor_network.cpp | Updates covariance tests to use runtime checks and new create_graph() call |
| tests/unit/test_tensor.cpp | Converts compile-time assertions to runtime checks |
| tests/unit/test_mbpt.cpp | Converts compile-time assertions to runtime checks |
| tests/unit/test_macros.cpp | Converts compile-time assertions to runtime checks |
| tests/unit/test_index.cpp | Converts compile-time assertions to runtime checks |
| tests/unit/test_eval_ta.cpp | Adds new tests for non-covariant indices with StrictBraKetSymmetry::No |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
these happen - only if not disabled in default context, - only if SEQUANT_ASSERT_ENABLED, and - in create_graph where we can ignore named indices
… (this is needed, for example, to evaluate cluster-specific RDMs)
evaleev
force-pushed
the
evaleev/fix/eval-nonaux-hyperindex
branch
from
f5d39c5 to
35b0eac
Compare
…naux-hyperindex # Conflicts: # SeQuant/core/tensor_network/v3.cpp
evaleev
force-pushed
the
evaleev/fix/eval-nonaux-hyperindex
branch
from
2f9ab4d to
458801c
Compare
| // strict bra-ket sanity checks | ||
| if constexpr (assert_enabled()) { | ||
| if (current_edge.vertex_count() > 2) { | ||
| [[maybe_unused]] std::size_t nbra = 0; | ||
| [[maybe_unused]] std::size_t nket = 0; | ||
| [[maybe_unused]] std::size_t naux = 0; | ||
| for (std::size_t i = 0; i < current_edge.vertex_count(); ++i) { | ||
| const Vertex &vertex = current_edge.vertex(i); | ||
| switch (vertex.getOrigin()) { | ||
| case Origin::Bra: | ||
| ++nbra; | ||
| break; | ||
| case Origin::Ket: | ||
| ++nket; | ||
| break; | ||
| case Origin::Aux: | ||
| ++naux; | ||
| break; | ||
| case Origin::Proto: | ||
| SEQUANT_UNREACHABLE; | ||
| if (get_default_context().strict_braket_symmetry() == | ||
| StrictBraKetSymmetry::Yes) { |
There was a problem hiding this comment.
This shouldn't be guarded by an assertion. If the user has strict braket symmetry enabled, this should always be checked (and it should throw an exception on violation rather than assert). Otherwise, the user is presented with an option that they can't rely on, which seems odd.
There was a problem hiding this comment.
My first reaction is no, in applications where I know my expressions are correct (e.g. in a QC user code) I want to be able to disable pre/post-condition checks globally, by configuring with SEQUANT_ASSERT_BEHAVIOR=IGNORE. For SeQuant to be usable as a component in the end-user code we have to be able to control behavior of all of its contract checks.
Disabling strictness is for situations where a blunt "ignore nonsense math" sign is needed, such as in tests. I can see how the new option may be read as "checks always on", though. I think dox explaining what strict_braket_symmetry means would be sufficient ... ?
There was a problem hiding this comment.
Maybe changing the name to include "assert" would already go towards making it clear what to expect from this? E.g. assert_strict_braket_symmetry
…ndexVec`. - Set of `Index` with `FullLabelCompare` comparator seems to appear in many use cases. - `Index::index_vector` is special because some containers like `boost::small_vector` cannot represent proto-indices as per the documentation on `Index::index_vector`. Then we must refer to the `Index::index_vector` type across the code base consistently using `IndexVec`.
3 participants
Executive Summary
this relaxes the restriction in #462 that hyperindices can
aux-only. This is needed, for example, to evaluate cluster-specific RDMs:This also allows to relax bra-ket strictness imposed by default by TNv3.
Non-aux hyperindices
As of #462 evaluation of tensor networks with hyperindices is supported as long as the hyperindices are in
auxslots. This is sufficient when the hyperindices are not external/named because summing over arbitrary combinations of primal/ket and dual/bra modes does not make sense;auxindices are the proper way to denote modes without vector-space structure (i.e., array-like). But as the cluster-specific RDM demonstrates it does makes sense to have named hyperindices that involvebra/ketslots. The key question is then what should these indices become? In the RDM example the hyperindices (i1,i2) appear in bothbraandketslots. It's clear that these indices should refer to anauxmode of the result: no meaningful primal-dual notion can be defined for these modes. One can imagine cases where it might make sense to designate the resulting modes asbraorket, e.g.:but there are yet many questions about such model (e.g. what should be pairing of the resulting modes). This PR postpones dealing with such cases. It's clear that more thinking is needed, and ultimately support for general tensor symmetries is the part of this issue.
Nonstrict bra-ket sanitization
Bra-ket separation exists to prevent the users from making accidental mistakes in dealing with expressions where primal/dual modes differ nontrivially (nonunit metric, complex fields, etc.). All TN implementations sanitize their input expressions accordingly. Nevertheless some (e.g., agents writing tests) will ignore this structure for pragmatic reasons. This PR adds the ability to turn off the bra-ket strictness in TNv3 (strictness is still on by default).