-
Notifications
You must be signed in to change notification settings - Fork 8
Custom Operator Registry Support #456
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
base: master
Are you sure you want to change the base?
Conversation
…ature/runtime-op-support
All connectivity info is now encoded using plain strings
All connectivity info is now encoded using plain strings
…ation order Constructors now take strings as Operator identifiers
…f perturbation order. Perturbation order does not make any difference on how the operator acts, so it is only for bookkeeping here.
All methods except A and S will check registry make sure the Operator label is registered.
Both hat and bar together looks complicated. Since we will eventually switch to an antisymmetrizer operator, it makes sense to not have the bar.
Ideally, this logic should be general: we should detect the presence of the Unicode character and avoid adding hats. Unfortunately this means introducing another dependency, so let's stick with this for now. Boost.Locale has a way to support normalization, but still need ICU backend.
Some logic needs antisymmetrizer/symmetrizer to be in the front.
…trizer label The graphs didn't change, just the label and the color assigned to the tensor
The indexing changed, but as the comment above says, all forms canonicalizes to the same form.
This lambda always assumed that symmetrizer is the first tensor, which may not be the case always. On the lines of #441.
The stcc and ostcc integration tests were failing with an assertion error in external_indices(). The function requires that symmetrizer/antisymmetrizer tensors be identical across all terms, but this does not happen without setting cardinal_tensor_labels for the TNC to use. Also setting the order ensures aesthetic printing.
6e2e51d to
5ba9ddc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the MBPT operator system from a predefined OpType enum to a runtime OpRegistry, enabling users to define custom operators beyond the predefined set. The implementation introduces a registry-based approach where operator labels map to operator classes (excitation, de-excitation, or general).
Key Changes:
- Introduces
OpRegistryfor runtime operator registration with two predefined registries (minimal and legacy) - Removes
OpTypeenum and replaces all usage with string-based operator labels - Adds support for perturbation orders 0-9 (previously limited to order 1)
- Replaces hard-coded reserved labels (L"A", L"S") with functions returning L"Â", L"Ŝ"
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
SeQuant/core/reserved.hpp |
New file defining reserved operator labels (antisymmetrizer, symmetrizer, kronecker, overlap) |
SeQuant/domain/mbpt/op_registry.hpp/cpp |
New registry class for mapping operator labels to OpClass |
SeQuant/domain/mbpt/context.hpp/cpp |
Extended to hold OpRegistry with thread-safe manipulation |
SeQuant/domain/mbpt/op.hpp/cpp/ipp |
Major refactoring: OpMaker/Operator now use strings; added perturbation order support |
SeQuant/domain/mbpt/vac_av.hpp/cpp |
OpConnections changed from OpType to std::wstring |
| Test files | Updated to use new string-based API and reserved label functions |
| Documentation | Added operator registry documentation and examples |
| Input files (nevpt2, ccsd) | Updated antisymmetrizer label from "A" to "Â" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 57 out of 57 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ill append to it - add default cardinal tensor labels: antisymm/symm reserved labels - update set_cardinal_tensor_labels() to append user-provided labels to defaults - add reset_cardinal_tensor_labels() to restore defaults - add clear_all_cardinal_tensor_labels() to remove all labels including defaults - add logic for duplicate label checking, only enabled if SEQUANT_ASSERT_ENABLED is true
…_tensor_labels()` Since antisymm_label() and symm_label() are now default cardinal labels (see f4e2d75), they will be automatically prepended when calling TensorCanonicalizer::set_cardinal_tensor_labels()
…ithout setting `cardinal_tensor_labels()` for TNC. See also f4e2d75
b4f63b8 to
fea1717
Compare
5d0abbc to
cb02281
Compare
User OpRegistry object, not pointer
Custom Operator Registry Support
This PR refactors Operator logic from depending on a predefined
OpTypeenum to a runtimeOpRegistry, enabling users to define custom operators beyond the predefined set.Major Changes
OpRegistry: a registry for operator labels and their classificationsmbpt::Contextnow holds anOpRegistryalong with the CSV setting.OpTypeenum is removed, all related logic is gone:OpMakerconstructors now use strings.OpConnectionsnow uses strings to specify operator pairs. For example:{{L"f", L"A"}, {L"g", L"A"}, ...}mbpt::Contextmanipulations are thread-safe, and can be toggled bySEQUANT_CONTEXT_MANIPULATION_THREADSAFEoption; follows the same pattern ascore::Context.Example Usage
Additional Changes
ÂandŜ, respectively. As a result, several tests and examples were updated. Most hardcoded strings were replaced with calls to the reserved label methods; however, some reference outputs and expressions are still constructed from plain strings.tests/integrationwere not usingmbpt::cardinal_tensor_labels()for expression canonicalization. After the label change, symmetrizer and antisymmetrizer tensors were moved to the end during canonicalization. Since the spin-tracing logic assumes all symmetrizer tensors share the same external indices, they must appear first. This was fixed by canonicalizing withmbpt::cardinal_tensor_labels(), which places symmetrizer tensors first and ensures consistent indexing.trace_productspin-tracing logic assumed that the symmetrizer tensor always appears first; this has been generalized. cc: @ABesharateval_{ta,btas}cases now usembpt::cardinal_tensor_labels()with TNC.