CHEM-3200 Lab 2 findings: SCF robustness, RKS/UKS labeling, batch serialization gaps, SLURM CLI + checkpoint/resume - #118
Merged
Conversation
…(M-SCF-ROBUST) Zero SCF-robustness settings anywhere in the package -- no level_shift, no init_guess override, nothing exposed via CalculationRequest.options -- at nine independent mf.kernel() call sites. This broke a real class deployment: CHEM-3200 Lab 2's Mn2+ hexaaquo single-point (B3LYP/def2-SVP, sextet) oscillated 40-60 Ha every ~9 cycles and never converged under plain defaults, even though the request itself was correct. See QuantUI-development-tracking/TODO/GOTCHAS.md and TODO/roadmaps/52-m-scf-robust-open-shell-convergence-roadmap.md. - quantui/scf_robust.py (new): shared run_scf_with_rescue() helper -- same-basis HF/UHF bootstrap density, then a level_shift=0.3 + init_guess='atom' fallback, only when the plain attempt doesn't converge. Both stages were validated against the real failing case before this helper was written; a calculation that converges on the first try is a true no-op (zero extra SCF passes). Provenance stamped onto mf.scf_rescue_stage (SCFR.5). - Wired into all nine sites: session_calc.py, optimizer.py (+ pes_scan.py's shared _QuantUIPySCFCalc), freq_calc.py (main SCF + the per-displacement IR-intensity loop), tddft_calc.py, nmr_calc.py, raman_calc.py, freq_ir_workers.py, freq_raman_workers.py, and config.py's standalone "Export Script" template (a self-contained copy, since that script has no QuantUI dependency). - scf_rescue: bool = True added to every top-level entry point (run_in_session, optimize_geometry, run_freq_calc, run_tddft_calc, run_nmr_calc, run_pes_scan, run_reorganization_energy) and threaded through backends/worker.py from CalculationRequest.options["scf_rescue"] (SCFR.3) -- a batch/reproducibility caller can now opt out without a monkeypatch, retiring the reason CHEM-3200's course-side Slurm-Batch/robust_worker.py had to exist. - session_calc.SessionResult and the batch result.json payload (worker_payload.session_result_payload) both carry the new scf_rescue_stage field. - tests/test_scf_robust.py (new): fake-object control-flow tests for every branch (no-op, bootstrap success, level-shift fallback, rescue=False, max_stage=1, both stages exhausted) plus two real-PySCF integration tests (cheap RHF/STO-3G water, not the expensive literal Lab 2 case). - tests/test_code_quality.py: new static guard -- test_no_bare_scf_kernel_outside_rescue_helper -- fails CI if a future mf.kernel() call bypasses the helper. pytest -m "not network and not slow and not notebook": 3091 passed, 14 skipped. ruff + black clean. Contributions: - Claude (Sonnet 5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…n UHF gaps Three Lab-2-surfaced findings, closely related (all in the results- serialization/display path), landed together: M-UX2 UXP2.10 — results panel never labels RKS vs UKS. The restricted/ unrestricted dispatch is fully automatic from multiplicity -- correctly so, never a user toggle -- but nothing said which one ran. A student running Lab 2's six-metal series (5/6 open-shell) asked the instructor directly whether QuantUI even supported UKS. - New `scf_variant` field (SessionResult, FreqResult, TDDFTResult, NMRResult), captured as the real PySCF class name (RHF/UHF/ROHF/RKS/UKS) right after dispatch, before any density-fit/PCM/GPU/D3 wrap can rename it. - app_formatters._method_basis_label() shows "B3LYP/def2-SVP (UKS)" in every live result card + the shared History card; suppressed when redundant (RHF/UHF methods) or absent (older saved results). - Persisted through both save paths: results_storage.save_result() (the interactive app's History) and worker_payload.py's four *_result_payload() functions (the SLURM batch path). M-ISSUES ISSUE.10 — session_calc.run_in_session() computes Mulliken charges + dipole moment on every SessionResult, but worker_payload.session_result_payload() dropped them before they reached batch result.json. Lab 2's In-Lab Part A and Postwork Q2 need exactly these two fields from each single-point batch result. Added mulliken_charges/dipole_moment_debye/dipole_vector_debye/atom_symbols, getattr-guarded for the GPU-offload path (mf.mulliken_pop is NotImplemented there) and older saved results. M-ISSUES ISSUE.11 — save_molden() called pyscf.tools.molden.from_mo() directly, whose single-spin-channel code path raises IndexError on UHF/UKS's paired (alpha, beta) mo_coeff/mo_energy/mo_occ (shape (2, nao, nmo)). The bare except swallowed it, but from_mo had already written [Atoms]/[GTO] before reaching the failing part, so the truncated file (no [MO] block) looked like a normal, if oddly short, Molden file. 5 of 6 hexaaquametal(II) complexes in Lab 2 came back this way; only Zn (RKS) was complete. Fix replicates pyscf.tools.molden.dump_scf()'s own UHF handling (open once, write header once, call orbital_coeff() per spin channel, appending) since save_molden() only has raw arrays, never the live mf object dump_scf() needs. Tests: tests/test_session_calc.py (scf_variant per dispatch branch), tests/test_freq_calc.py / test_tddft_calc.py / test_nmr_calc.py (wiring confirmation), tests/test_app_formatters.py (label suppression rules), tests/test_results_storage.py (scf_variant + save_molden round-trip), tests/test_export_molden.py (new TestSaveMoldenWithOpenShellOrbitals — real UHF [MO] block + round-trip via molden.load(), plus an RHF control case), tests/test_worker_payload.py (new file — first direct unit coverage of worker_payload.py's four payload functions). Contributions: - Claude (Sonnet 5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…EL-aware M-CLUSTER2 CL2.7 — no CLI/scriptable equivalent to SlurmBackend.dispatch() existed (only log/gpu/analytics/setup/run subcommands). Real cost: CHEM-3200 Lab 2 needed 30 jobs (6 metals x 5 calc types) run centrally, no interactive app involved -- meant hand-authoring 30 CalculationRequest JSON files, a from-scratch sbatch template, and reinventing resource sizing by trial and error, only discovering afterward that estimate_slurm_resources() already existed and predicted the right numbers. - `quantui submit REQUEST_JSON [REQUEST_JSON ...]` wraps SlurmBackend.dispatch() + estimate_slurm_resources() directly -- same resource-sizing accuracy the interactive app gets, no reinventing it. Accepts one or more request files (the 30-job case): submits each in turn, sleeping out SlurmBackend's own post-submit cooldown between requests rather than tripping it partway through a legitimate scripted batch (proactive sleep, not reactive retry). - `--dry-run` prints the resolved cores/memory/walltime for each request without submitting -- cheap, local-only preview. - `--cores`/`--memory-gb`/`--walltime`/`--email`/`--mail-events`/ `--job-name`/`--depends-on`/`--partition`/`--no-apptainer`/ `--apptainer-image` mirror dispatch()'s own override kwargs. - Respects the same QUANTUI_ENABLE_SLURM site gate as the interactive app (dry-run exempt -- local computation only, no sbatch involved). - Partial-failure tolerant: one bad request file doesn't stop the rest of the batch; exit code reflects whether anything failed. M-CLUSTER2 CL2.9 — estimate_slurm_resources()'s frequency calc_factor=4.0 was independently validated correct for the *serial* IR-displacement loop (a real 19-atom def2-SVP UKS frequency job needed exactly the 120-128GB it recommends), but had no awareness of QUANTUI_FREQ_PARALLEL=1: that opt-in fans per-displacement SCFs out across N worker *processes* running concurrently, each holding its own integrals/Fock matrix, so memory need scales with N, not thread count. Not yet triggered on a real deployment (the env var wasn't set for Lab 2) -- a documented latent risk closed before it became an observed one. - estimate_slurm_resources() now checks freq_ir_workers.freq_parallel_opt_in() or a "frequency" request and applies a pick_worker_count()-sized memory multiplier, clamped to the site's MAX_MEMORY_GB. Always returns freq_parallel_memory_multiplier (1 when inactive) for transparency. - `quantui submit --dry-run` surfaces the multiplier when it fires. Tests: tests/test_cli.py (new TestSubmit -- dry-run, site gate, missing file, successful submit, resource overrides, multi-request batch with mocked sbatch + zeroed cooldown, partial failure), tests/ test_backends_registry.py (new TestEstimateResourcesFreqParallelAwareness -- multiplier off by default, off for non-frequency calc types, fires and scales when the env var is set, clamps to the site memory cap). pytest -m "not network and not slow and not notebook": 3136 passed, 14 skipped (full suite, all Lab 2 items in this session together). ruff + black clean. Contributions: - Claude (Sonnet 5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
quantui/checkpoint.py already lets optimize_geometry() save the ASE trajectory + BFGS Hessian after every step and resume from the last one, and run_pes_scan() save each completed scan point and only recompute the missing ones -- but quantui/backends/worker.py (the headless SLURM entrypoint) never constructed a Checkpoint or passed resume=True into either function (confirmed by grep: zero hits for "checkpoint"/"resume"). Every batch resubmission, including a job that OOM'd or timed out after finishing most of its work, restarted from nothing. Most of a pes_scan's cost is its N independent scan points, so this matters most for a killed run at, say, point 20 of 25 -- now only 5 are left to redo. - _begin_worker_checkpoint(): opens a Checkpoint scoped to <staging_dir>/.checkpoint/ (the job's own staging area, not the interactive app's ~/.quantui/checkpoints, so it stays self-contained and never collides with another job's), reads resumable_state() *before* calling begin() (begin() rewrites the metadata to "running", so reading after would describe the run about to start, not the one that stopped). - Wired into _run_geometry_opt and _run_pes_scan. - _maybe_run_preopt (shared by frequency/tddft/pes_scan) gets the same checkpoint, plus one extra wrinkle: its own starting geometry is fixed, but the *downstream* calc type's checkpoint identity requires exact geometry match, and two independent optimizer runs of the same molecule aren't guaranteed bit-identical (BLAS/OpenMP reduction order can differ run to run). So the preopt result is cached once, next to the request (preopt_geometry_<tag>.json) -- any later attempt on the same job reuses the fixed geometry instead of re-optimizing, guaranteeing the downstream checkpoint identity stays byte-identical across attempts (and skips the preopt's own cost on every resubmission besides). - frequency/tddft themselves still have no checkpoint hook anywhere in QuantUI (each is one atomic library call, not a chunked/resumable loop) -- nothing here can help those beyond the preopt step ahead of them. Ports and upstreams the validated course-side reference implementation (CHEM-3200's Slurm-Batch/checkpoint_patch.py, a runtime monkeypatch) -- this is now the real fix, not a workaround. Tests: tests/test_backends_worker.py (new TestCheckpointWiring) -- a fresh checkpoint on the first attempt (checkpoint/resume kwargs reach optimize_geometry/run_pes_scan correctly), resume=True + the right log line when a prior attempt left real progress (a non-empty trajectory file / a completed scan point), and the preopt geometry cache preventing a second optimize_geometry call on a simulated resubmission. Contributions: - Claude (Sonnet 5): code edits, review, and conceptual discussion - Jonathan Schultz: overall vision, planning, review, and orchestration Co-authored-by: Jonathan Schultz <nccu-schultz-lab@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… submodule test_scf_robust.py's _patch_bootstrap_scf fixture routed scf_robust.py's internal `from pyscf import scf` to a fake module via sys.modules["pyscf.scf"], but only patched the pyscf package's own `.scf` attribute `if _PYSCF_AVAILABLE`. On the windows-latest runner PySCF isn't installed at all (Linux/macOS/WSL only), so sys.modules["pyscf"] was never populated to begin with -- `from pyscf import scf` failed resolving the parent package itself, before the sys.modules["pyscf.scf"] patch was ever consulted, raising ModuleNotFoundError even in the fully-mocked _FakeSCF control-flow tests that need no real chemistry. Fix: when PySCF isn't importable, fake out sys.modules["pyscf"] too (a bare namespace exposing .scf), so the import resolves entirely from sys.modules without touching a real package on disk. Verified against a real ModuleNotFoundError (a meta-path finder that blocks pyscf import when not already cached) before applying the fix to the test file. Full suite green locally: 3136 passed, 14 skipped.
jonathanschultzNU
deleted the
claude/quantui-lab2-improvements-no3hwc
branch
September 7, 2026 03:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Six items from CHEM-3200 Lab 2's real NCShare deployment (2026-09-02–05), scoped and shipped in this session. Full writeups live in the (private) planning repo's
TODO/GOTCHAS.mdandTODO/roadmaps/{38,44,49,52}-*.md.M-SCF-ROBUST — Zero SCF-robustness settings anywhere in the package (no
level_shift/init_guessoverride) at ninemf.kernel()call sites — broke a real class deployment (Mn²⁺ hexaaquo single-point never converged). Adds a sharedquantui/scf_robust.pyhelper (same-basis bootstrap → level-shift fallback, no-op when the first attempt already converges), wired into all nine sites +pes_scan.py/reorganization_energy.py. Exposed viaCalculationRequest.options["scf_rescue"]throughbackends/worker.py. A static guard test (test_no_bare_scf_kernel_outside_rescue_helper) fails CI if a future baremf.kernel()bypasses it.M-UX2 UXP2.10 — Results panel never labeled RKS vs UKS (the restricted/unrestricted DFT dispatch is automatic from multiplicity, but nothing said which one ran — a student asked the instructor directly whether QuantUI even supported UKS). Adds a
scf_variantfield, shown as e.g. "B3LYP/def2-SVP (UKS)" in every result card and saved History result.M-ISSUES ISSUE.10 / ISSUE.11 — Two batch/headless serialization gaps found via Lab 2's centralized Slurm run: (1) Mulliken charges + dipole moment were computed but dropped before reaching batch
result.json; (2)save_molden()wrote a truncated file (missing[MO]block) for every UHF/UKS result, since it calledpyscf.tools.molden.from_mo()directly instead ofdump_scf()'s per-spin-channel handling. Both fixed.M-CLUSTER2 CL2.7 / CL2.8 / CL2.9 — Headless batch tooling gaps surfaced by running Lab 2's jobs centrally (30 jobs, 6 metals × 5 calc types, no interactive app):
quantui submit REQUEST_JSON [...]CLI wrappingSlurmBackend.dispatch()+estimate_slurm_resources(), with--dry-run, resource overrides, theQUANTUI_ENABLE_SLURMsite gate, and cooldown-aware multi-request batching.Checkpoint/resume wired intoworker.pyforgeometry_opt/pes_scan(+ a preopt-idempotency fix ahead of frequency/tddft/pes_scan) — upstreams a validated course-side reference implementation.estimate_slurm_resources()now applies a memory multiplier whenQUANTUI_FREQ_PARALLEL=1is active for afrequencyrequest.Testing
pytest -m "not network and not slow and not notebook": 3136 passed, 14 skipped, no failures.ruff+blackclean on the full tree. New/extended coverage:test_scf_robust.py(new),test_worker_payload.py(new), plus extensions totest_session_calc.py,test_optimizer.py,test_freq_calc.py,test_tddft_calc.py,test_nmr_calc.py,test_raman_calc.py,test_freq_ir_workers.py,test_freq_raman_workers.py,test_pes_scan.py,test_reorganization_energy.py,test_app_formatters.py,test_results_storage.py,test_export_molden.py,test_code_quality.py,test_cli.py,test_backends_registry.py,test_backends_worker.py.Authorship
Generated by Claude Code