Skip to content

Decoder evaluation: paired DUT/reference comparison and shot-corpus save/load - #432

Closed
ciaranra wants to merge 5 commits into
devfrom
decoder-eval-harness
Closed

Decoder evaluation: paired DUT/reference comparison and shot-corpus save/load#432
ciaranra wants to merge 5 commits into
devfrom
decoder-eval-harness

Conversation

@ciaranra

@ciaranra ciaranra commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

Adds paired decoder comparison: decode every shot in a SampleBatch with a decoder under
test (DUT) and a reference decoder, and report the joint outcome counts.

Today PECOS can tell a researcher "your logical error rate is 5%". It cannot tell them
whether a stronger decoder would rescue those same shots — so the choice between widen
the decoder
, fix the noise model, and stop optimizing is a guess. This makes the
first of those questions answerable.

SampleBatch.compare_decoders(dem, dut_decoder_type, reference_decoder_type, alpha=0.05)
returns a DecoderComparisonResult with:

  • the raw 3x3 counts (DUT outcome x reference outcome, each correct / mismatch /
    decode error), exposed both as a nested list and as named per-cell getters;
  • dut_only_failures — DUT wrong where the reference was right, i.e. measurable headroom;
  • both_failed — shots neither decoder got;
  • Jeffreys intervals on both headline proportions, via the existing pecos-num helper.

Three behaviors are deliberate and tested:

  1. A decode error is its own outcome, never folded into a logical mismatch. The
    existing decode_count folds them; that behavior is unchanged, and not copied.
  2. One decode error does not abort the run. The existing decode_each bails on the
    first error; this loop records the shot and continues. Both decoders are always run
    before either result is classified, so a DUT error cannot hide the reference's answer.
  3. Comparison is on wide ObsMask, with no 64-observable narrowing anywhere in the
    path.

Nothing here claims termination status, MAP-optimality, or "irreducible" failure. Those
are not knowable through ObservableDecoder — Tesseract's adapter discards
low_confidence, MWPF discards timeout status, and A* returns budget exhaustion as an
ordinary Ok — so they are reported as unavailable rather than inferred.

Verification

  • 7 Rust unit tests using deterministic stub decoders (no optional decoder features
    required): all-correct, a known wrong subset, DUT errors, reference errors, a

    64-observable difference at bit 70, the Jeffreys interval matching a direct
    pecos-num call, and determinism across repeated runs.

  • Guards mutation-tested: short-circuiting on a DUT error kills 2 tests; classifying an
    error as a mismatch kills 3; narrowing the comparison so wide bits vanish kills exactly
    the wide test. (A first narrowing attempt survived because the two masks had different
    word-vector lengths — a faulty mutant, not a vacuous test; re-run correctly it kills.)
  • Python integration test run against a maturin-built extension.
  • cargo fmt, cargo clippy -p pecos-rslib --all-targets -D warnings, and repo-wide
    pre-commit run --all-files all clean.

Scope

This is the first slice of a larger design (pecos-docs/design/decoder-failure-diagnosis.md),
which went through two adversarial review rounds and shrank substantially as a result.
Deliberately not here: a diagnostic trait, candidate-list/support-loss analysis, a
status ontology, work-metric plumbing, and evidence-weighted committees. Each is recorded
in the design note with the condition that would justify building it.

The natural follow-up is corpus export, now also on this branch.

Shot-corpus save/load (second commit)

SampleBatch.save(path, dem=..., metadata_json=None) / SampleBatch.load(path) freeze a
sample corpus to a single self-describing file: PECOSCORPUS\0 magic, u32 header length,
JSON header, then detector and observable columns as little-endian u64. The header carries
dimensions, the resolved seed, the exact DEM text, sha256 of both DEM and payload, an
opaque caller metadata_json string, and a format_version.

Why store shots rather than re-seed: generate_samples is serial and seed-deterministic,
so a seed does reproduce shots on another machine at the same PECOS version — but RNG
stream stability across versions is explicitly not promised (pecos-random/src/rapid_rng.rs
reserves the right to a "deliberate, release-noted reproducibility break"), an external
tool has no access to PECOS's RNG at all, and archived figures should not depend on any
internal staying fixed. (The thread-count non-determinism in sample_statistics_parallel
is real but applies to the parallel statistics/decode-count paths, which discard shots
entirely and therefore cannot be captured at all — a separate, named limitation.)

Supporting fix: generate_samples previously did
PecosRng::seed_from_u64(rand::rng().random()) when seed=None, generating and
immediately discarding the seed that produced the samples. It now resolves the seed once,
uses that value, and stores it on the batch, where save records it.

save requires the DEM (the batch does not carry one, and a corpus without it cannot be
decoded) and validates its detector/observable counts against the batch — write time is
the last point where a wrong-DEM mistake is cheap to catch. Decoder identities and configs
are deliberately not in the format: a corpus is the shots, a decoder spec belongs to a
run, and baking today's config shapes into a file format would rot it; callers record that
in metadata_json.

Verified: 12 Rust tests plus 8 Python tests, all re-run independently against a
maturin-built extension. Integrity guards mutation-tested — neutering the payload checksum,
the payload length check, the format_version check, or the save-time DEM dimension check
each kills at least one test. The load path uses checked arithmetic throughout and maps
malformed files to ValueError, never a panic.

Robustness review round (third commit)

An adversarial review of the diff returned REVISE. The headline finding was reproduced by
experiment before acting on it: editing one header field ("num_shots": 65 -> 66)
passed the dimension check, the payload-length check, and both hashes, and load()
returned a batch with a fabricated all-zero 66th shot. Only the DEM and payload were
authenticated; the header was not.

Fixes in this commit:

  • Whole-file integrity. The digest moved out of the JSON into a fixed 32-byte field
    after the length prefix and now covers header || payload, verified before any header
    field is interpreted semantically. payload_sha256 is gone (subsumed); dem_sha256
    remains as a convenience identifier, not the integrity mechanism. Duplicate JSON keys
    stop being a concern once the header bytes are authenticated.
  • Bounded dimensions. With num_shots == 0 the payload length was zero regardless of
    declared column count, so a tiny file could declare billions of detectors and drive a
    huge allocation. Explicit MAX_SHOTS / MAX_DETECTORS / MAX_OBSERVABLES are now
    checked before anything is allocated from them.
  • Honest DEM claim + real binding. The save-time check only compares dimensions, which
    cannot distinguish two same-dimensional models; the docstring said otherwise and has been
    corrected. Separately, a batch loaded from a corpus now rejects a different DEM across
    save, compare_decoders, and the decode_* family, with an explicit
    allow_dem_mismatch=True opt-out for deliberate cross-model work.
  • Argument validation before work. Empty batches and out-of-range alpha now raise
    ValueError up front instead of surfacing as RuntimeError after decoding the batch.
  • Provenance no longer silently dropped when re-saving a loaded batch; clear_metadata
    is the explicit way to discard it.
  • Canonical zero padding in the trailing word (masked on save, rejected on load),
    and FileNotFoundError instead of a bare OSError.

Verified: 55 Rust and 18 Python tests, re-run independently; the original tamper attack is
now rejected on num_shots, seed, and num_detectors. Guards mutation-tested —
neutering the content digest kills 4 tests (including both header-tamper tests), removing
the dimension checks kills 3, and accepting nonzero padding kills 1.

Deliberately not fixed, recorded instead: a 32-bit overflow in SparseDem::from_dem_str
(pre-existing, different crate, unreachable on 64-bit), the reader's payload copies
(performance, not correctness), and the ObservableDecoder state contract.

Pre-existing defect fixes (fourth commit, requested on-PR instead of as issues)

Two defects found during this work but pre-existing on dev:

DEM parser dimension overflow (32-bit targets). SparseDem, DemCheckMatrix, and
DemMatchingGraph all computed max_index as usize + 1; on a 32-bit target the maximal
index wraps to zero dimensions in release builds (silent misparse) or panics in debug. A
shared fallible dimension_count helper now promotes through u64 and converts with
usize::try_from, preserving 64-bit behavior exactly (a D4294967295 regression test
pins num_detectors == 4294967296) while producing a clean InvalidConfiguration where
the count cannot fit. parse_dem_metadata's raw u32 + 1 counters — which could panic
even on 64-bit — are now checked as well.

Decoder errors were folded into logical-error counts. decode_count,
decode_count_parallel, decode_stats, and decode_stats_parallel scored a decode
error as a logical error via .map_or(true, ...); worse, sample_decode_count_parallel
substituted the full observable selection mask as the prediction, so an erroring decoder
scored correct on any shot whose truth flips every observable. All scoring now goes
through one fail-loud helper (decoder_scoring.rs): any decoder error aborts with a
RuntimeError naming the failing shot (parallel paths report the globally lowest
failing shot index), matching the serial sample_decode_count precedent. Callers who
need per-shot error tolerance use compare_decoders, which reports errors as a distinct
outcome. For decoders that never error, all counts are unchanged — pinned by a seeded
regression (48 errors from 257 shots) and by masking/timing decorators that preserve the
existing per-shot semantics.

Verified: 189 Rust tests + 30 Python tests re-run independently; reverting the fail-loud
guard kills exactly the two new stub-decoder tests, including the
all-observables-flipped trap. The 32-bit error branch has no demonstrable kill on a
64-bit host — the fix is behavior-preserving there by design and the arithmetic is total
by construction; stated rather than faked.

@ciaranra ciaranra closed this Aug 4, 2026
@ciaranra ciaranra changed the title Paired DUT/reference decoder comparison with joint outcome counts Decoder evaluation: paired DUT/reference comparison and shot-corpus save/load Aug 6, 2026
@ciaranra

ciaranra commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Merged into #421 at the maintainer's request; the branch history is preserved there via a merge commit, and this PR's body remains the detailed record of the evaluation work's review rounds.

@ciaranra ciaranra closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant