Repository navigation
Modernize 2026 - #1
Merged
Merged
Conversation
Removes material that is not needed to build or run kc on today's
platforms, as groundwork for a single portable build:
- old_src/, DPHF/ and Scripts/: retired generators, the Pascal KIN tool
and site-specific driver scripts with hard-coded 1990s paths
- kc-inst and the per-platform Makefile.* variants (AIX, DOS, HP-UX,
ULTRIX, ...); a CMake build replaces them in a later commit
- DOS 8.3 copies in Solvers/ (*.c0, *.c00, *.c01, eigen-te.c,
inte-tes.c) and the DOS fork kksoldos.c
- generated parser/lexer output (parser.c, lex.c, kc.c, y.tab); they are
now generated at build time and ignored
- orphans codegen2.* and finn.*, which no build ever compiled
- src/{complex,matrix,eigen,quench}.h, identical (quench.h: whitespace
only) to the Solvers/ copies that src/ already gets via -I
Everything remains available in git history.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replaces the per-platform Makefiles and kc-inst with a single CMake build that needs only a C compiler, flex and bison: - kcsolvers: static library with the numerical routines, linked into kc and into generated simulators - kc: the compiler; parser.c and lex.c are generated at build time - install: kc, kci, libkcsolvers.a, the solver headers (include/kc) and kksolver.c (share/kc) kci is the successor of the script kc-inst used to write: it runs kc in mode 3, compiles the model with kksolver and runs the simulation in the current directory. It now works in a temporary directory, so it no longer overwrites model.c/model.h in the user's directory. kc.l gets "%option noyywrap", which behaves like the yywrap() from libl/libfl and removes the need to link that library. The legacy sources still need pre-C99 compiler settings (gnu89, -fcommon, implicit declarations allowed). These are marked as temporary in CMakeLists.txt and are removed once the code is cleaned up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each example model in test/ is run through kc in all five code
generator modes, one CTest test per model and mode (365 tests):
cmake -B build && cmake --build build && ctest --test-dir build -j8
test/runner.c compares every file kc generates (model.c, in.dat,
model.f, kcm5*.p, stdout, stderr) with test/expected/<model>.m<mode>/.
For mode 3 it also compiles the model with kksolver, runs the
simulation and compares its final state (sim.last). Numbers are
compared with a relative tolerance of 1e-6 and timestamps are masked,
so the results should hold across compilers and platforms. KC_UPDATE=1
rewrites the expected output.
The expected output was recorded with the original, unmodified sources
(built with -O0), so later commits can show that they preserve
behaviour.
Running the original sources at -O0, at -O2 and with malloc scribbling
showed that several outputs depend on uninitialized memory. These cases
are listed in test/skip.txt and skipped until fixed:
- mode 1 crashes or hangs for 10 models
- mode 1 eigenvalue output varies for 8 models
- mode 2 writes uninitialized init-val values to in.dat for 23 models
Mode 3 simulations that fail with the original code (generated code
does not compile, step size underflow, crashes) have no sim.last, so
only their generated code is compared.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The KnCont generator checked a preference value with "(temp2>1.0) && (temp2<0.0)", which can never be true, so values outside [0, 1] were written without the intended warning. Use || so the check works as intended. None of the examples has a preference value outside [0, 1], so the expected test output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sources now compile without any warning on current clang and GCC, so the temporary pre-C99 compiler settings are gone. The expected test output is unchanged. - Globals defined in headers (codegen.h, misc.h, symbmath.h, tableman.h, Solvers/matrix.h, Solvers/odesolv.h) are now declared extern and defined once. They failed to link under -fno-common, the default since GCC 10 and clang 11. - Missing includes and prototypes added. Header typos fixed: GetReactionNo/NewParamter in tableman.h, and an #ifdef include guard in codecall.h that hid its prototypes. - Platform switches removed: _PLATFORM_*, MALLOCTYPE, the Boehm GC and dbmalloc variants, and HP-UX mallopt. - kc.l: proper prototypes for the helper functions. Removed the unused incl() and the "K" rule, which could never match (names win). - main.c: the warning for an unknown option printed argv[i][1] with %1s, using an unrelated global i (undefined behaviour). It now prints optopt. - TreeEval initializes its result, which was returned uninitialized when an operand failed to evaluate. - Unused variables removed, default cases added to switches over enums, and misleading indentation in Elmhes re-indented. Its semantics are unchanged and match the EISPACK elmhes algorithm. _POSIX_C_SOURCE is defined for getopt() and the fileno() used by flex. CMAKE_C_STANDARD can be overridden, e.g. -DCMAKE_C_STANDARD=23. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With GCC the mode 3 simulations of A2, C1 and D5 end within a relative difference of about 1e-5 from clang, because the adaptive step size control makes slightly different decisions. Compare sim.last with a relative tolerance of 1e-4. The Lorenz model is chaotic, so its end state cannot be compared at all and is skipped. The runner also builds without -Wformat-truncation warnings under GCC now: all formatting goes through one helper that fails on truncation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Builds with -Werror and runs the test suite on Linux (GCC and clang) and macOS (clang and GCC 14), plus one Linux job with C23. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GCC's -Wmaybe-uninitialized only fires with optimization, so the Release CI jobs with -Werror would have failed. None of these change the expected output: - tableman.c: lookup functions returned an uninitialized value when the name was not found; they now return 0.0 (callers still get NotFound). - tableman.c: NewDynVarConst/GetDynVarConst were missing braces, so the inner loop ran with an uninitialized j for non-matching entries. - mixed.c: without `epsa` in the model, the default was assigned to `temp` instead of `eps`, leaving eps uninitialized. It was 0 in practice; that value is now explicit. The intended 1.0e-18 changes mode 1 results, so that is left for a separate change. - kncont.c (need_dd_jac) and eigen.c hqr2 (r, s, z): initialize. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NormalizingMatrix picks the reference row by the modulus of V[row][col-1] + i*V[row][col]. For the last column (col == n) this read one element past the end of the row, so the chosen row, and with it the printed eigenvectors, depended on heap contents. AddressSanitizer reports it in mode 1 for E5, farrow and test3. The last column now has no imaginary part. Expected output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Clang fuses a*b+c into one FMA instruction by default on arm64; GCC (in ISO C mode) and x86-64 builds do not. The eigenvector computation in mode 1 compares values exactly, so the extra rounding step changed the printed eigenvectors for E5, farrow and test3: the expected output, recorded with clang on arm64, failed under GCC. Build with -ffp-contract=off and re-record those three files. Clang and GCC now produce identical mode 1 output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The timestamp pattern used lowercase 'a', which is_date() never checks, so those positions matched any byte, including the terminating NUL, and the scan could run past the end of the buffer. AddressSanitizer found it when the runner was built with -fsanitize=address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Solvers/complex.{c,h} was a small 1994 library (struct with re/im and
ComplexAdd/Mul/Div/... functions). C99 <complex.h> is part of the C
standard library, so it fits the "flex/bison + libc only" requirement,
and the custom header no longer shadows the standard one.
- eigen.c, quench.c, mixed.c: double complex with plain arithmetic,
creal/cimag/cabs/carg. Values are built as `x + y*I`, which is exact
for finite values; CMPLX() is avoided because Apple's <complex.h>
only defines it for clang.
- ComplexVectorAlloc/ComplexMatrixAlloc and their Free functions move
from matrix.{c,h} to eigen.{c,h}, their only user. matrix.h is
included by generated simulators (via kksolver.c), so it stays free of
the `I` and `complex` macros, which could clash with species names.
- complex.h is no longer installed.
cabs() and complex division are computed more carefully than the old
hand-written formulas; the expected output is unchanged on clang and
GCC.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EISPACK's hqr2 leaves the search for two consecutive small subdiagonal
elements when m reaches l ("if (m .eq. l) go to 150"). The C translation
compared against the digit 1 instead of the letter l, so for l > 1 the
loop ran past l and the QR step then read h[k-1][k-2] with k == 1, one
element before the row. AddressSanitizer reports it for test3 in mode 1
once the intended precision (epsa) default is used. Expected output is
unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without `epsa` in the model, mode 1 meant to use 1.0e-18 as the precision for the eigenvalue analysis but assigned it to the wrong variable, so the precision was 0 in practice. With 0, the QR iteration in hqr2 only stops on exact zeros: 8 models reported "Failure in hqr2 function. Do not trust the given eigenvectors and -values", and D3 printed all-zero eigenvectors. Now 1.0e-18 is used. Re-recorded mode 1 output for the 16 models without `epsa` whose results changed (mostly eigenvector normalization and sign). The 8 "output varies" entries in test/skip.txt are gone: with the out-of-bounds reads and FMA differences fixed, their output is stable across repeated runs and identical under clang and GCC, so it is now recorded and tested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the kc-inst and DOS installation notes with instructions for building, installing and testing with CMake, and describe kci. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a model uses time explicitly, kc adds "time" as the last dynamic variable. The names of the dynamic variables were stored one slot too low (an extra -IsNonAutoSystem()), so the last species name was overwritten, or species[-1] was written when the model has no species (test13). The values in x_ already used the right index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generated model.c calls strcpy without including <string.h>, and kksolver.c declared main as returning void. Both are errors in C99 and later, so the tests compiled the simulators with -std=gnu89. Emit the include, let main return int, and compile with -std=c17. With strcpy declared, _FORTIFY_SOURCE caught the out-of-bounds write fixed in the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generated reac_F had no return type, so it defaulted to int and truncated the double it computes. Implicit int is also invalid since C99. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When kc exits with an error, the runner now writes the exit status to a "status" file and compares it with the expected output like any other file, instead of failing the test right away. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment skippers never checked for end of input, so an unclosed comment made kc loop forever. They also consumed the character after every '*', so a closing "*/" right after another '*' (as in "**/") was missed, depending on how the stars lined up. Replace both with one function that tracks the previous character and reports an unclosed comment as an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Newlines inside comments were not counted, so every line number in kc's messages after a multi-line comment was too low. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The lexer copied every name into a 25-byte buffer without checking its length, so a longer species or constant name overwrote memory (and crashes kc when built with _FORTIFY_SOURCE). Report it as an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The flex shipped with Xcode on the GitHub macOS runners generates a lex.c that compares yy_size_t with int, so -Wextra -Werror broke the macOS CI builds. Suppress -Wsign-compare around the include of lex.c only, so our own code stays warning-free. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TreeCreate() returned uninitialized nodes. TreeReduce() always creates a scratch node but fills it only for non-leaf expressions, then frees it with TreeKill(), which follows the node's tag. When the leftover memory looked like an operator, function or negation node, TreeKill() freed stale pointers. With glibc 2.43 (Ubuntu 26.04, used by the CI runners) this crashed kc with a segfault or "double free or corruption" on many models, and made one CI job hang until the runner was killed. Allocate nodes with calloc so a fresh node is a harmless constant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The README already pointed to CHANGES.md for the changed mode 1 results, but the file did not exist. List the build, test, behaviour and bug-fix changes made on this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TreePrintXtra wrote every integral constant with (int)value. A value outside the int range, such as the rate constant 2*Krs*ksb = 4e11 in the reak model, is undefined behaviour to convert: arm64 saturates it to 2147483647 and x86-64 gives -2147483648. The generated KnCont code was wrong on both, and reak.m5 failed on the x86-64 Linux CI runners. Print such values with %e, like non-integral constants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The runner only reports the first differing line, which is not enough to see why a test fails on a platform we cannot run locally. Keep build/test-work as an artifact of failed jobs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
x86-64 computes 0/0 as a NaN with the sign bit set, which glibc prints
as "-nan"; arm64 gives "nan". dimitri.des.m1 and test6.des.m1 print
such values, so they failed on the x86-64 Linux CI runners. The
runner already treats any two NaNs as equal, but in padded columns
(" -nan" vs " nan") the extra blank made the texts
fall out of step before the numbers were reached. Treat runs of blanks
as equal regardless of length.
Co-Authored-By: Claude Opus 5.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.
Summary of the changes / Why this is an improvement
Checklist