Symbolic conjugation redesign: canonical braket-conjugate fold, per-tensor conj report, core Re/Im algebra - #602
Symbolic conjugation redesign: canonical braket-conjugate fold, per-tensor conj report, core Re/Im algebra#602kshitij-05 wants to merge 16 commits into
Conversation
The loop started at named_indices.size(), assuming the leading edges are the named ones. A named index that is not an edge (e.g. a pure proto index) shifted that cutoff onto an anonymous edge; the skipped edge's ordinal was then handed to another edge of the same space, yielding a non-injective rewrite that duplicated a slot index. Latent until the Conjugate braket fold reordered the edge sort.
Mirror Variable and Power: Tensor gains a conjugated_ marker -- in the
hash (contributing only when set, so unconjugated tensors hash
identically to before), in static_equal and static_less_than (T orders
before conj(T)), rendered as ^* on the label in to_latex and in the v1
serializer (label^*{...}, matching the Variable spelling; deserializer
grammar extension deferred -- conjugation currently arises only from
canonicalization at runtime, never from parsed input).
conjugate() toggles the marker and touches no slots; adjoint() is
deliberately unchanged (for BraKetSymmetry::Conjugate the swap IS the
adjoint, the conj being carried by the symmetry relation, so the marker
commutes through it -- certified by the new test).
This is the representation half of making the canonicalizer's
conjugation byproduct symbolic: instead of a network-level bool
(SlotCanonicalizationMetadata::conj / EvalExpr::canon_conj_), the
orientation fold will toggle conjugated_ on the tensor itself, giving
per-tensor granularity by construction.
…round-trip is now lossless
…ity, and the conjugation marker
…ot report, eval leaf identity
…tions, is_hermitian_network
…tarred spellings via EvalOp::Adjoint EvalExpr(Tensor) no longer folds BraKetSymmetry::Conjugate leaves onto one bra/ket orientation (flat: TensorBlockCanonicalizer with the fold disabled; ToT: new CanonicalizeSlotsOptions/CreateGraphOptions::fold_conjugate_braket knob giving orientation-sensitive bundle colors and no conj byproduct): leaves keep their as-written spelling, so leaf yielders and evaluators need no conjugation awareness. Folding fresh leaves onto one orientation-shared cache slot is the lazy-conj eval follow-up; the conjugation marker still colors the graph, so C*C vs C*·C stay distinct (the cache-aliasing fix). An already-starred spelling is served through an explicit EvalOp::Adjoint node over its unmarked value-orientation operand, mirroring the U+207A adjoint-label channel. Fixes the eval_with_tiledarray / ta_tot_adjoint_end_to_end CI failures; catalogue tests updated to this contract.
Krzmbrzl
left a comment
Krzmbrzl
left a comment
There was a problem hiding this comment.
Imo, all non-template function implementations should go into the corresponding cpp file instead of in the header. Otherwise, we end up with headers coupled so strongly to each other that we start having to play tricks to use the productively.
| // a Conjugate half-tensor (empty bra or ket bundle) has no vertex for the | ||
| // empty bundle, so its recorded canonical bundle position is a | ||
| // value-initialized sentinel -- the graph cannot decide its orientation; | ||
| // the content-based refold after the lexicographic relabel decides it | ||
| if (braket_symmetry(tensor) == BraKetSymmetry::Conjugate && | ||
| (bra_rank(tensor) == 0 || ket_rank(tensor) == 0)) | ||
| continue; |
There was a problem hiding this comment.
Why is this specific to conjugate braket symmetry? In other words, why does this not cause any issues for BraKetSymmetry::Symm?
There was a problem hiding this comment.
I thought it should be obvious that in BraKetSymmetry::Symm reorienting is not value-changing, but in Conjugate it is. I have updated the comments to explicitly state why this is...
There was a problem hiding this comment.
I don't understand why we can't reorient T{a1;} to T{;a1}* though, which seems to be what this code prevents 👀
| if (canonical_bra_ket_bundle_order[i][0] > | ||
| canonical_bra_ket_bundle_order[i][1]) { | ||
| canonical_bra_ket_bundle_order[i][1] && | ||
| !ranges::equal(tensor._bra(), tensor._ket())) { |
There was a problem hiding this comment.
What does this extra check guard against? Imo, we shouldn't try to be too clever at this point and just transfer all information from the graph canonicalization to the expression. Anything else always bears risk of breaking canonicalization due to introducing input-dependence of the "canonical" form.
There was a problem hiding this comment.
This is a skip exactly to avoid input dependence. Can you explain more precisely what you mean by "just transfer all information from the graph canonicalization to the expression"?
How do you suggest we do tie-breaking over indistinguishable vertices, e.g., diagonal tensors with identical bra and ket colors when BraKetSymmetry::Conjugate? @evaleev
There was a problem hiding this comment.
Can you explain more precisely what you mean by "just transfer all information from the graph canonicalization to the expression"?
What I mean by that is that if the graph canonicalization indicates that we should swap bra and ket indices (which is what canonical_bra_ket_bundle_order[i][0] > canonical_bra_ket_bundle_order[i][1] checks for), we should do it without adding logic that only does it only sometimes. Aka.: Only take the information from the graph and don't add extra conditions.
How do you suggest we do tie-breaking over indistinguishable vertices, e.g., diagonal tensors with identical bra and ket colors when BraKetSymmetry::Conjugate?
This comes down to whether we write t{a1;a1} or t{a1;a1}*, right? Ideally, the graph would be constructed in a way that reflects this symmetry, then the graph canonicalization includes tie-breaking. Otherwise, we could check for this during the second-pass of canonicalization after graph-canonicalization and deterministically choose one of the two. This can even happen after index relabeling as the bra==ket property will be retained through that.
| // Re-apply the per-tensor braket orientation fold now that indices carry | ||
| // their FINAL labels: the fold's full-space-tie decision is | ||
| // label-sensitive, and a decision taken on pre-relabel labels need not | ||
| // be a fixed point of the relabeled expression. The fold is convergent | ||
| // (it decides on the VALUE orientation), so this pass makes the whole | ||
| // canonicalization idempotent. | ||
| for (auto &tensor_ptr : tensors_) { | ||
| DefaultTensorCanonicalizer::canonicalize_braket(*tensor_ptr); | ||
| } |
There was a problem hiding this comment.
Why wasn't this needed before (i.e. for BraKetSymmetry::Symm)?
There was a problem hiding this comment.
As stated in the previous comment reply
| // Symm, keeping string fixtures coherent with ctor-built tensors. | ||
| const sequant::io::serialization::DeserializationOptions opts{ | ||
| .def_perm_symm = sequant::Symmetry::Nonsymm, | ||
| .def_braket_symm = sequant::Hermiticity::Hermitian}; |
There was a problem hiding this comment.
I don't think we should apply hermitian symmetry by default. Afaik the only tensors for which this (almost) always applies are the integrals. The default braket symmetry will become Nonsymm once the default symmetry PR is merged.
There was a problem hiding this comment.
Why is it needed before that? 👀
There was a problem hiding this comment.
Why did these have to be changed?
There was a problem hiding this comment.
The tensor's symmetries were not explicitly mentioned. But configuring with a complex field now changes the values of the TN due to BraKetSymmetry::Conjugate. I believe this would be addressed in #596
There was a problem hiding this comment.
Ah, this is about the default symmetries used by deserialization, right?
| /// @return whether this tensor is complex-conjugated elementwise (no slot | ||
| /// reordering; contrast adjoint(), which swaps bra and ket) | ||
| bool conjugated() const { return conjugated_; } | ||
|
|
||
| /// @brief complex-conjugates this tensor elementwise: toggles conjugated(); | ||
| /// the slots are untouched. For a BraKetSymmetry::Conjugate tensor the | ||
| /// value identity T{q;p} = conj(T{p;q}) means a bra<->ket swap combined | ||
| /// with conjugate() preserves the represented value -- which is how the | ||
| /// canonicalizer folds the two orientations onto one spelling. | ||
| void conjugate() { | ||
| conjugated_ = !conjugated_; | ||
| reset_hash_value(); | ||
| } |
There was a problem hiding this comment.
I feel like adjoint needs to be interwoven with the conjugated flag.
There was a problem hiding this comment.
I have adjusted the docs. Although pure conj (without permutation) needs a separate flag, as it's not the same as adjoint
There was a problem hiding this comment.
By pure I mean elementwise conj
There was a problem hiding this comment.
Yes, I agree that pure conjugation needs to be tracked. What I meant to say was that when calling adjoint that flag should be toggled as well (at least for a complex-valued tensor). And I recall some place in the code where adjoint had to be followed by conjugate to achieve the desired effect (where I think just calling adjoint should be sufficient as it implies complex conjugation)
…s + sound fixes Docs (the reviewer's why-only-for-Conjugate questions answered in place): for Symm braket symmetry both orientations denote the SAME value, so orientation decisions are label-independent and any spelling is harmless; for Conjugate the orientations denote CONJUGATE values, so a swap must toggle the elementwise-conjugation marker, sentinel/automorphic graph verdicts must not be acted on, and the full-space-tie label tie-break makes the post-relabel refold necessary. Also: vertex-painter comment explains why the marker perturbs only marked tensors' colors (an unconditional combine would reshuffle every marker-free network's canonical form); conj/swap/ adjoint algebra table moved into Tensor's class docs and adjoint() docs spell out how it composes with conjugated(); catch2 braket fallback documented as mirroring the ctor default; cost_analysis README explains the explicit :N-N-S pins. Code: - drop SlotCanonicalizationMetadata::conj (no consumers since the eval boundary went fold-free); tests assert conjugated_tensors instead - canonicalize_braket: single three-way space comparison - mbpt::swap_bra_ket: in-place _swap_bra_ket on a copy instead of a with_slots rebuild - complex.hpp: non-template implementations moved to new complex.cpp - canonicalize_slots 3-arg forwarder un-deprecated (kept, documented)
PR #596 makes the programmatic Tensor ctor defaults fixed conservative (Tensor::Defaults, NonHermitian) and moves the deserializer defaults into the Context; braket symmetry is derived from hermiticity and base field. Make this branch independent of the ambient deserializer defaults so the two PRs compose in either order: - ta_tot_adjoint_end_to_end: pin the ToT leaves' braket symmetry (:N-C-S) explicitly -- the test's premise is a Conjugate (Hermitian) leaf, not whatever the ambient default resolves to (identical behavior today) - catch2 to_expression: concrete TODO(PR #596) on the Hermitian fallback (it mirrors the ctor default; flips or is dropped in lockstep) - cost_analysis README: default-agnostic wording for the explicit :N-N-S pins (they state physical facts, independent of ambient defaults) The remaining eval_node/tapp test sites added by this branch already pin {.def_braket_symm = Hermiticity::NonHermitian} explicitly, matching #596's future default.
There was a problem hiding this comment.
🟡 Changes recommended
Several core paths currently assume a conjugated tensor must be braket-Conjugate (and/or use Adjoint semantics), which can assert or mis-handle legitimate ^* usage and should be gated or generalized.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR redesigns SeQuant’s symbolic conjugation and braket-orientation canonicalization by (1) making conjugation a first-class tensor identity marker, (2) folding Conjugate bra/ket orientations into a canonical spelling (with per-tensor reporting), and (3) adding core symbolic Re/Im algebra plus exact folding of conjugate summand pairs.
Changes:
- Canonical braket-conjugate folding (graph- and content-based), with per-tensor
conjugated_tensorsreporting and options-basedcanonicalize_slots. - Introduces first-class tensor conjugation marker
^*(parsing + serialization), propagation through rebuild/transforms, and graph coloring support. - Adds core
RealPart/ImagPartnodes andfold_conjugate_pairs, plus targeted test updates and a new conjugation test catalogue.
File summaries
| File | Description |
|---|---|
| utilities/cost_analysis/README.md | Documents explicit symmetry pinning in equation files to avoid default-Hermitian fold surprises. |
| utilities/cost_analysis/examples/df_r1.inp | Pins tensor symmetries in example input to keep networks physically non-Hermitian. |
| utilities/cost_analysis/examples/ccsd_r2.inp | Pins tensor symmetries in example input to keep networks physically non-Hermitian. |
| tests/unit/test_wick.cpp | Updates expected canonical spellings to swapped+starred forms under new folding. |
| tests/unit/test_tensor.cpp | Adds unit tests for tensor conjugation marker behavior (hash/order/serialize/adjoint). |
| tests/unit/test_tensor_network.cpp | Updates network tests for V3-only support and conjugate-fold metadata expectations. |
| tests/unit/test_spin.cpp | Introduces a real-orbital scoped context helper and updates spintrace expectations. |
| tests/unit/test_parse.cpp | Adds parse/roundtrip coverage for t^*{...} tensor syntax. |
| tests/unit/test_mbpt.cpp | Updates LaTeX/structure expectations for swapped+starred Hermitian spellings. |
| tests/unit/test_mbpt_cc.cpp | Makes ext-index assertions robust to fold-induced bra/ket placement changes. |
| tests/unit/test_extract_subtrees.cpp | Pins deserializer defaults to NonHermitian to keep tree-shape tests stable. |
| tests/unit/test_export.cpp | Pins export test defaults to NonHermitian to prevent fold-driven reshaping. |
| tests/unit/test_export_python.cpp | Pins tensor constructors/deserialization to NonHermitian for deterministic export fixtures. |
| tests/unit/test_eval_tapp.cpp | Pins eval tests’ deserialization defaults to NonHermitian for stable IR shapes. |
| tests/unit/test_eval_ta.cpp | Adds end-to-end adjoint serving coverage for starred ToT leaves. |
| tests/unit/test_eval_node.cpp | Pins eval-node mechanics tests to NonHermitian and adds conjugate-fold mechanics section. |
| tests/unit/test_eval_expr.cpp | Expands eval-boundary conjugation/fold behavior tests and aliasing regressions. |
| tests/unit/test_eval_btas.cpp | Pins BTAS eval tests to NonHermitian defaults for stability. |
| tests/unit/test_conjugation.cpp | New comprehensive test catalogue for conjugation identities, folding, and TN behavior. |
| tests/unit/test_canonicalize.cpp | Updates canonicalization tests for explicit non-Hermitian tensors and new options APIs. |
| tests/unit/CMakeLists.txt | Registers new unit test source test_conjugation.cpp. |
| tests/unit/catch2_sequant.hpp | Aligns deserialize defaults with ctor defaults (Hermitian braket fallback). |
| SeQuant/domain/mbpt/spin.cpp | Preserves conjugation marker through mbpt transforms (swap/remove spin/expand/swap spin). |
| SeQuant/domain/mbpt/rules/thc.cpp | Normalizes folded tensors to value orientation before THC transform rebuilds. |
| SeQuant/domain/mbpt/rules/df.cpp | Normalizes folded tensors to value orientation before DF transform rebuilds. |
| SeQuant/domain/mbpt/rules/csv.cpp | Normalizes folded tensors to value orientation before CSV transform rebuilds. |
| SeQuant/domain/mbpt/biorthogonalization.cpp | Migrates canonicalize_slots call to options struct form. |
| SeQuant/core/tensor_network/vertex_painter.hpp | Adds optional conjugation coloring parameter to vertex painting. |
| SeQuant/core/tensor_network/vertex_painter.cpp | Implements conditional conjugation coloring in vertex hashes. |
| SeQuant/core/tensor_network/v3.hpp | Adds conjugated_tensors reporting and CanonicalizeSlotsOptions. |
| SeQuant/core/tensor_network/v3.cpp | Implements conjugate-braket fold rules, idempotent refold, and metadata reporting. |
| SeQuant/core/tensor_canonicalizer.hpp | Centralizes foldability predicates and adds fold_conjugate controls. |
| SeQuant/core/tensor_canonicalizer.cpp | Implements canonical Conjugate folding with marker-convergent unfolding and tie-breaks. |
| SeQuant/core/options.hpp | Adds SimplifyOptions::FoldConjugatePairs option. |
| SeQuant/core/options.cpp | Implements SimplifyOptions::copy_and_set(FoldConjugatePairs). |
| SeQuant/core/optimize/single_term_detail.hpp | Migrates canonicalize_slots to options struct usage. |
| SeQuant/core/io/serialization/v1/serialize.cpp | Serializes tensor conjugation marker as label^*{...}. |
| SeQuant/core/io/serialization/v1/deserialize.cpp | Parses optional ^* marker after tensor labels. |
| SeQuant/core/io/serialization/v1/ast.hpp | Extends tensor AST node with conjugated flag. |
| SeQuant/core/io/serialization/v1/ast_conversions.hpp | Applies parsed conjugation marker to constructed Tensor; asserts operators cannot be conjugated. |
| SeQuant/core/expressions/tensor.hpp | Adds conjugation marker storage, LaTeX rendering, rebuild API, and value_oriented() helper. |
| SeQuant/core/expressions/expr_algorithms.hpp | Declares conjugate, fold_conjugate_pairs, and is_hermitian_network APIs. |
| SeQuant/core/expressions/expr_algorithms.cpp | Implements conjugation distribution, conjugate-pair folding, simplify hook, and hermitian-network check. |
| SeQuant/core/expressions/complex.hpp | Introduces RealPart/ImagPart nodes and builders with eager rules. |
| SeQuant/core/expressions/complex.cpp | Implements RealPart/ImagPart behavior, hashing, and eager builder logic. |
| SeQuant/core/expressions/abstract_tensor.hpp | Adds virtual hooks for elementwise conjugation marker in AbstractTensor interface. |
| SeQuant/core/eval/eval_expr.cpp | Disables conjugate fold at eval boundary, colors graphs by conjugation, and serves starred spellings via unary IR. |
| CMakeLists.txt | Adds new core complex expression sources to the build. |
Review details
- Files reviewed: 48/48 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…onjugate fold Addresses the PR review round: the marker (sequant::conjugate, deserialized ^*) can land on leaves of any braket symmetry, but value_oriented and binarize assumed it was only the Conjugate-fold byproduct. - value_oriented: total over the marker x braket-symmetry grid -- Conjugate unfolds, Symm clears the value-redundant marker, Nonsymm throws std::logic_error (was an elidable SEQUANT_ASSERT that would otherwise silently swap slots) - binarize(Tensor): marked-Nonsymm leaves refused up front, BEFORE the '⁺' label channel (which would otherwise serve a still-marked bare leaf, dropping the conjugation); marked-Symm leaves served unmarked; the Conjugate channel reuses value_oriented and shares one make_adjoint_node builder with the '⁺' channel - leaf-hash rule centralized in hash_terminal_tensor: the marker enters the hash only where value-distinctive (Nonsymm), so t and t* no longer alias one cache slot; dead strip-for-hash copy in the EvalExpr ctor removed - conjugate(Product): comment justifying Flatten::No (mirrors Product::clone) - docs/includes: fold_conjugate_braket doc no longer names the deleted metadata.conj field; complex.hpp sheds constant.hpp; <compare> moved into the std include block New regression tests: value_oriented_totality ([conjugation]) and the 'starred non-Conjugate leaves' section in test_eval_expr.
Supersedes the symbolic half of #591; evaluation/export changes follow in a separate PR.
Canonicalization
BraKetSymmetry::Conjugatetensors fold onto a single canonical bra/ket orientation: per-tensor inDefaultTensorCanonicalizer::canonicalize_braket(unfold-first, marker-convergent), and inTensorNetworkV3where orientation is graph-dictated (content rule demoted to a tie-break on equal bra/ket space multisets), with a post-relabel refold socanonicalizeis idempotent.create_graphbundles, gated by newbraket_foldable/braket_conjugate_foldablepredicates (c-number + symmetry checks in one place instead of five hand-rolled copies).SlotCanonicalizationMetadata::conjugated_tensors(input ordinals) reports which tensors need conjugation, replacing the singleconjparity bit (kept one cycle as[[deprecated]]; intended to dissolve intoTreeIndexwhen it lands).CanonicalizeSlotsOptionsstruct with defaultednamed_index_compare; the 3-argcanonicalize_slotsremains as a deprecated forwarder, in-tree call sites migrated.Conjugation algebra
Re/Imexpression nodes (core/expressions/complex.hpp) with eager rules: linearity, real-scalar hoist, i-rotation, composition table, conj action.fold_conjugate_pairs: exactA + A* → 2·Re(A)/A − A* → 2i·Im(A), hash-bucketed matching, pluggable conjugate op;fold_conjugate_pairs_of_real_sumkept as a back-compat wrapper. Auto-fold insimplify()is behindSimplifyOptions::FoldConjugatePairs, default No until evaluators understand Re/Im.sequant::conjugate(ExprPtr)(distributes over Product/Sum, rejects operator content) andis_hermitian_network().Tensor::with_slotsrebuild API carriesconjugated()through tensor-rebuilding transforms (remove_spin, swap_bra_ket, expand_antisymm, swap_spin) — the root-cause class of earlier marker-loss bugs.#591 review objections addressed: per-tensor conj report instead of one network-level bool; orientation dictated by the canonicalized graph; conj handled in regular canonicalization, not only
canonicalize_slots; params bundled into an options struct with the foldability predicate.Not in this PR: evaluation (lazy conj), export/ITF golden churn, anti-conjugate
BraKetSymmetryfor time reversal (TODO recorded at the predicate).Tests: new
tests/unit/test_conjugation.cppcatalogue (atoms, Sum/Product, TN, eval-boundary compat); full ctest green on Debug+THROW (246/246) and Release+THROW (279/279).