dmft: fix five latent bugs in the qmc driver (with regression tests) - #102
Merged
Conversation
FourierTransformer::backward_ft short-circuits Green's-function channels whose high-frequency tail coefficients are all zero (c1 == c2 == c3 == 0) by writing G_tau = 0 instead of transforming. The third condition tested the truthiness of c3 (firing when c3 != 0) instead of c3 == 0 -- the opposite of its two siblings and of the comment on the same line. With the typo, an off-diagonal channel with an all-zero tail took the full-transform branch and leaked whatever bogus off-diagonal data the input G_omega carried into G_tau, instead of being zeroed. No production transformer reaches the multi-site off-diagonal path today (all in-tree constructors force n_site == 1), so this is latent -- but it silently corrupts any future cluster extension. The new test drives the base-class transformer at n_site == 2 with real diagonal data and bogus off-diagonal input: it fails on the pre-fix code (off-diagonal leak, max |G_tau| = 2) and passes with the guard corrected. Found by systematic audit of the DMFT applications in a downstream modernization fork; the fix is identical there and is covered by the same characterization test.
…ansform SemicircleHilbertTransformer::operator() (the imaginary-time Hilbert transform, reachable from main.C's Hirsch-Fye itime branch via selfconsistency_loop) was never implemented: it printed a message and called exit(1), followed by ~17 lines of statically-unreachable code that default-constructed a boost::shared_ptr<FourierTransformer> and immediately dereferenced it -- a guaranteed null-deref had control ever reached it (it cannot; exit(1) terminates first). Replace the body with a thrown std::logic_error so the unimplemented path reports through the driver's existing exception handling (clean error + message) instead of hard-aborting mid-run, and delete the dead tail. To be clear about the state of the feature: the Hirsch-Fye itime self-consistency path has no working imaginary-time Hilbert transform -- use the Matsubara (OMEGA_LOOP) path. The new test constructs the transformer and asserts operator() throws std::logic_error; it fails on the pre-fix code (the process exits non-zero via exit(1) before the assertion) and passes after. Found by systematic audit of the DMFT applications in a downstream modernization fork; the fix is identical there and is covered by the same characterization test.
ExternalSolver::solve() (the imaginary-time path) wrote the solver's
input archive with /parameters/OUTFILE set to the *input* filename:
p["INFILE"]=infile;
p["OUTFILE"]=infile; // <- copy-paste; solve_omega() sets outfile
An external solver executable that honours the persisted OUTFILE
parameter (the documented contract, and what solve_omega() provides)
writes its result over its own input file and the driver then fails to
find the output it is waiting for.
The new test runs solve() with a deterministic TMPNAME and a bogus
solver executable (so the input archive survives for inspection),
reads back /parameters from the input archive, and asserts OUTFILE
equals the .out.h5 path. It fails on the pre-fix code (OUTFILE points
at .in.h5) and passes after.
Found by systematic audit of the DMFT applications in a downstream
modernization fork; the fix is identical there and is covered by the
same characterization test.
The AFM branch of GeneralFSHilbertTransformer::operator() iterated
for(spin_t f=0; f<G_omega.nflavor()/2; f+=2)
with 2*f / 2*f+1 element indexing. The halved bound combined with the
stride-2 step visits only f=0 (and f=2 first at nflavor==8, mapping to
flavours 4,5 -- skipping 2,3 entirely): in any multiband AFM run
(FLAVORS >= 4) every flavour pair beyond the first is silently left
untransformed, so G0 for those bands never moves and their
self-consistency is broken. At FLAVORS==2 the loop is correct, which
is why the standard one-band AFM case never exposed it.
Rewrite the loop to iterate every (f, f+1) pair directly -- the same
form SemicircleFSHilbertTransformer::operator() already uses --
identical at nflavor==2 by inspection.
The new test drives the AFM transform at FLAVORS==4 with pair (2,3)
set identical to pair (0,1) over an identical two-band DOS and asserts
the output is symmetric across the pairs. It fails on the pre-fix code
(pair (2,3) passes through unchanged, asym ~ 1.8) and passes after.
Found by systematic audit of the DMFT applications in a downstream
modernization fork; the fix is identical there and is covered by the
same characterization test.
main.C's Matsubara solver-selection chain was a standalone-if /
if-else-if tangle that, for SOLVER=="Interaction Expansion":
- built the single-site half-filling solver for FLAVORS==1 and then
immediately overwrote it with another ImpuritySolver (a discarded
construction; the half-filling factory was never the final choice),
- compared FLAVORS as a string ("1"/"2") while every other reader in
the tree uses int, and
- fell through to ExternalSolver("Interaction Expansion") -- a
guaranteed exec failure -- for any other (FLAVORS, SITES)
combination.
Extract the (FLAVORS, SITES) -> run-type decision into a single
mutually-exclusive helper, select_interaction_expansion() in the new
interaction_expansion_choice.h, and drive main()'s selection from it.
The mapping preserves the previous *effective* behaviour for every
working configuration -- FLAVORS==2 -> single-site Hubbard run, else
SITES==1 -> multiband-density run -- now reading FLAVORS/SITES as ints
and building exactly one solver. The dropped half-filling factory
never ran (its product was always overwritten), and the remaining
(FLAVORS!=2, SITES!=1) combination -- previously the broken external
fall-through -- is now reported with an explicit error.
Maintainer judgment invited on one point: if the *original intent* was
for FLAVORS==1 to run the half-filling factory, the right fix is to
wire that case into the helper instead of dropping it; this patch
deliberately preserves the code's long-standing effective behaviour
rather than guessing at intent.
Locked by the selection-table unit test (all six (FLAVORS, SITES)
cases) and the dmft executable compile.
Found by systematic audit of the DMFT applications in a downstream
modernization fork; the fix is identical there and is covered by the
same characterization test.
skilledwolf
marked this pull request as ready for review
July 19, 2026 13:43
Ooolab
approved these changes
Jul 21, 2026
Contributor
|
Sure. Will wait for the workflow to finish and then will merge it. |
Contributor
|
failing builds because of Use build.verbose instead of cmake.verbose. Fixing by pulling in main. |
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
3 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.
Five self-contained bug fixes in
applications/dmft/qmc/, one commit each, every one carrying a regression test registered viaadd_alps_testin the app's CMakeLists (all pass underctest -R dmft_, 5/5). They came out of a systematic audit of the DMFT applications in a downstream modernization fork; each bug was re-confirmed present verbatim on current master (73b331006) and each test was checked to fail on the pre-fix code.1.
fouriertransform.C— backward_ft off-diagonal guard truthiness typoThe "nothing happening in this gf" short-circuit tests
c3_[f][s1][s2](truthy, i.e. fires when c3 ≠ 0) where its two siblings and the comment mean== 0. An off-diagonal channel with an all-zero tail therefore takes the full-transform branch and leaks bogus off-diagonal input into G_tau instead of being zeroed. Latent today (in-tree transformers force n_site==1); corrupts any future cluster extension.2.
hilberttransformer.C— itime semicircle transform: reachable exit(1) + null-deref dead codeSemicircleHilbertTransformer::operator()(reachable from the Hirsch-Fye itime branch viaselfconsistency_loop) was never implemented: it prints a message and callsexit(1), followed by ~17 statically-unreachable lines that dereference a default-constructed nullboost::shared_ptr<FourierTransformer>. Replaced with a thrownstd::logic_error(matchingHilbertTransformer::initial_G0's existing convention) and the dead tail deleted. To be explicit: the itime self-consistency path has no working Hilbert transform — use the Matsubara (OMEGA_LOOP) path.3.
externalsolver.C— itime solve() persists OUTFILE = the input pathAn external solver that honours the persisted
OUTFILEparameter (the contract, and whatsolve_omega()provides) writes its result over its own input file, and the driver then never finds the output.4.
hilberttransformer.C— AFM loop skips flavour pairs in multiband runsThe AFM branch of
GeneralFSHilbertTransformer::operator()iteratesfor(f=0; f<nflavor()/2; f+=2)with2*findexing: the halved bound combined with the stride-2 step means every flavour pair beyond the first is silently left untransformed in any multiband AFM run (FLAVORS ≥ 4) — those bands' G0 never moves, breaking their self-consistency. Correct at FLAVORS==2, which is why the one-band AFM case never exposed it. Rewritten to iterate every (f, f+1) pair, the same formSemicircleFSHilbertTransformeralready uses.5.
main.C— OMEGA_LOOP Interaction-Expansion factory selection untangledThe Matsubara solver-selection chain was a standalone-if / if-else-if tangle that (a) built the single-site half-filling solver for FLAVORS==1 and then immediately overwrote it with another solver (a discarded construction — the half-filling factory could never win), (b) compared FLAVORS as a string, and (c) fell through to
ExternalSolver("Interaction Expansion")— a guaranteed exec failure — for other (FLAVORS, SITES) combinations. The decision is extracted intoselect_interaction_expansion()in a new self-contained header, unit-tested over all six cases, preserving the previous effective behaviour (FLAVORS==2 → single-site, else SITES==1 → multiband) and reporting the previously-broken combination with an explicit error.Maintainer judgment invited on one point: if the original intent was for FLAVORS==1 to run the half-filling factory, the helper is the one place to wire that in — this PR deliberately preserves the long-standing effective behaviour rather than guessing at intent.
How to reproduce
ctest -R dmft_ --output-on-failure # 5/5 pass with this PREach test is a small standalone program next to the code it guards in
applications/dmft/qmc/; revert the corresponding fix commit and its test fails with the message listed below.Tests
dmft_fouriertransform_offdiag_numericdmft_hilberttransformer_itime_throwdmft_externalsolver_outfile_param.in.h5dmft_hilberttransformer_afm_multiband_numericdmft_interaction_expansion_choice