Skip to content

Implement coefficient thresholding in Hamiltonian generation - #69

Open
AlbertLee125 wants to merge 11 commits into
mainfrom
Threshold
Open

Implement coefficient thresholding in Hamiltonian generation#69
AlbertLee125 wants to merge 11 commits into
mainfrom
Threshold

Conversation

@AlbertLee125

@AlbertLee125 AlbertLee125 commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR adds a configurable coefficient threshold for the spatial-to-spin-orbital tensor expansion in the Hamiltonian generator.

This PR addresses issue #31 by making the spatial-to-spin-orbital tensor coefficient threshold configurable in QHAT.

The new configuration option is:

hamiltonian.coefficient_threshold = 1.0e-8

The default value of 1.0e-8 reproduces the historical OpenFermion tensor-filtering behavior. Setting the value to 0.0 disables magnitude-based filtering during the spin-orbital tensor construction stage.

Implementation

The implementation is intentionally limited in scope:

  • config.py exposes the example threshold setting.
  • hamgen_types.py stores and validates the configured threshold.
  • thresholding.py contains the configurable spatial-to-spin-orbital tensor expansion.
  • hamgen.py passes the configured threshold into that expansion.

The original OpenFermion Jordan–Wigner and Bravyi–Kitaev mapping paths remain unchanged.

The existing active-space cache behavior and the .tensors.npz format are also unchanged. The .npz file continues to contain only:

constant
one_body
two_body

Regression results

The change was tested using Li₂/STO-3G with a 10-qubit active space.

Case Nonzero one-body entries Nonzero two-body entries
Existing main output 14 436
Current branch with 1.0e-8 14 436
Current branch with 0.0 50 2500

The 0.0 case retained:

  • 36 additional one-body entries below 1.0e-8;
  • 2,064 additional two-body entries below the historical cutoff.

A regression script also verified that applying the historical cutoff to the zero-threshold tensors reconstructs the tensors produced with the explicit 1.0e-8 setting.

These results show that the explicit default reproduces the existing behavior while 0.0 preserves the previously filtered tensor entries.

Scope

This PR controls coefficient filtering only during the spatial-to-spin-orbital tensor expansion.

OpenFermion’s standard Jordan–Wigner and Bravyi–Kitaev mappings may still apply their own small-coefficient handling when constructing the final Pauli Hamiltonian. Pauli-level threshold control is outside the scope of this PR and can be investigated separately.

- Introduced `hamiltonian.coefficient_threshold` to control the absolute cutoff for spin-orbital and final Pauli coefficients.
- Updated relevant functions to utilize the new threshold setting, allowing for disabling magnitude-based pruning.
- Documented changes in README and added a detailed hotfix explanation in THRESHOLDING_HOTFIX.md.
- Introduced `coefficient_threshold` in configuration with a default of 1e-8.
- Updated README to document the coefficient cutoff behavior.
- Modified `hamgen.py` to apply the coefficient threshold during Hamiltonian construction.
- Enhanced metadata logging to include threshold statistics.
Copilot AI review requested due to automatic review settings July 15, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread hamiltonian_generator/hamgen.py
Comment thread hamiltonian_generator/thresholding.py Outdated
@AlbertLee125

Copy link
Copy Markdown
Collaborator Author

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
AlbertLee125 requested a review from reuben-tate July 16, 2026 16:15
@AlbertLee125

Copy link
Copy Markdown
Collaborator Author

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 AlbertLee125 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread hamiltonian_generator/hamgen.py
Comment thread hamiltonian_generator/thresholding.py
Comment thread hamiltonian_generator/README.md Outdated
@AlbertLee125

Copy link
Copy Markdown
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants