Fix Standard-mode parameter checks and latent bugs in the C generator - #62
Open
aoymt wants to merge 6 commits into
Open
Fix Standard-mode parameter checks and latent bugs in the C generator#62aoymt wants to merge 6 commits into
aoymt wants to merge 6 commits into
Conversation
…values When _ur.dat or _jr.dat are absent, read_W90 returns early without setting tUJ[itUJ] or tUJindx[itUJ]. The subsequent cleanup code calls free() on these uninitialised pointers, which is undefined behaviour and causes SEGV on Linux/glibc when the heap has been previously used. Initialise all three elements to NULL immediately after malloc so that free(NULL) is called instead, which is safe per the C standard. Diagnosed with valgrind: 4x "Conditional jump or move depends on uninitialised value(s)" in free() called from StdFace_Wannier90. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without _ur.dat (NtUJ[1]==0), Uspin[] is malloc'd but never initialised. The superexchange formula J = 4t²/U then reads garbage values, producing non-reproducible output (inf or random J) and potential SEGV. Add an explicit check before Uspin is used: if _ur.dat is absent, print a clear error message and exit rather than proceeding with uninitialised memory. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tice In the spin-model branch the unused-parameter check for t'' inspected StdI->tp (already checked as t' on the previous line), so a specified t'' was silently ignored instead of being rejected. Check StdI->tpp. Note: inputs that previously specified t'' with a spin model were silently accepted; they now abort with the standard unused-parameter error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spin-model branch checked StdI->t twice: once correctly labeled "t" and once mislabeled "t0" (the real t0 is checked on the next line). Remove the redundant mislabeled line; specifying t in a spin model still aborts, now with a single correctly-labeled error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ycomb V' only acts as the fallback input for V0'/V1'(/V2') via StdFace_InputCoulombV, but Tetragonal and Honeycomb then re-ran StdFace_PrintVal_d on it, logging it as if it were an independent parameter (and resetting the consumed variable to 0). No other lattice does this and nothing reads Vp afterwards; stdout logging is the only observable change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The braceless ifs guarded only the sprintf, so when box[i][j] were still unset StdFace_PrintVal_d would run unconditionally with a stale label and a garbage default (NaN_i * 0.5). Unreachable in practice because StdFace_InitSite fills box first, so output is unchanged; the braces make the intended conditional explicit (and normalise a stray double space in the JVec line). Co-Authored-By: Claude Fable 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.
This PR collects six small fixes to the Standard-mode C generator found
during a systematic review (cross-checked against the Python port). All
fixes are confirmed against the full base-mode integration suite
(94/94 tests pass before and after; none of the changes affect the
generated definition files).
Fixes
TriangularLattice:
t''unused-parameter check inspected the wrongvariable (
e52627d)The spin-model branch checked
StdI->tp(already checked ast'onthe previous line) instead of
StdI->tpp, so a specifiedt''wassilently ignored.
⚠ Behavior change: spin-model inputs that specify
t''weresilently accepted before and now abort with the standard
unused-parameter error.
Kagome: duplicated, mislabeled check (
aca0821)StdI->twas checked twice in the spin-model branch, the second timeunder the wrong label
"t0"(the realt0is checked on the nextline). The redundant mislabeled line is removed; specifying
tstillaborts, now with a single correctly-labeled message.
Tetragonal / Honeycomb: re-print of the consumed
V'fallback(
e6d897d)V'only serves as the fallback input forV0'/V1'(/V2') viaStdFace_InputCoulombV, but these two lattices then re-ranStdFace_PrintVal_don it, logging it as if it were an independentparameter. stdout logging is the only observable change; no other
lattice does this.
Wannier90: braceless
ifs in thecutoff_*Vecparameter loops(
74bd911)The
ifguarded only thesprintf, so withbox[i][j]unset,StdFace_PrintVal_dwould run with a stale label and a garbagedefault (
NaN_i * 0.5). Unreachable in practice becauseStdFace_InitSitefillsboxfirst — output is unchanged — but thebraces make the intended conditional explicit.
Wannier90:
tUJ/tUJindxinitialised to NULL (5645bb4)Prevents freeing uninitialised pointers on early-exit paths.
Wannier90: error out when
_ur.datis missing for the spin model(
115e605)The superexchange J = 4t²/U needs the on-site U values; a missing
_ur.datpreviously crashed later instead of giving a clear error.Not included
A seventh finding — the
OmegaImdefault is computed as0.01 * (int)StdI->LargeValue(StdFace_main.c), truncating thefractional part of
LargeValue— changes a physics-facing default andis proposed separately as its own PR (
fix/omegaim-default) so it canbe discussed and merged independently.
Notes for reviewers
before and after the fixes; none of the reference files change.
pytest (3.10)CI job may fail on this branch for an unrelated,pre-existing reason (
tomllibis Python 3.11+ stdlib; fixed on thePython development line). This PR intentionally touches only
src/.🤖 Generated with Claude Code