fix(test): make test_stops build on Windows (mkdtemp compat shim)#352
Merged
Conversation
test_stops.c uses POSIX mkdtemp() to make a scratch dir in the CWD, but
MinGW-w64 does not declare mkdtemp, so the test failed to compile on the
Windows job - and only there:
tests/test_stops.c:73:9: error: implicit declaration of function 'mkdtemp';
did you mean 'mktemp'? [-Wimplicit-function-declaration]
make: *** [Makefile:318: tests/test_stops.exe] Error 1
That halted `make test` at the C-test stage on Windows (test_uring is
correctly Linux-only, so test_stops was the only blocker).
Added a compat_mkdtemp shim to compat.h, following the file's existing
convention (every platform difference lives there; the .c stays clean):
_mktemp fills the trailing X's in place (same contract as mkdtemp), then
_mkdir creates the directory. Also added <direct.h> for _mkdir. On Linux
compat.h is a complete no-op, so POSIX mkdtemp is untouched there.
Verified: test_stops builds clean on MinGW and all 5 sub-cases pass:
tokenizer special-flag parsing, config/generation_config eos union,
no-generation_config fallback, both-configs-mutilated tokenizer sweep,
and the T=NULL validation path.
This was referenced Jul 17, 2026
JustVugg
pushed a commit
that referenced
this pull request
Jul 17, 2026
β¦ not O(V log V) qsort (#335) dist_build() sorted the entire 151936-entry vocab by probability (qsort) on every sampled token whenever 0 < g_nuc < 1 β the serve default β and again per draft position under rejection sampling. Measured cost: 5.6-8.0 ms/call; the actual work is finding the few-hundred-token head whose cumulative mass reaches g_nuc. Replace the full qsort + linear scan with a max-heap partial select: - Floyd heapify g_pidx over V by descending g_pbuf prob (O(V), cache-friendly) - pop winners to the array's high end until cum >= g_nuc (k * O(log V)) - the remaining heap prefix IS the tail -> zero it, renormalize the head Winners land in g_pidx[out..V-1] in descending order, so s2 accumulates in the same order as before -> head is unchanged on tie-free shapes (ties were already unspecified under the unstable qsort). All four dist_build/dist_sample contract properties hold: g_pbuf stays id-indexed, g_pidx stays internal, the tail is fully zeroed, the head renormalizes to 1. No API change, no caller change, no new globals. c/tests/test_topp.c (new): drives the REAL dist_build via the include-glm.c pattern against an independent double-precision reimplementation of the OLD algorithm. 123 cases: 6 sizes (1..1519) x 5 shapes (uniform/peaked/geometric/ plateau/sharptail) x 4 nuc values, plus the g_nuc>=1 guard-off paths and V=1. Tie-free shapes compare head values within 1e-6 rel (float vs double renorm noise); tie shapes compare value-multisets. No scratch files -> builds clean on Windows MinGW without the unmerged mkdtemp shim (#352).
Merged
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.
What
make testhalted on the Windows job:tests/test_stops.cuses POSIXmkdtemp(), which MinGW-w64 doesn't declare, so the test failed to compile on Windows (and only there).This blocked the entire C-test phase on Windows. (
test_uringis correctly gated to Linux viaifneq (,$(LINUX)), sotest_stopswas the only Windows blocker.)Fix
Added a
compat_mkdtempshim toc/compat.h, following the file's stated convention β "every platform difference lives here; the.cfiles stay clean." POSIXmkdtemptakes achar[]template ending inXXXXXX, replaces the X's randomly, mkdirs0700, and returns the pointer (or NULL). On Windows:_mktemp(CRT,<io.h>) has the sameXXXXXXcontract;_mkdirneeded<direct.h>, added. On Linuxcompat.his a complete no-op β POSIXmkdtempis untouched.Why a shim and not a test-local fix
The earlier fix in
test_stops.c(commit on this file) already moved off/tmp/...to a CWD-relative path to fix the Windows job, butmkdtempitself was still POSIX-only. Acompat.hshim is the project's established pattern for exactly this (seegetline,setenv,posix_fadvise,pread,rename...) and keeps the test portable without#ifdef _WIN32inside the test source.Verification
On MinGW:
The two unrelated
-Wallwarnings that still show in this branch's build (compat.h:241pragma,glm.c:1210g_numa_nodes) are addressed by #350 β this PR is based onorigin/devand is independent of it.