Implement coefficient thresholding in Hamiltonian generation - #69
Implement coefficient thresholding in Hamiltonian generation#69AlbertLee125 wants to merge 11 commits into
Conversation
- 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.
There was a problem hiding this comment.
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_thresholdthrough configuration, active-space building, cache validation, metadata, and tensor.npzoutputs. - Documented behavior, caveats, and cache implications in
README.mdandTHRESHOLDING_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.
|
After reviewing the current implementation, I am concerned that this hotfix changes too much of The current PR combines several changes:
I think it would be safer to restore My proposed first step is:
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. |
|
The PR has been narrowed to configurable thresholding during spatial-to-spin-orbital tensor construction. The original JW/BK mapping, cache workflow, and I updated the PR description, resolved the Copilot review threads, completed the Li₂ regression checks for the historical behavior, explicit |
AlbertLee125
left a comment
There was a problem hiding this comment.
Blocking issues
- Threshold-independent active-space caching can produce tensors under one cutoff while metadata reports another.
- No committed test or CI path exercises the new converter or its cache interaction.
Nonblocking issues
- Clarify units, strict boundary behavior, and the exact active-space cache filename in the README.
- Consider exposing the setting through
build_config.pyandbuild_config_L_sweep.py, since bond-length sweeps are the motivating workflow.
Questions
- Is this intended as only the tensor-construction portion of Issue #31? For Li₂,
1e-8and0.0produce 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, historical14/436cached tensors, metadata0.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.
|
Pushed commits
Main changes
Tests run
Comments intentionally not addressed
Remaining risks
|
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:
The default value of
1.0e-8reproduces the historical OpenFermion tensor-filtering behavior. Setting the value to0.0disables magnitude-based filtering during the spin-orbital tensor construction stage.Implementation
The implementation is intentionally limited in scope:
config.pyexposes the example threshold setting.hamgen_types.pystores and validates the configured threshold.thresholding.pycontains the configurable spatial-to-spin-orbital tensor expansion.hamgen.pypasses 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.npzformat are also unchanged. The.npzfile continues to contain only:Regression results
The change was tested using Li₂/STO-3G with a 10-qubit active space.
mainoutput1.0e-80.0The
0.0case retained:1.0e-8;A regression script also verified that applying the historical cutoff to the zero-threshold tensors reconstructs the tensors produced with the explicit
1.0e-8setting.These results show that the explicit default reproduces the existing behavior while
0.0preserves 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.