Estimator overhaul (M-PROGRESS Phase C) + calculation checkpoints (M-CHECKPOINT); v0.7.0 - #46
Open
NCCU-Schultz-Lab wants to merge 3 commits into
Open
Estimator overhaul (M-PROGRESS Phase C) + calculation checkpoints (M-CHECKPOINT); v0.7.0#46NCCU-Schultz-Lab wants to merge 3 commits into
NCCU-Schultz-Lab wants to merge 3 commits into
Conversation
Phase C was opened to fix a pre-run time estimate that M-EST's tier-4 validation had shown sitting outside its +/-25% band, on the assumption that the cost model needed work. Replaying the recorded history through the estimator said otherwise: the median *signed* error was ~0 for every calc type, so the predictor was not biased. It just had enormous spread. The data explains why. The history held 448 records for the identical tuple (frequency, H2O, RHF, 6-31G) with wall times from 0.34 s to 143 s. No cost model fits ground truth that disagrees with itself by 400x. Root cause: QuantUI's own tests were writing into ~/.quantui/logs/perf_log.jsonl. The *_analysis_history suites drive _do_run with a mocked calculation, so each one recorded a fabricated "H2O frequency" whose elapsed_s was really pytest-xdist wall time under contention. Roughly four fifths of the 2773 records were test artifacts. conftest isolated QUANTUI_RESULTS_DIR and QUANTUI_SETTINGS_PATH but never QUANTUI_LOG_DIR. Second contaminant of the same shape: benchmarks._calibration_worker started its perf_counter before importing quantui/pyscf, charging every calibration record with a fresh subprocess's import cost that an in-app run never pays. Worth recording that the pollution was *flattering* the score. Excluding the test-fixture tuples moves median |err| from 37% to 74.5%: 448 identical records are trivially self-predicting, so they inflated apparent accuracy while teaching the model nothing about chemistry. So this commit is about what gets measured and how it is labelled: - conftest._isolate_log_dir points QUANTUI_LOG_DIR at a temp dir for the whole session. The fix at the source; the rest is damage limitation. - log_calculation gains source/warm/import_s/stages, all additive and all omitted when unknown, so "untagged" stays distinguishable from "known to be an app run". - estimate_time partitions the pool by source the way it already did by gpu_used, falling back with a confidence downgrade. The downgrades compose, so falling back on both axes doesn't read as merely "medium". Two tagged records switch a calc type's pool over, so a polluted history is superseded rather than needing to be deleted. - Per-stage wall times are collected in _LogCapture at the existing emit_status boundaries, so no calc module needed new plumbing. _stage_key strips step counters so per-step re-announcements accumulate into one entry. - estimator_eval.py replays the history causally and reports accuracy next to coverage, so a model that stays silent can't look good by refusing to answer. The stage-aware frequency model is deliberately NOT built yet: no record carries stage data, and shipping an unvalidatable model is the exact mistake this phase exists to correct. 59 tests, mutation-checked against three seeded defects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An interrupted long run currently discards everything it finished. A geometry optimization eleven steps into twelve, a PES scan on its last point, a job killed by a closing laptop lid — all start again from nothing. This adds the storage layer and wires it into the three calc types that can usefully continue. The design decision that shaped the rest: resume and warm start are different operations and must not share a key. Resume continues *this* run, so its key includes geometry and calc type. Strict on purpose: resuming into a checkpoint from a different molecule would silently splice two calculations together, which is much worse than not resuming. Warm start reuses a converged density as an initial guess, so its key deliberately omits coordinates. An exact geometry match is the wrong test — a density from a nearby geometry is a good guess, which is exactly what a geometry optimization exploits internally. Checkpoints live in ~/.quantui/checkpoints, not under the result directory as originally sketched: a result directory is created when a calculation succeeds, and the runs that need a checkpoint are the ones that never get there. The constraint that outranks the feature: a checkpoint must never break a calculation. Corrupt metadata, a truncated append, a foreign schema version and an unwritable directory all behave exactly like "no checkpoint". load_state() returning None is the normal failure mode, not an exception — which matters here because a checkpoint is read precisely when something has already gone wrong. Hence os.replace for metadata, append-only for scan points, and treating a bare directory as "no progress" rather than offering a saving that doesn't exist. CHK.1: session_calc points mf.chkfile at the checkpoint and looks up a compatible earlier density. Never reuses the file it is about to overwrite — at best this run's previous attempt, at worst a partial write from the crash being recovered from. CHK.2: the ASE trajectory moves out of the TemporaryDirectory that was discarding it, and resume reloads the BFGS Hessian as well as the geometry. Without the Hessian, BFGS restarts as steepest descent and spends several steps relearning curvature it already had. Starting fresh into an existing checkpoint clears the stored Hessian first, since inheriting curvature from a run this one isn't continuing would be silent and undetectable. CHK.3: points are banked as they finish, and the cache is self-validating — each record stores the coordinate value it was computed at, so changing the scan range makes the match fail and the point recompute. Reusing a point also moves the live ASE atoms onto the stored geometry: each point relaxes from where the last one finished, so skipping without moving would start the next computed point from the wrong place and quietly change the profile. CHK.5: the offer lives on the Calculate tab rather than in History, where the app can say "8 of 20 scan points already computed" instead of asking a bare "Resume?". It refreshes on the same triggers as the time estimate, so it can never describe a calculation the user moved on from. CHK.4 (frequency displacement restart) is deferred. The displacement loop has no natural sequence point to checkpoint at, and freq_ir_workers can run displacements concurrently, so it would need to be safe under parallel append and resume a set rather than a prefix — a design task rather than a wiring one. 111 tests, mutation-checked. Full suite: 2321 passed, 25 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two milestones land here: M-PROGRESS Phase C (the estimator overhaul, which turned out to be a measurement problem rather than a modelling one) and M-CHECKPOINT (CHK.1/2/3/5 — resume an interrupted calculation). Minor rather than patch: checkpointing is a new user-facing capability, and perf records gained new fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Two milestones. They landed together because the first one's finding — that
QuantUI's own tests were writing into the user's real performance log — was
found while grounding the second.
M-PROGRESS Phase C — the model was never the problem
Phase C existed to fix a pre-run time estimate that M-EST's tier-4 validation
showed sitting outside its ±25% band. The assumption was that the cost model
needed work. Replaying the recorded history through the estimator said
otherwise:
Signed error ≈ 0 everywhere — no bias, which is what a wrong cost model looks
like. Just enormous spread. And the same predictor gets NMR to 87%, so the
spread had to be coming from the data.
It was. The history held 448 records for the identical tuple (frequency,
H2O, RHF, 6-31G) with wall times from 0.34 s to 143 s. No cost model fits
ground truth that disagrees with itself by 400×.
Root cause: the
*_analysis_historysuites drive_do_runend-to-end witha mocked calculation, and
conftest.pyisolatedQUANTUI_RESULTS_DIRandQUANTUI_SETTINGS_PATHbut neverQUANTUI_LOG_DIR. Each test recorded afabricated "H2O frequency" whose
elapsed_swas really pytest-xdist wall timeunder contention. Confirmed directly — running one test file appended 8 records
claiming 56.67 s and 57.47 s. Roughly four fifths of the 2 773 records were
test artifacts.
Second contaminant of the same shape:
benchmarks._calibration_workerstartedits
perf_counterbefore importing quantui/pyscf, charging every calibrationrecord with a fresh subprocess's import cost that an in-app run never pays.
test-fixture tuples moves median |err| from 37% to 74.5% — 448 identical
records are trivially self-predicting, so they inflated apparent accuracy while
teaching the model nothing about chemistry.
So this half is about what gets measured and how it's labelled:
conftest._isolate_log_dir— the fix at the source.log_calculationgainssource/warm/import_s/stages, all additive andomitted when unknown, so "untagged" stays distinguishable from "known to be
an app run".
estimate_timepartitions bysourcethe way it already did bygpu_used,with composing confidence downgrades. Self-healing: two tagged records
switch a calc type's pool over, so the polluted history is superseded rather
than needing deletion.
_LogCaptureat the existingemit_statusboundaries — no calc module needed new plumbing.
python -m quantui.estimator_eval— causal replay reporting accuracy nextto coverage, so a model that stays silent can't look good by refusing to
answer.
The stage-aware frequency model is deliberately not built: no record
carries stage data yet, and shipping an unvalidatable model is the exact
mistake this phase exists to correct. Tracked as PROG.C6.
M-CHECKPOINT — CHK.1, CHK.2, CHK.3, CHK.5
The design decision that shaped the rest: resume and warm start are
different operations and must not share a key. Resume continues this run, so
its key includes geometry — resuming into a checkpoint from a different
molecule would silently splice two calculations together. Warm start reuses a
converged density as an initial guess, so its key deliberately omits
coordinates: a density from a nearby geometry is a good guess, which is exactly
what a geometry optimization exploits internally.
Checkpoints live in
~/.quantui/checkpoints, not under the resultdirectory as the roadmap sketched — a result directory is created when a
calculation succeeds, and the runs that need a checkpoint never get there.
The constraint that outranks the feature: a checkpoint must never break a
calculation. Corrupt metadata, a truncated append, a foreign schema version
and an unwritable directory all behave exactly like "no checkpoint".
mf.chkfileinto the checkpoint dir; compatible earlier densityas the initial guess. Never reuses the file it's about to overwrite.
TemporaryDirectorythat wasdiscarding it. Resume reloads the BFGS Hessian as well as the geometry;
without it BFGS restarts as steepest descent and relearns curvature it had.
record stores the coordinate value it was computed at). Reusing a point also
moves the live ASE atoms, since each point relaxes from where the last
finished.
"8 of 20 scan points already computed" rather than asking a bare "Resume?".
CHK.4 (frequency displacements) deferred —
freq_ir_workerscan rundisplacements concurrently, so it needs to be safe under parallel append and
resume a set rather than a prefix. A design task, not a wiring one.
Testing
170 new tests. Both suites mutation-checked against seeded defects. The
checkpoint wiring tests are structural on purpose — the failure mode here is a
checkpoint created but never passed to the calculation, or a control built and
never added to its container, both of which have precedent in this repo.
Full suite: 2321 passed, 25 skipped.
🤖 Generated with Claude Code