feat: PT2 configuration selection for HI-NQS v3 (ADR-005)#31
Conversation
Co-authored-by: leo07010 <leo07010@gmail.com>
- HINQSSQDConfig: 9 new frozen fields (use_pt2_selection, pt2_top_k, max_basis_size, convergence_window, temperatures, loss weights) All default to backward-compatible values. - New _pt2_helpers.py with 3 pure functions: - compute_pt2_scores: EN-PT2 scoring via get_connections - evict_by_coefficient: ASCI-pattern |c_i|² eviction - compute_temperature: linear annealing - 22 TDD tests (11 config + 3 PT2 + 4 eviction + 4 temperature) Co-authored-by: leo07010 <leo07010@gmail.com>
_train_nqs_teacher now accepts teacher_weight/energy_weight/entropy_weight kwargs. Uses full |c_x|² joint distribution (NOT α/β marginal product). Energy term uses diagonal advantage with REINFORCE gradient. Gradient clipping (max_norm=1.0) added for training stability. Backward compatible: default weights give teacher-only loss. Co-authored-by: leo07010 <leo07010@gmail.com>
a94ac57 to
4c87876
Compare
…P3, ADR-005) When use_pt2_selection=True: - Temperature annealing (initial→final linear schedule) - PT2 EN scoring filters unique_new configs (keep top_k) - Coefficient-based eviction after diag (ASCI |c_i|² pattern) - Convergence window (N consecutive ΔE < threshold) - 3-term loss weights passed to _train_nqs_teacher When use_pt2_selection=False (default): zero change to existing behavior. All 32 PT2 tests + 405 regression tests pass. Co-authored-by: leo07010 <leo07010@gmail.com>
When basis exceeds _SPARSE_DIAG_THRESHOLD (10K), CIPSI uses build_sparse_hamiltonian + scipy eigsh instead of dense matrix_elements_fast + numpy eigh. Prevents OOM on large bases. Co-authored-by: leo07010 <leo07010@gmail.com>
- Fix np.argsort()[::-1] producing negative-stride arrays that torch cannot handle — add .copy() in hi_nqs_sqd.py and _pt2_helpers.py - Update AGENTS.md: add _pt2_helpers.py to repo structure - Update CHANGELOG.md: add PT2 selection, 3-term loss, CIPSI sparse entries LiH E2E verified: PT2 ON = 0.07 mHa (7.7x better than OFF = 0.54 mHa) Co-authored-by: leo07010 <leo07010@gmail.com>
There was a problem hiding this comment.
Pull request overview
Implements ADR-005’s PT2-based configuration selection flow for HI+NQS+SQD v3, reintroducing the PR #30 ideas with updated conventions and adding a CIPSI sparse diagonalization fallback.
Changes:
- Add standalone PT2 utilities (Epstein–Nesbet scoring, coefficient eviction, temperature annealing) and wire them into
run_hi_nqs_sqdbehind a config gate. - Extend
HINQSSQDConfigand enhance_train_nqs_teacherwith optional 3-term loss weights. - Add sparse
eigshdiagonalization path inCIPSISolverfor large bases, plus a comprehensive new PT2 selection test suite and ADR documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/qvartools/methods/nqs/_pt2_helpers.py |
New helper module for PT2 scoring, eviction, and annealing. |
src/qvartools/methods/nqs/hi_nqs_sqd.py |
Config extensions + PT2 selection integration + 3-term teacher training weights. |
src/qvartools/solvers/subspace/cipsi.py |
Sparse diagonalization fallback for large CIPSI bases. |
tests/test_methods/test_pt2_selection.py |
New tests covering config defaults, helper functions, teacher loss signature, integration wiring, and CIPSI sparse path. |
docs/decisions/005-pt2-selection-hi-nqs-v3.md |
New ADR documenting design decisions and implementation plan. |
CHANGELOG.md |
Changelog entries for PT2 selection, helpers, 3-term loss, and CIPSI sparse fallback. |
AGENTS.md |
Updates repository map to include _pt2_helpers.py and PT2 selection notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Critical fixes: - #1/#2: Use persistent prev_coeffs/prev_batch_configs for PT2 scoring (was using best_coeffs before it was defined in current iteration) - QuantumNoLab#5: Eviction checks cumulative_basis size (was checking best_batch_configs) - QuantumNoLab#7: Convert numpy indices to torch.LongTensor before indexing CUDA tensor - QuantumNoLab#8: Default energy_weight=0.0, entropy_weight=0.0 (was 0.1/0.05, silently changing existing behavior) Lower priority fixes: - QuantumNoLab#6: CIPSI docstring mentions build_sparse_hamiltonian requirement - QuantumNoLab#9: Warn when energy_weight>0 but hamiltonian=None Not applicable (won't fix): - QuantumNoLab#3/QuantumNoLab#4: config_integer_hash returns tuple for n_sites>=64; our CAS max is 58Q (<64), so int cast is safe Co-authored-by: leo07010 <leo07010@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove int() casts on config_integer_hash results. For n_sites >= 64, the hash is a tuple (hash_hi, hash_lo), not an int. Using hash values directly as dict keys works for both int (<64) and tuple (>=64) cases. Previously dismissed as "not applicable" (max registry 58Q) but user confirmed 64-72Q support is planned. Co-authored-by: leo07010 <leo07010@gmail.com>
Copilot fixes: - #1: top_idx on same device as unique_new (CUDA compat) - #2: use prev_energy (consistent with prev_coeffs eigenvector) - QuantumNoLab#3: eviction log only when eviction actually occurred - QuantumNoLab#4: remove dead test code, clarify as smoke test New: - 64+Q hash support (remove int() casts, use hash values directly) - Cr₂-CAS(12,32) @ 64Q and Cr₂-CAS(12,36) @ 72Q registry entries - Both MOLECULE_REGISTRY and _MOLECULE_INFO_REGISTRY synced (26 total) Co-authored-by: leo07010 <leo07010@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…antumNoLab#31) Co-authored-by: leo07010 <leo07010@gmail.com>
Critical fix: eviction previously used best_batch_configs (random subset) as cumulative_basis replacement, silently discarding configs from other batches. Now runs a dedicated full-basis diag (dense or sparse) to get |c_i|² for ALL configs before evicting — correct ASCI approach. Also: - CIPSI sparse test now spies on build_sparse_hamiltonian to verify dispatch - prev_batch_configs.clone() prevents memory leak from tensor reference - Log warning when max iterations exhausted without convergence Co-authored-by: leo07010 <leo07010@gmail.com>
517004c to
df38dd1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…lot PR QuantumNoLab#31) - diag_e moved to NQS device to prevent device mismatch - CIPSI test uses monkeypatch instead of manual restore (exception-safe) - Renamed test to test_cipsi_sparse_energy_matches_dense with actual comparison Co-authored-by: leo07010 <leo07010@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- cipsi.py: h_matrix.detach().cpu().numpy() instead of np.asarray (CUDA safe) - hi_nqs_sqd.py: after eviction, persist post-eviction basis/coeffs/energy as PT2 reference (was using stale pre-eviction batch state) Co-authored-by: leo07010 <leo07010@gmail.com>
CI flaky failure on Python 3.11 due to non-deterministic NQS sampling. Co-authored-by: leo07010 <leo07010@gmail.com>
- Use α/β marginals for teacher weights (not Cartesian product expansion which caused 76GB memory + 2.5hr runtime) - Add nuclear_repulsion to solve_fermion energy (returns electronic only) - C2H2 verified: -76.0246 Ha (0.004 mHa gap vs PR QuantumNoLab#30 target) Co-authored-by: leo07010 <leo07010@gmail.com>
α×β Cartesian product in solve_fermion gives dramatically better accuracy: - C2H2 24Q: 0.004 mHa (vs 133 mHa with gpu_solve_fermion) - N₂ 40Q: 13.1 mHa (vs 126 mHa with gpu_solve_fermion) Auto-detect at runtime; falls back to gpu_solve_fermion when not installed. Co-authored-by: leo07010 <leo07010@gmail.com>
- Remove recover_configurations (S-CORE) from IBM path — designed for noisy quantum hardware, not clean NQS samples. NH3 went from 1.5hr to 5.2s with no accuracy loss (FCI precision). - Remove dead import of recover_configurations - Read spin_sq from mol_info instead of hardcoded singlet (spin_sq=0) Verified: H2O FCI exact, NH3 FCI exact, C2H2 0.004 mHa, N₂ 40Q 13.1 mHa. Co-authored-by: leo07010 <leo07010@gmail.com>
|
關於 3 個 Copilot 留言(3025025401, 3025025430, 3025025439): 已在 commit e0423b3 修復:
最終 Benchmark(IBM solve_fermion + Numba,充足參數)
詳細 SCI 對比見最新留言。 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code fixes: - _train_nqs_teacher raises ValueError when energy_weight > 0 without hamiltonian - Tests patch _IBM_SQD_AVAILABLE=False for deterministic behavior Documentation updates: - AGENTS.md: 26 molecules (12+14 CAS), 64/72Q rows, IBM solver + S-CORE gotchas - README.md: 26 molecules, Cr2-CAS(12,32) 64Q and Cr2-CAS(12,36) 72Q - CHANGELOG.md: IBM solver auto-enable, S-CORE removal, nuclear_repulsion fix - api_reference.md: _pt2_helpers API (PT2 scoring, eviction, temperature) Co-authored-by: leo07010 <leo07010@gmail.com>
SCI vs HI-NQS 最終 Benchmark 對比(2026-04-02,Numba + IBM solver,充足參數)C2H2 24Q
HI-NQS 比 SCI 低 0.46 mHa,快 2.4 倍,組態更少。 N₂ CAS(10,20) 40Q
SCI 能量低 28.8 mHa,但花了 11.5 倍時間、30 倍記憶體,且在 50K configs 仍未收斂。 分析
|
Add smoke test, docs build, concurrency, pip cache descriptions to match the merged CI workflow.
C2H2: HI-NQS wins SCI by 0.46 mHa, 2.4x faster. 40Q: SCI wins by 28.8 mHa but 11.5x slower, 30x more RAM.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
advantage(x) = H_xx - <H_xx>_{p_teacher} (teacher-weighted mean),
not H_xx - E_0 as previously documented.
Code reviewFound 3 issues:
qvartools/src/qvartools/methods/nqs/hi_nqs_sqd.py Lines 322 to 325 in fe0e9ce
qvartools/src/qvartools/solvers/subspace/cipsi.py Lines 51 to 56 in fe0e9ce
qvartools/src/qvartools/methods/nqs/hi_nqs_sqd.py Lines 575 to 578 in fe0e9ce 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
1. use_ibm_solver: tri-state (None=auto, True=force, False=disable) instead of bool that silently overrides user opt-out 2. CIPSI _SPARSE_DIAG_THRESHOLD: 10K → 8K so sparse path is reachable with default max_basis_size=10K 3. Document PT2 E0 approximation from batch sub-sample vs full-basis Co-authored-by: leo07010 <leo07010@gmail.com>
回覆 code review(3 條,commit c961e0b)1.
2. CIPSI sparse dead code ✅ 已修正
3.
|
feat: HI-NQS v3 PT2 配置篩選(ADR-005)
重新實作 PR #30(@leo07010)的演算法改進,遵循專案慣例。
Co-authored-by: leo07010 leo07010@gmail.com
變更內容
use_pt2_selection=True)max_basis_size)initial_temperature→final_temperaturesolve_fermion自動啟用:安裝qiskit_addon_sqd時自動使用 α×β Cartesian product(精度大幅提升)_pt2_helpers.py:獨立純函式(PT2 scoring、eviction、temperature)_train_nqs_teacher加強驗證:energy_weight > 0但沒給hamiltonian時 raise ValueError_IBM_SQD_AVAILABLE確保不受安裝環境影響Benchmark 結果
小分子(HI-NQS IBM)
C2H2 24Q — HI-NQS vs SCI 對比(Numba + IBM solver,充足參數)
HI-NQS 低 0.46 mHa,快 2.4 倍,組態更少。
N₂ CAS(10,20) 40Q — SCI vs HI-NQS 對比
SCI 低 28.8 mHa,但 11.5x 時間、30x 記憶體、仍未收斂。差距改善方向:Issue #35 Tier 1(H-connection + E_PT2)。
實作進度
_pt2_helpers.py:PT2 scoring、eviction、temperature)run_hi_nqs_sqd主迴圈設計文件
見
docs/decisions/005-pt2-selection-hi-nqs-v3.md(ADR-005)。測試計畫
_IBM_SQD_AVAILABLE確保行為確定性部分解決 Issue #35(Tier 1 改善延後至 follow-up)