Implement coefficient thresholding in Hamiltonian generation

[AlbertLee125]

Summary

Adds configurable coefficient filtering while expanding active-space spatial integrals into spin-orbital tensors. This addresses the tensor-stage threshold identified in #31 while retaining the historical 1e-8 default and allowing 0.0 at that stage.

Threshold behavior

• hamiltonian.coefficient_threshold controls tensor construction, defaults to 1e-8 Hartree, and accepts finite non-negative values.
• 0.0 disables magnitude filtering during tensor construction.
• After the standard OpenFermion JW or BK mapping, QHAT intentionally applies a separate fixed 1e-8 Hartree cutoff to final Pauli-string coefficients. This second cutoff is not currently configurable.
• Both thresholds are recorded separately in generated .dat metadata.

Cache compatibility

Active-space pickles record the tensor threshold. QHAT rejects a cache whose stored threshold differs from the requested value; legacy caches are interpreted as using the historical 1e-8 cutoff. The .tensors.npz format remains unchanged.

Tests

Regression coverage verifies validation, OpenFermion default parity, zero-threshold tensor behavior, strict cutoff boundaries, active-space construction, cache compatibility, JW/BK Pauli filtering, and threshold metadata.

[Copilot]

Pull request overview

Introduces a configurable absolute coefficient cutoff for Hamiltonian generation, so users can reproduce historical OpenFermion-style pruning (default 1e-8) or disable magnitude-based pruning (0.0) while preserving provenance in outputs and cache behavior.

Changes:

• Added a local thresholding implementation for (1) spatial→spin-orbital tensor expansion and (2) JW/BK mapping that applies cutoff only after Pauli-term aggregation.
• Plumbed hamiltonian.coefficient_threshold through configuration, active-space building, cache validation, metadata, and tensor .npz outputs.
• Documented behavior, caveats, and cache implications in README.md and THRESHOLDING_HOTFIX.md.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
hamiltonian_generator/thresholding.py	Implements threshold validation, tensor thresholding, and mapping that defers cutoff until after Pauli aggregation.
hamiltonian_generator/THRESHOLDING_HOTFIX.md	Hotfix write-up explaining motivation, cache behavior, and validation notes.
hamiltonian_generator/README.md	Documents the new hamiltonian.coefficient_threshold setting and its semantics.
hamiltonian_generator/hamgen.py	Uses the new thresholding path, records provenance, and enforces cache compatibility.
hamiltonian_generator/hamgen_types.py	Exposes and validates coefficient_threshold in the configuration object.
hamiltonian_generator/config.py	Adds the example configuration knob for the coefficient threshold.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[AlbertLee125]

After reviewing the current implementation, I am concerned that this hotfix changes too much of hamgen.py for the original goal of exposing the OpenFermion coefficient threshold.

The current PR combines several changes:

• replacing the active-space Hamiltonian construction;
• replacing the existing JW/BK mapping path;
• changing cache-loading behavior;
• adding threshold statistics and logging;
• adding new fields to the .tensors.npz output;
• adding unrelated defensive changes.

I think it would be safer to restore hamgen.py close to the current main branch and apply the threshold changes with minimal surgery.

My proposed first step is:

1. Keep the new threshold configuration and validation.
2. Use a small QHAT-owned function only for the spatial-to-spin-orbital construction where OpenFermion currently applies its fixed cutoff.
3. Preserve the existing JW/BK mapping code in hamgen.py.
4. Preserve the existing .tensors.npz format.
5. Add only the minimum metadata needed to record the selected threshold.
6. Handle more extensive cache provenance, tensor statistics, and Pauli-level thresholding as separate follow-up changes.

There is still an important question about OpenFermion applying another effective cutoff while constructing the final Pauli Hamiltonian. I think that issue should be tested and discussed separately rather than replacing the entire mapping path as part of the initial hotfix.

This would make the first change easier to review, reduce the risk of changing existing behavior, and clearly separate the tensor-threshold issue from the later Pauli-mapping issue.

[AlbertLee125]

The PR has been narrowed to configurable thresholding during spatial-to-spin-orbital tensor construction. The original JW/BK mapping, cache workflow, and .tensors.npz format are preserved.

I updated the PR description, resolved the Copilot review threads, completed the Li₂ regression checks for the historical behavior, explicit 1e-8, and 0.0, and all CI checks are passing. This is ready for human review.

[AlbertLee125]

Blocking issues

1. Threshold-independent active-space caching can produce tensors under one cutoff while metadata reports another.
2. No committed test or CI path exercises the new converter or its cache interaction.

Nonblocking issues

1. Clarify units, strict boundary behavior, and the exact active-space cache filename in the README.
2. Consider exposing the setting through build_config.py and build_config_L_sweep.py, since bond-length sweeps are the motivating workflow.

Questions

1. Is this intended as only the tensor-construction portion of Issue #31? For Li₂, 1e-8 and 0.0 produce different tensors but identical final JW/BK operators (156 terms), so Issue #31’s final Hamiltonian term-count question remains open.

Tests run and outcomes

• git diff --check main...Threshold: passed.
• Hamiltonian-generator smoke command: passed after using the checkout on PYTHONPATH; it loaded the tracked active-space cache and did not exercise the changed path.
• Targeted threshold parity, zero, boundary, and validation checks: passed.
• Cache-provenance reproduction: failed the expected invariant (requested 0.0, historical 14/436 cached tensors, metadata 0.0).
• pytest hamiltonian_generator: 2 unrelated tests passed.
• pytest common/tests: 190 passed.
• pytest analysis/tests: 270 passed.
• GitHub Actions is green, but the hamgen job bypasses the new converter through the tracked cache.

Merge verdict

No. I would not merge this until the blocking issues above are addressed.

[AlbertLee125]

Pushed commits

• 661bf09 adds cache provenance, regression coverage, GitHub Actions coverage, and documentation updates.
• 17a6403 simplifies the cache fix so get_ham2() retains its original control flow and only rejects an incompatible cache.

Main changes

• Newly generated active-space pickles record coefficient_threshold. Existing pre-feature pickles are interpreted as the historical 1e-8; a configured mismatch now stops with an instruction to remove [filestub]_[astag].pickle instead of silently emitting incorrect metadata.
• Added hamiltonian_generator/tests/test_thresholding.py and the hamgen-unit-tests GitHub Actions matrix entry. The existing GitLab configuration was intentionally left unchanged.
• Documented Hartree units, strict < cutoff behavior, equality retention, tensor-stage scope, and the exact cache filename. Updated the tracked Li₂ output with the threshold metadata line.

Tests run

• python -m pytest hamiltonian_generator -q -p no:cacheprovider: 17 passed on the final simplified implementation.
• python -m pytest common/tests -q -p no:cacheprovider: 190 passed.
• python -m pytest analysis/tests -q -p no:cacheprovider: 270 passed.
• Documented python hamgen.py smoke command: passed.
• GitHub Actions YAML parse and git diff --check: passed.
• The new GitHub Actions run is currently pending.

Comments intentionally not addressed

• Threshold flags in build_config.py and build_config_L_sweep.py were not added because they are optional sweep UX and outside this minimal hotfix. They can be handled separately.
• Pauli-level threshold control and the expected final Hamiltonian term count remain outside this PR. Stock OpenFermion JW/BK mapping is unchanged, so Issue Thresholding in QHAT #31 should remain open for that investigation.
• The two older Copilot threads required no further change: their custom mapping code had already been removed and those threads are resolved and outdated.

Remaining risks

• An unprovenanced cache produced by an intermediate version of this branch at 0.0 is indistinguishable from an official historical cache; it must be removed manually.
• Cache mismatches require manual removal by design. Automatic invalidation was intentionally avoided to keep get_ham2() unchanged apart from the validation guard.

[AlbertLee125]

Following the feedback to include thresholding at the qubit-operator stage, I added a second coefficient threshold on top of the existing configurable spin-orbital tensor threshold.

The two thresholds now apply at different stages:

• hamiltonian.coefficient_threshold controls filtering while constructing the spin-orbital tensors.
• A fixed Pauli-string coefficient threshold is applied after either the Jordan–Wigner or Bravyi–Kitaev mapping, removing final Pauli terms whose coefficient magnitude is strictly below the cutoff.

The Pauli-string threshold is intentionally kept internal and non-configurable for this baseline implementation. Making it configurable can be considered in a follow-up change. The applied Pauli-string threshold is also recorded in the generated .dat file metadata so that the two filtering stages are clearly distinguishable.

[AlbertLee125]

Blocking issues

1. The newly added fixed post-mapping cutoff changes final JW/BK Hamiltonians independently of the documented configuration and contradicts the stated tensor-only scope.

Nonblocking issues

1. The ground-energy comparison script is not reproducible as committed because all inputs are hard-coded and the required archives are untracked.

Questions

None.

Tests run and outcomes

• PYTHONPATH=.. python -m pytest hamiltonian_generator/tests/test_thresholding.py -v: 17 passed.
• git diff --check upstream/main...HEAD: passed.
• Latest GitHub Actions checks: all reported checks passed, including hamgen, hamgen unit tests, common unit tests, and analysis unit tests.
• Broader local pytest collection for hamiltonian_generator/tests common/tests analysis/tests: could not run to completion because this environment lacks qualtran, pyLIQTR, and numba; collection stopped with 13 import errors. The targeted changed-code suite completed successfully.
• Initial root-level targeted invocation without PYTHONPATH: could not import qhat because the checkout is not installed in the active environment; rerunning against the checkout via PYTHONPATH=.. passed.

Merge verdict

No. I would not merge this until the blocking issues above are addressed.

GitHub does not permit the PR author to submit REQUEST_CHANGES, so this is submitted as COMMENT.

[AlbertLee125]

Pushed commit

• 8f57bf0 addresses the latest review feedback.

Main changes

• Documented the intentional fixed 1e-8 Hartree Pauli-string cutoff separately from configurable tensor-stage thresholding, including the fact that tensor threshold 0.0 does not disable final Pauli filtering.
• Updated the PR description to match the implemented two-stage behavior.
• Removed compare_tensor_ground_energies.py; its hard-coded, untracked inputs made the experiment unreproducible as committed.

Tests run

• PYTHONPATH=.. python -m pytest hamiltonian_generator -q -p no:cacheprovider: 19 passed.
• git diff --check: passed.

Comments intentionally not addressed

• The request to remove or make the Pauli-stage threshold configurable was not implemented because the fixed Pauli cutoff is an intentional requirement for this change. The documentation and PR description now state that behavior explicitly; existing tests preserve JW/BK filtering, strict-boundary, and metadata behavior.
• Older resolved threads required no further changes; their cache, regression-test, documentation, and mapping fixes remain present.

Remaining risks and follow-up items

• The Pauli-stage threshold remains fixed and cannot be disabled independently. A separate configuration option can be considered in a follow-up change if that becomes a requirement.
• Broader common and analysis suites were not rerun locally because this environment lacks qualtran, pyLIQTR, and numba; the modified scope is covered by the passing Hamiltonian-generator suite.

[AlbertLee125]

@reuben-tate, could you please review this PR when you have a chance? The latest review feedback has been addressed, the intended tensor- and Pauli-stage threshold behavior is documented, and the targeted Hamiltonian-generator test suite passes. I believe it is ready for review.

[AlbertLee125]

We decided to remove the additional fixed Pauli-string threshold from this PR. The configurable tensor threshold directly addresses the original issue at the spin-orbital tensor-construction stage. Applying the same 1e-8 cutoff after the fermion-to-qubit mapping would introduce a separate approximation in a different representation, where that numerical value does not have an equivalent meaning because multiple fermionic coefficients can contribute to each Pauli coefficient. OpenFermion also applies its own numerical tolerance while constructing the Jordan-Wigner and Bravyi-Kitaev operators.

A fixed QHAT post-mapping cutoff could therefore silently discard valid mapped terms and could not be disabled when users request a zero tensor threshold. If Pauli-level truncation is useful, it should be introduced separately as an explicit, configurable QHAT feature with clearly defined behavior and error implications.