Fix the qubit mapper and FCIDUMP export for 2-body tensors without 8-fold symmetry

[Kristjan Eimre (eimrek)]

Fixes #684. Unblocks #619.

Summary

The spin-summed fast path (majorana_map_impl) assumes 8-fold permutation symmetry of the two-body integrals, and nothing validated it — spin_symmetric=true went straight there. A tensor carrying only the 4-fold symmetry of #684 was therefore silently mapped to a different Hamiltonian, and to_fcidump_file likewise wrote only the ij <= kl triangle without checking the tensor could be reconstructed from it.

This adds the missing validation. The subtlety is that the general spin-channel path is not a blanket fallback for everything lacking 8-fold symmetry: it merges the alpha-beta and beta-alpha channels under (pq|rs) = (rs|pq), which together with the (pq|rs) = (qp|sr) fold makes it exact on the 4-fold subgroup and silently symmetrizing outside it. So the input is classified rather than tested against a single predicate.

The classification

A two-body tensor can carry three independent index symmetries, and they mean different things:

• (pq|rs) = (rs|pq) — electron exchange. This one does not change the operator at all: any part of the tensor that is antisymmetric under it cancels out, so it is purely a question of how the tensor is stored.
• (pq|rs) = (qp|sr) — Hermiticity of the Hamiltonian.
• (pq|rs) = (qp|rs) — the Coulomb bra swap, which holds only for real orbitals.

The first two are the 4-fold subgroup of #684, and any Hermitian two-body operator has them; adding the third gives the 8-fold symmetry of ordinary ERIs. Four cases follow, and no others are possible — see classify_two_body_symmetry:

Class	Input	Behavior
EightFold	ordinary ERIs over real orbitals	spin-summed fast path, as before
FourFold	any Hermitian two-body operator — SW-PT2, downfolding	general path, exact
NotBraKetSymmetric	the right operator, stored without the exchange symmetry	rejected; message says to average the tensor
NonHermitian	not a Hermitian operator at all	rejected

Per container

• Cholesky. sum_Q L^Q_pq L^Q_rs is 8-fold only when every factor is pair-symmetric, so that is now validated instead of forwarding the flag unchecked. Establishing the weaker 4-fold property would require materializing the dense tensor this path exists to avoid, so asymmetric factors are rejected rather than redirected.
• Sparse. An unstored entry means zero, but the engine expands one stored representative over its whole permutation class. Each class must now be stored once or in full; partial storage is ambiguous and rejected.
• FCIDUMP. Readers reconstruct all eight permutations while the writer emits only the ij <= kl triangle, so reduced-symmetry tensors cannot round-trip. The check runs before the destination is opened, leaving an existing file untouched on refusal. SparseHamiltonianContainer overrides to_fcidump_file and needed the sparse form of the check.

Tolerance

The comparison is relative to the largest integral in the tensor, because round-off from a four-index transformation scales with integral magnitude rather than sitting at an absolute floor — measured 3e-15 relative (cc-pVDZ) rising to 9e-12 (cc-pVQZ). An absolute threshold would reject ordinary closed-shell RHF Hamiltonians produced by this library's own SCF, which #684 requires to keep working, and would simultaneously stop detecting genuine asymmetry in small-magnitude tensors.

Behavior changes

majorana_map_hamiltonian and to_fcidump_file (base and sparse override) gain throwing preconditions — the "reject unsupported inputs explicitly" half of #684. Inputs reaching those throws were previously being mis-mapped or mis-exported, so this converts wrong answers into diagnostics; no input that previously produced a correct result is affected.

Notes for review

• The new tests check the mapped operator against a Fock-space reference built in the occupation-number basis, sharing no code with the engine, comparing sorted eigenvalues so the check is independent of qubit and mode ordering. Comparing spin_symmetric=true against false would not work here, since the classifier redirects the former to the latter for exactly the tensors under test.
• The unrestricted path makes the same exchange assumption on the aabb block, unchecked — the buffer API has no eri_bbaa to validate against. Pre-existing and orthogonal, so deliberately out of scope.

[Copilot]

🟡 Changes recommended

The new FCIDUMP Raises/@throws documentation omits invalid_argument/ValueError cases that the implementation can still throw, making the API docs incomplete.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR fixes incorrect behavior when mapping or exporting Hamiltonians whose two-body tensors do not have full 8-fold ERI permutation symmetry, by adding symmetry classification/validation and converting previously-silent mis-mappings / lossy FCIDUMP exports into explicit diagnostics.

Changes:

• Add dense/sparse/Cholesky-specific symmetry validation and dispatch (8-fold fast path vs 4-fold general path vs rejection).
• Add FCIDUMP precondition checks to prevent lossy exports (including sparse-container override).
• Add C++ and Python tests covering 4-fold mapping correctness, rejection cases, and relative-tolerance behavior; update docs accordingly.

File summaries

File	Description
python/tests/test_qdk_qubit_mapper.py	Adds Python-level regression tests for restricted 4-fold dispatch and explicit rejection cases (dense + sparse).
python/src/qdk_chemistry/algorithms/qubit_mapper/qdk_qubit_mapper.py	Updates mapper docs to describe new symmetry classification behavior and container-specific requirements.
python/src/pybind11/data/majorana_mapping.cpp	Updates Python binding docs for Majorana mapping to reflect 4-fold fallback and rejection cases.
python/src/pybind11/data/hamiltonian.cpp	Updates Python binding docs for FCIDUMP export to reflect new 8-fold precondition.
docs/source/user/comprehensive/algorithms/qubit_mapper.rst	Updates user docs describing sparse/cholesky constraints and 4-fold support for dense containers.
cpp/tests/test_majorana_mapping.cpp	Adds an independent Fock-space spectral oracle and tests for 8-fold/4-fold mapping + rejection paths.
cpp/tests/test_hamiltonian.cpp	Adds FCIDUMP tests for relative tolerance acceptance and rejection without touching existing files.
cpp/src/qdk/chemistry/data/two_body_symmetry.hpp	Introduces shared symmetry classification helpers and sparse/Cholesky validators.
cpp/src/qdk/chemistry/data/majorana_map_engine.cpp	Enforces symmetry classification/validation and redirects restricted 4-fold tensors to the general path.
cpp/src/qdk/chemistry/data/hamiltonian.cpp	Adds FCIDUMP 8-fold symmetry pre-check before opening the destination file.
cpp/src/qdk/chemistry/data/hamiltonian_containers/sparse.cpp	Adds sparse-specific FCIDUMP pre-check to reject partial/disagreeing permutation classes.
cpp/include/qdk/chemistry/data/majorana_mapping.hpp	Updates C++ API docs to describe new symmetry behavior and error conditions.
cpp/include/qdk/chemistry/data/hamiltonian.hpp	Updates C++ API docs for FCIDUMP export to mention new symmetry precondition.
cpp/include/qdk/chemistry/data/hamiltonian_containers/sparse.hpp	Updates sparse-container FCIDUMP docs for new rejection conditions.

Review details

Suppressed comments (2)

python/src/pybind11/data/hamiltonian.cpp:773

• This Raises: section lists only RuntimeError, but the underlying HamiltonianContainer::to_fcidump_file can also throw std::invalid_argument (mapped to Python ValueError) for restricted alpha/beta active-space size mismatches. Please include that in the docstring.

Raises:
    RuntimeError: If I/O error occurs, the Hamiltonian is unrestricted, or its
        two-body integrals lack the 8-fold permutation symmetry that FCIDUMP
        readers reconstruct.
)",

cpp/include/qdk/chemistry/data/hamiltonian.hpp:637

• Same as the container method: this Doxygen @throws clause documents only std::runtime_error, but the underlying implementation can also throw std::invalid_argument (e.g., restricted alpha/beta active-space size mismatch). Please document that for API completeness.

   * @throws std::runtime_error if I/O error occurs, the Hamiltonian is
   * unrestricted, or its two-body integrals lack 8-fold permutation symmetry
   */

• Files reviewed: 14/14 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The new C++ spectrum-based mapping reference test uses a self-adjoint eigensolver without first asserting Hermiticity, which can silently symmetrize a non-Hermitian mapped operator and weaken regression detection.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

cpp/tests/test_majorana_mapping.cpp:107

• mapped_spectrum passes a general complex matrix directly to Eigen::SelfAdjointEigenSolver without first asserting it is Hermitian. SelfAdjointEigenSolver only reads one triangle, so a non-Hermitian mapped operator could be silently symmetrized and still pass the spectrum comparison, weakening the test's ability to catch regressions. Add an explicit Hermiticity check (scaled tolerance) and verify the solver succeeded before using the eigenvalues.

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Lite