Hi -- apologies in advance if this is the wrong channel. I know a feature
contribution isn't really a "bug," but PractRand's Discussion forum looks
lightly trafficked and I was worried a post there might just get missed.
Better to ask forgiveness than permission -- please redirect me if there's a
more appropriate place for this, and no offense taken if so.
I'm Aly Graham. I've been using PractRand (0.96) as an independent
statistical check on pgprng, a PRNG construction I've been developing (with
Claude's help). Part of that testing means running PractRand against a
single generator's output continuously out to multiple terabytes, which on
real hardware means runs that take many hours to multiple days. Over that
timescale, an unattended machine WILL eventually get interrupted -- in my
case, Windows Update forcing a reboot mid-run, more than once -- and stock
PractRand has no way to pick a long run back up: an interruption meant
losing the entire test battery's accumulated state and starting over from
byte zero.
So I put together (again, with Claude's help) a checkpoint/resume layer for
the stdin-driven test tool, verified it under real repeated-kill conditions,
and found a real bug in PractRand itself while stress-testing it. I'd like
to offer the checkpoint/resume code back to the project in case it's useful
to you or to anyone else with the same long-unattended-run problem, and to
flag the bug regardless of whether you want the rest of it.
WHAT'S ATTACHED (two zips):
1 -practrand_checkpoint_contribution.zip
2 - practrand_checkpoint_examples.zip
practrand_checkpoint_contribution.zip -- built against stock PractRand
0.96: two new files plus one small patch to an existing file, plus two
small run scripts included purely as a worked example (see below):
RNG_test_stdin_checkpointed.cpp
A new, self-contained driver tool, not a modification of tools/RNG_test.cpp.
It reads one external RNG's raw 64-bit words from stdin (see "how output
gets fed in" below), runs the expanded test battery, and periodically
saves/restores full test-battery state to a file via an optional command-
line argument. I deliberately kept this as its own file rather than
editing RNG_test.cpp in place: RNG_test.cpp's job (built-in RNG-by-name
selection, metatests, table formatting) is orthogonal to reading one
external stream, and keeping the checkpoint feature in its own driver
keeps it auditable as a self-contained addition rather than tangled into
the existing tool.
CheckpointableTestManager.h
A subclass of TestManager (plain, single-threaded -- not
MultithreadedTestManager) adding save/load of the manager's own state:
blocks_so_far, plus the small "repeat region" some tests read backward
across block boundaries via negative indices. This is the piece that
took the most care to get right -- see the design note below.
gap16_warmup_fix.patch
A one-line real bug fix in the existing src/tests_walk_state.cpp,
described in full below. This is the only edit to stock source; the
checkpoint mechanism itself is additive (the two files above).
pgprng_v2_stream.py and restart_pgprng_v2_run.sh
NOT an addition to PractRand's own code -- these are pgprng-side run
scripts, included only as a worked example of coordinating the two
checkpoints together. See "why a coordinating run script is also
needed" below for what they do and why they're relevant here.
practrand_checkpoint_examples.zip -- the three worked-example seeds
referenced under THREE WORKED EXAMPLES below (nothing else in it).
RNG_test_stdin_checkpointed reads raw 8-byte (uint64) words from stdin via a
small StdinRNG64 adapter (a PractRand::RNGs::vRNG64 whose raw64() does a
blocking read from stdin, treating EOF as std::exit(0)). Any external
generator that can write its output as a raw binary stream of 64-bit words
to stdout can be tested with:
your_generator | ./RNG_test_stdin_checkpointed [checkpoint_file]
The checkpoint_file argument is optional and is the only thing added to the
command line versus a plain (non-checkpointed) stdin driver: omit it and
behavior is unchanged from a normal run; give it a path and the tool resumes
from it if a valid checkpoint exists there, and (re)writes it after every
completed doubling round via write-to-temp-then-rename (so a crash mid-write
can never leave a truncated checkpoint in place of a good one).
Checkpointing PractRand's own test-battery state solves half the problem.
The other half is that the RNG being tested usually has its OWN internal
state that also needs to survive a restart -- otherwise resuming PractRand
from its checkpoint just feeds it a different (or worse, non-reproducible)
byte stream than the one it thinks it's continuing. The two checkpoints (the
generator's, and PractRand's) have to be kept in sync: if a kill lands
between "PractRand just saved having tested N bytes" and "the generator just
saved having produced N bytes," the resumed run silently diverges from what
was actually tested.
I'm including, as a worked example rather than as something for PractRand's
own codebase (it's pgprng-side, not PractRand-side, and isn't part of what
I'm asking you to consider adding to PractRand), the two small scripts that
manage this for my own generator:
Getting this synchronization right took a few real iterations (an early
version could still lose sync under an external kill that landed mid-write,
not just between rounds) -- happy to share the details if useful, but I
don't want to pad this report with pgprng-internal debugging history that
has nothing to do with PractRand itself. The short version: anyone pairing
PractRand's new checkpoint/resume with a generator that has its own
persistent state will hit this same coordination problem, so I'm including
a working example rather than just the two PractRand-side files in
isolation.
CheckpointableTestManager's walk_state() saves the "repeat region" (the
trailing REPEATED_BLOCKS of already-tested data that some tests read
backward across block boundaries) by deriving it from the trailing blocks of
everything tested so far, at save time -- not by checkpointing whatever
buffer region prep_blocks() happens to have staged for the round that just
finished. That specific care is a lesson from an earlier prototype I built
against PractRand 0.86 while first exploring this idea: a naive version that
checkpointed the round's staging buffer directly was off by exactly one
round, since that buffer is only refreshed at the START of the next round.
Every test that only reads its own accumulated state was unaffected, but FPF
(the one test that reads backward across block boundaries) picked up a
handful of very slightly wrong bucket counts at resume boundaries. I'm
mentioning this only as context for a design choice that might otherwise
look unnecessarily indirect -- the 0.86 prototype itself isn't part of this
contribution (different PractRand version, and it also picked up some
unrelated multithreading/timing work I'm not proposing here), but the lesson
carried forward directly into how the 0.96 code above is written, so I
wanted that reasoning to be visible rather than silent.
PractRand::Tests::Gap16::walk_state() never walks the warmup member:
void PractRand::Tests::Gap16::walk_state(StateWalkingObject *walker) {
walk_base_state(walker);
walker->handle(autofail);
counts.walk_state(walker);
...
Gap16::init() always resets warmup to 65536 (its "not yet warmed up" value)
before a restore runs. Since warmup is never part of the walked state, a
restore always leaves it stuck at that initial value -- even though the real
warmup phase (which completes after roughly 1.5MB of data, tiny compared to
any run long enough to need checkpointing) finished long before any
checkpoint was ever taken. test_blocks()'s periodic check,
if (warmup) autofail = true;
then permanently autofails Gap-16 (and its Low16/64-folded sibling, which
shares this code path) on every round after any resume past roughly 512MB --
not a statistical failure, a mechanical one: the test silently and
permanently stops being meaningful for the rest of the run, without any
error or warning distinguishing it from a real anomaly.
Fix (attached as gap16_warmup_fix.patch against src/tests_walk_state.cpp):
void PractRand::Tests::Gap16::walk_state(StateWalkingObject *walker) {
walk_base_state(walker);
walker->handle(autofail);
walk_int(walker, warmup);
counts.walk_state(walker);
...
Verified via a rebuild plus a fresh smoke test and multiple real
checkpoint-stress runs (see below) -- Gap-16 now correctly reports as
warmed-up after any resume past the warmup boundary, matching a continuous
(non-interrupted) run.
To exercise both checkpoint mechanisms together under real, repeated,
forced-kill conditions (not just an orderly shutdown), I ran 20
independently-seeded generator+PractRand pairs, each one genuinely SIGKILLed
at three points (2^30, 2^31, and 2^32 bytes), with the generator's
in-memory state fully discarded and reconstructed from its own saved
checkpoint file before relaunching a brand-new PractRand process against the
same PractRand checkpoint file. 19 of 20 came back completely clean; I'm
attaching the logs and small state files for two of the clean ones plus the
one that flagged something, specifically so the flagged case is visible
rather than cherry-picked out:
All three seeds' files are bundled in practrand_checkpoint_examples.zip:
<seed>_gen_ckpt.json (the generator's own checkpoint, ~37KB) and
<seed>_practrand.log (PractRand's own console output for that seed,
~14-16KB) for each. NOT included: <seed>_practrand_ckpt.bin, PractRand's
own internal test-accumulator checkpoint file for each seed -- these are
470-600MB EACH (the expanded battery's fixed table sizes, not something that
shrinks for a demo), so 60 of them is not a reasonable ticket attachment.
Happy to provide any specific one of these if it would actually help verify
something -- just let me know which seed.</seed></seed></seed>
Again, sorry if a bug tracker isn't the right venue for the checkpoint/resume
offer -- I mainly wanted to make sure the Gap16 bug and the working
checkpoint code didn't just get lost in a discussion thread. Happy to answer
questions, provide any of the omitted large checkpoint files, or rework any
of this if you'd want it structured differently for inclusion.
Aly Graham
pgprng.dev@gmail.com
Correction: I copied the title from a text document. Didn't notice that I missed the initial character. First word in title should be Checkpoint. Also missed stating that the bug was found and fixed. New title is:
Checkpoint/Resume for long stdin-driven runs (offered as a contribution, not a bug, sorry!), also one real Gap16 state-restore bug found and fixed along the way.
Not sure why I can't edit my own submission but I am stuck with a bad title. First time user with glitches.