From 06373817a2632c683992bc79228130b6c373cab2 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Wed, 5 Aug 2026 17:43:51 +0200 Subject: [PATCH] refactor(config): give the tuned constants a real provenance, and make them reachable Two problems, both of which made a number look more settled than it is. The provenance was a dead link. config.hpp cited docs/rep4-optimizer-results.md for prob_threshold, extinction_sec, anneal_sec and the expansion default. That file was renamed to model-bakeoff.md and then rewritten; the comments were never repointed, so the most consequential constant in the pipeline appeared to have no source at all. Following it up produced something worse than a broken link. prob_threshold=0.754 comes from the ORIGINAL rep4 document (still readable at `git show d340da7:docs/rep4-optimizer-results.md`). The rewrite that replaced it reports finding "a real scoring bug in optimize.py: a candidate whose hardest film's replay timed out was averaged over survivors instead of penalized, silently rewarding partial coverage. Affected 3 of 16 training combos". So 0.754 was fitted under scoring that was later found wrong, the corrected sweep converged elsewhere, and no corrected prob_threshold is recorded anywhere. The comment now says that, along with the surviving document's own verdict that the optimum "generalizes unevenly -- strong on 3 of 5 held-out films, badly broken on 2". Four constants were unreachable. ownership_logodds lived on TrackRegistry::Config, and max_views/admit_below/rho_max on EvidenceDiscounter::Config, which main built with the one-argument constructor -- so nothing short of a recompile could move any of them. rho_max's own comment defers to "the sweep (VR-007)" for where it belongs, and that sweep could not reach it. They now live in Config with CLI flags and are exposed to the replay harness. ownership_logodds is worth singling out: below it a track makes no presence claim at all, so it decides whether an actor is reported rather than how confidently -- arguably the most consequential constant after prob_threshold, and until now unswept and unsettable. No behaviour change: every default is the value that was compiled in. TRACES: AR-025, AR-017 | SR-002 --- scripts/optimizer/replay.py | 7 +++- src/config.hpp | 75 +++++++++++++++++++++++++++++++++---- src/kpn_bindings.cpp | 5 +++ src/main.cpp | 24 +++++++++++- src/scene_preview.cpp | 7 +++- 5 files changed, 106 insertions(+), 12 deletions(-) diff --git a/scripts/optimizer/replay.py b/scripts/optimizer/replay.py index 14ac018..17bf5b8 100644 --- a/scripts/optimizer/replay.py +++ b/scripts/optimizer/replay.py @@ -277,7 +277,12 @@ def build_minimal(annotations, movie, fps, cfg) -> dict: # a stale entry here silently inert rather than loudly wrong. CFG_KEYS = ["detector_conf", "prob_threshold", "match_prior", "track_alpha", "track_min_iou", "track_assoc_min_prob", - "track_extinction_sec"] + "track_extinction_sec", + # AR-025 ownership and evidence accumulation. Newly reachable: + # these were in-class defaults no sweep could vary, which is why + # VR-007 never covered them despite rho_max deferring to it. + "ownership_logodds", "evidence_rho_max", "evidence_admit_below", + "evidence_max_views"] # Swept like a Config key but consumed entirely in Python, by build_minimal. # Kept separate so nobody has to guess which of these the pipeline actually diff --git a/src/config.hpp b/src/config.hpp index 5e45323..e361ad0 100644 --- a/src/config.hpp +++ b/src/config.hpp @@ -72,11 +72,38 @@ struct Config { std::string arcface_engine; // optional path to a pre-built TRT engine; bypasses ORT int embed_batch_size{4}; // max faces per ORT Run() call — bounds per-call latency float match_prior{0.5f}; // base-rate prior; 0.5 = use calibrated sigmoid directly - // prob_threshold tuned by Differential Evolution against Amazon X-Ray per-scene - // presence over 4 films, per-second metric (see docs/rep4-optimizer-results.md). - // Best model+mode: LVFace-B_Glint360K, full gallery, expansion on. Supersedes the - // earlier 9-film scene-union-metric tuning (0.76) — that metric is now known to - // have hidden out-of-cast false positives (see docs/optimizer-experiments.md). + // Tuned by Differential Evolution against Amazon X-Ray per-second presence + // over the 4-film rep4 matrix. Best model+mode: LVFace-B_Glint360K, full + // gallery, expansion on. Supersedes an earlier 9-film scene-union tuning + // (0.76); that metric hid out-of-cast false positives. + // + // **Read the provenance before trusting the value.** Two things about it: + // + // 1. The document it came from no longer exists under that name. It was + // docs/rep4-optimizer-results.md, renamed to docs/model-bakeoff.md and + // then rewritten (0bd2747). This comment pointed at the dead path for + // long enough that the number looked unsourced. The original is still + // readable at `git show d340da7:docs/rep4-optimizer-results.md`, where + // the shipped triple appears as + // `prob_threshold=0.754, anneal_sec=35.5`. + // + // 2. **0.754 predates a scoring bug fix and was never re-derived.** That + // same rewrite reports finding "a real scoring bug in optimize.py: a + // candidate whose hardest film's replay timed out was averaged over + // survivors instead of penalized, silently rewarding partial coverage. + // Affected 3 of 16 training combos". The corrected sweep converged + // somewhere else — the surviving document records anneal_sec=59.2, + // extinction_sec=59.2 against the 35.5/57.4 shipped alongside this + // threshold — and no corrected prob_threshold is recorded anywhere. + // (The other two constants are now withdrawn outright, which is why + // only this one still matters.) + // + // The doc is also candid that the optimum "generalizes unevenly — strong on + // 3 of 5 held-out films, badly broken on 2 (one with a 974-count misID + // blowup)", and that it is shipped anyway because it still beats the old + // defaults on average. That is a defensible call and not a settled, + // film-agnostic optimum; it should be visible here rather than only in a + // document this comment used to point at incorrectly. float prob_threshold{0.754f}; // posterior P(match | sim, prior) threshold // TRACES: AR-024 | SR-002 // match_threshold (0.45), match_ratio (0.80) and match_ratio_ceil (0.65) are @@ -150,6 +177,35 @@ struct Config { // ordinary miss, once for a cut). Seconds mean one thing at any sample rate. double track_extinction_sec{5.0}; + // ── Ownership and evidence accumulation (AR-025) ────────────────────────── + // TRACES: AR-025, AR-017 | SR-002 + // These four decided how presence is claimed and were unreachable: they + // lived as in-class initialisers on TrackRegistry::Config and + // EvidenceDiscounter::Config, and main constructed the discounter with the + // one-argument constructor, so nothing short of a recompile could move + // them. rho_max's own comment defers to "the sweep (VR-007)" for where it + // belongs — a sweep that could not reach it. + // + // ownership_logodds is arguably the most consequential constant in the + // pipeline after prob_threshold: below it a track produces no presence + // claim at all, so it decides whether an actor is reported rather than how + // confidently. 2.0 is a posterior of ~0.88. Unswept. + float ownership_logodds{2.0f}; + + // How much a single observation may move a track's belief. n_eff = + // n / (1 + (n-1)·rho), so rho_max caps what a repeated view can ever be + // worth: 0.5 caps it at two independent observations however long the shot + // runs. It is deliberately below 1 — a held pose still yields a fresh + // detection, alignment and noise realisation, so a little independent + // evidence survives. Setting it to 1 freezes belief after the first frame, + // which is the bug this replaced. + float evidence_rho_max{0.5f}; + // P(same view) below this and the observation counts as a genuinely new + // look, so it joins the per-track view set. + float evidence_admit_below{0.6f}; + // Distinct views remembered per track, which bounds the novelty comparison. + int evidence_max_views{8}; + // ── Scene tracking ──────────────────────────────────────────────────────── // TRACES: AR-012, AR-013 | SR-002 // extinction_sec (57.4) and anneal_sec (35.5) are GONE, along with @@ -178,9 +234,12 @@ struct Config { // new reference views; they are promoted into a per-film, in-memory annex so // later frames/tracks of that actor at similar poses recognise. See // gallery/track_gallery.hpp. - // Default ON: rep4 matrix (docs/rep4-optimizer-results.md) found expansion helps - // recall on the full (unrestricted) gallery for the winning model/mode — the - // opposite of the earlier assumption that it only helps restricted galleries. + // Default ON: the rep4 matrix (docs/model-bakeoff.md, "Two effects in + // isolation") found expansion helps recall on the full (unrestricted) + // gallery for the winning model/mode — the opposite of the earlier + // assumption that it only helps restricted galleries. The same section is + // explicit that on the full gallery it buys +2.1pp F1 and +3.9pp recall + // "at a real cost" in misIDs, where in restricted mode it is a clean win. bool expand_gallery{true}; // master switch int expand_buffer_size{20}; // per-track diversity buffer capacity // TRACES: AR-018, AR-024 | SR-005 diff --git a/src/kpn_bindings.cpp b/src/kpn_bindings.cpp index 6634116..dd5cca7 100644 --- a/src/kpn_bindings.cpp +++ b/src/kpn_bindings.cpp @@ -173,6 +173,11 @@ static Config config_from_dict(nb::dict d) { getf("track_min_iou", cfg.track_min_iou); getf("track_assoc_min_prob", cfg.track_assoc_min_prob); getd("track_extinction_sec", cfg.track_extinction_sec); + // AR-025: swept knobs, previously unreachable from any config. + getf("ownership_logodds", cfg.ownership_logodds); + getf("evidence_rho_max", cfg.evidence_rho_max); + getf("evidence_admit_below", cfg.evidence_admit_below); + geti("evidence_max_views", cfg.evidence_max_views); // gallery expansion (usually off for sweeps; expose so it can be toggled) if (d.contains("expand_gallery")) cfg.expand_gallery = nb::cast(d["expand_gallery"]); /// TRACES: GR-004 | SR-001 diff --git a/src/main.cpp b/src/main.cpp index a79d3c6..6303edc 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -36,7 +36,14 @@ // AR-011). Lowering it runs the model off-distribution. // --dense-scale downscale decoded frames in scene-detect mode (0 max faces kept per frame (default: 10) +// --max-faces max faces kept per frame (default: 0 = uncapped) +// --ownership-logodds belief needed to own a track (default: 2.0 ≈ P 0.88). +// Below it a track makes no presence claim at all. +// --evidence-rho-max ceiling on correlation between two observations of +// one track (default: 0.5 = a repeated view is worth +// at most two independent ones). AR-025. +// --evidence-admit-below

P(same view) under this counts as a new look +// --evidence-max-views distinct views remembered per track // --expand-gallery enable per-film gallery expansion from track continuity // --expand-buffer per-track diversity buffer size (default: 20) // --expand-band-lo

store admission floor, P(same person) (default: 0.90) @@ -166,6 +173,10 @@ static Config parse_args(int argc, char** argv) { else if (arg("--track-min-iou")) cfg.track_min_iou = std::stof(next()); else if (arg("--track-min-prob")) cfg.track_assoc_min_prob = std::stof(next()); else if (arg("--track-extinction")) cfg.track_extinction_sec = std::stod(next()); + else if (arg("--ownership-logodds")) cfg.ownership_logodds = std::stof(next()); + else if (arg("--evidence-rho-max")) cfg.evidence_rho_max = std::stof(next()); + else if (arg("--evidence-admit-below")) cfg.evidence_admit_below = std::stof(next()); + else if (arg("--evidence-max-views")) cfg.evidence_max_views = std::stoi(next()); else if (arg("--expand-gallery")) cfg.expand_gallery = true; else if (arg("--expand-buffer")) cfg.expand_buffer_size = std::stoi(next()); else if (arg("--expand-band-lo")) cfg.expand_band_lo = std::stof(next()); @@ -253,8 +264,17 @@ int main(int argc, char** argv) { auto same_person = same_person_probability(matcher_fn.calibration()); TrackRegistry::Config reg_cfg; reg_cfg.track_extinction_sec = cfg.track_extinction_sec; + reg_cfg.ownership_logodds = cfg.ownership_logodds; + /// TRACES: AR-025 | SR-002 + // The discounter's parameters come from Config now. They used to be + // in-class defaults reached through the one-argument constructor, so the + // VR-007 sweep that rho_max's own comment defers to could not vary it. + EvidenceDiscounter::Config disc_cfg; + disc_cfg.max_views = cfg.evidence_max_views; + disc_cfg.admit_below = cfg.evidence_admit_below; + disc_cfg.rho_max = cfg.evidence_rho_max; auto registry = std::make_shared( - reg_cfg, EvidenceDiscounter(same_person)); + reg_cfg, EvidenceDiscounter(same_person, disc_cfg)); matcher_fn.set_registry(registry); diff --git a/src/scene_preview.cpp b/src/scene_preview.cpp index d237be6..afe9ad3 100644 --- a/src/scene_preview.cpp +++ b/src/scene_preview.cpp @@ -157,8 +157,13 @@ int main(int argc, char** argv) { auto same_person = same_person_probability(matcher_fn.calibration()); TrackRegistry::Config reg_cfg; reg_cfg.track_extinction_sec = cfg.track_extinction_sec; + reg_cfg.ownership_logodds = cfg.ownership_logodds; + EvidenceDiscounter::Config disc_cfg; + disc_cfg.max_views = cfg.evidence_max_views; + disc_cfg.admit_below = cfg.evidence_admit_below; + disc_cfg.rho_max = cfg.evidence_rho_max; auto registry = std::make_shared( - reg_cfg, EvidenceDiscounter(same_person)); + reg_cfg, EvidenceDiscounter(same_person, disc_cfg)); matcher_fn.set_registry(registry); FaceTrackerFunc ftracker_fn{cfg, registry, same_person};