Skip to content

docs/model_scoreboard.md: every model in one table, generated from committed data - #141

Merged
jonfroehlich merged 9 commits into
mainfrom
docs/model-scoreboard
Aug 19, 2026
Merged

docs/model_scoreboard.md: every model in one table, generated from committed data#141
jonfroehlich merged 9 commits into
mainfrom
docs/model-scoreboard

Conversation

@jonfroehlich

@jonfroehlich jonfroehlich commented Aug 18, 2026

Copy link
Copy Markdown
Member

docs/model_comparison.md is the comprehensive log and should stay that way — but it
answers "which model is best, and by how much" across a dozen per-split tables in
chronological order. This adds the other view of the same data: rows are models, columns
are metrics
, plus five figures for the findings that read faster as a picture.

Generated, not hand-maintained

scripts/analysis/scoreboard.py re-scores every (leg, split) pair from the committed
bundles and benchmark/model_detections/, splices the tables into the doc between
HTML-comment markers, and --check fails when the committed page no longer matches the
committed data. No GPU, no credentials, no network, no .model_cache.

Registries are imported, never restated: splits from low_floor_sweep.US_SPLITS, models
from rampnet/roster.py. So budapest cannot be pooled here while held out there, and a leg
reaches this page by being registered rather than by someone remembering to add it.

Two defects it caught in its own first draft

RampNet's AP was truncated, and the table said so instead of fixing it. The city bundles
are a production run, so they stop at the deployed 0.55; an AP computed from them
integrates a curve cut off at the operating point. That read 0.720 pooled and put
RampNet below the YOLO arms' 0.730 — an artifact of the floor, not a result, and a
footnote is not the right answer to a wrong ordering. analysis_out/op_cache/ is the #54
re-extraction of the same panoramas down to 0.05, already committed. Read from it, RampNet's
pooled AP is 0.849 against the best YOLO arm's 0.734.

The substitution is gated on measured truncation (bundle floor >0.1 above the cache
floor), not a list of split names — manual_gold's bundle is already at 0.05, so it keeps
its own AP rather than silently trading a flip-TTA export for a no-TTA one.

The page asserted an operating point without showing the curve it sits on. Threshold
choice is this repo's question (#54/#55), so the answer belongs here. New figure, pooled
over the seven US splits: curves for every model with a calibrated score, single points for
the ones without, RampNet's deployed 0.55 and recommended 0.30 marked on its curve. Plus a
two-row table of RampNet at both — computed from the cache by a different code path than
low_floor_sweep, landing on that document's published pooled row to three decimals
(0.964 / 0.722 / 0.826 deployed, 0.900 raw precision at 0.30), and now asserted in a test so
a drift in either is a real disagreement rather than a silent one.

What pooling makes visible

  • RampNet 0.827 F1, +0.221 over the best challenger, and the top score in all ten splits.
  • The supervised YOLO baseline and the best zero-shot VLM are a dead heat (0.604 vs
    0.606) — the Train a supervised YOLO baseline (YOLO11 / YOLO26) on the RampNet dataset — isolate architecture vs. data #51 ablation in one line.
  • In-distribution the gap closes: YOLO11x's manual_gold AP (0.931) edges RampNet's
    (0.917) at the same floor, and RampNet's export had flip-TTA while YOLO's did not. The
    0.20 F1 it gives back out of domain is what the architecture buys.
  • Precision is not the differentiator; recall is. YOLO11x posts the board's best
    precision (0.969) at 0.416 recall against RampNet's 0.728.

Aggregation choices, stated beside the numbers

  • Macro-mean over the seven pooled US splits. budapest / sao_paulo / manual_gold keep their
    own columns and their HELD_OUT reasons — model_comparison.md says outright that
    budapest must not be averaged into a headline.
  • Operating points are per model class and inherited, not chosen here: RampNet 0.55, YOLO
    0.25 (Pre-register the YOLO-baseline evaluation & checkpoint-selection protocol #71), open-vocab at the 0.05 export floor, chat VLMs where they are.
  • The six single-split legs (2 Vistas, 4 Claude) are reported per split in their own table
    and kept out of the pooled headline and the three aggregate figures. A one-city mean
    beside a seven-city one is the confusion the coverage field exists to prevent.

Gaps recorded rather than left blank

gemini-3.1-pro and gemini-3.6-flash have no published manual_gold detections and none
in this workstation's .model_cache (probed by reconstructing the keys; the same probe hits
124/124 on richmond). The F1 0.568 / 0.540 quoted in the log are currently the only numbers
in the comparison a clean clone cannot re-derive.

Also here

It also regenerates docs/model_comparison.md for the #132 seam wrap

Not expected from a scoreboard PR, so stating it plainly: this branch rewrites the log's
per-split tables, 66 cells over six splits.
The cause is on main, not here. eccadda turned
on wrap_x in score_pano, so a detection at x=0.99 now matches ground truth at x=0.01
instead of being treated as ~1,010 px away — correct, and it changes measured results, but
model_comparison.md was never regenerated for it. Nothing on main re-derives those numbers,
which is why main stayed green: this page is the first artifact that recomputes them, so it
is the messenger rather than the cause.

19 (model, split) pairs each gain one genuine match. Every one is a chat VLM or open-vocab
detector; RampNet, the three YOLO arms, both Vistas arms and the four Claude legs are
unchanged to three decimals — which is why the effect was missed when #132 landed, since it
was checked against RampNet. Nothing reorders: RampNet's pooled F1 stays 0.827 and it is
still top on all ten splits; the best challenger moves 0.606 → 0.608 and the lead
0.221 → 0.219. Largest single move is gemini-3.1-pro on clovis, F1 0.503 → 0.514.

The tables are rewritten in place from the scorer, not by hand — P/R/F1/AP and tp/fp/fn,
markup and column widths preserved — and a note at the head of Results records the
regeneration, the mechanism, and which models are unaffected.

Review pass

A review of this PR found three defects in the plumbing, two in the numbers as written, and
a handful of doc drift; all are fixed in 77df9d7, and the findings are written up in full in
a comment on this PR. The three that mattered:

  • analysis_out/scoreboard.json was 7.76 MB, 98% of it unintended. 89eda8b added
    "curves" with the full recalls/precisions arrays and write_json dumps the dict, so
    OWLv2's 57,479 points and Grounding DINO's 63,344 went into the commit — against a comment
    and a .gitignore note that both said the arrays were excluded. json_payload now keeps
    only what the page cites. 7,761,091 → 72,706 bytes.
  • --check never checked the JSON, though its help and write_json's docstring both
    said it did; falsifying models[0].f1 passed with exit 0. It is now byte-compared, which
    also pins LF.
  • --models silently rewrote the committed page, then crashed — a one-row headline
    bolding YOLO11x as winner in every column, spliced above prose reading "RampNet wins by
    0.221 F1", followed by StopIteration. A subset run now leaves the committed artifacts
    alone unless the output flags name somewhere else.

Two consequences worth keeping: every number on the page is now asserted against
model_comparison.md (90 model×split pairs, ten splits), with RampNet's AP the one
deliberate exception, carried per row as ap_bundle and printed in a generated
AP-provenance table. And 02c5cd7 fixed something the new byte-compare caught immediately —
the committed JSON was not reproducible across environments, because AP comes out of
numpy and a different numpy build reorders the last bits of an accumulation. CI went 3.12
green / 3.10 red with every value in agreement. Floats are now rounded at the writer to six
decimals, 1,000× finer than anything the page reports.

Tests

tests/test_scoreboard.py, 44 tests: spot-checks the generated numbers against the values
committed in model_comparison.md, asserts budapest is never pooled, that pinned legs load
their own detections rather than a sibling's, that the AP ordering is not floor-dependent,
that the marked thresholds agree with operating_point.md, that a scoreless model never gets
a curve, that the committed JSON is current and carries no float longer than the writer
rounds to, that LF pinning holds on the bytes (splitlines() strips \r\n and \n alike,
so both report_usage and write_json passed with the CRLF bug), and that this page is not
stale. Suite: 1303 passed, 1 skipped.

Closes nothing on its own; feeds #20, #51, #122, #126.

🤖 Generated with Claude Code (claude-opus-5[1m])

jonfroehlich and others added 9 commits August 18, 2026 15:09
…itted data

docs/model_comparison.md is the comprehensive log and should stay that way, but
it answers "which model is best, and by how much" across a dozen per-split
tables in chronological order. This adds the other view of the same data — rows
are models, columns are metrics — plus four figures for the findings that read
faster as a picture.

Generated, not hand-maintained. scripts/analysis/scoreboard.py re-scores every
(leg, split) pair from the committed bundles and benchmark/model_detections/,
splices the tables into the doc between HTML-comment markers, and --check fails
when the committed page no longer matches the committed data. No GPU, no
credentials, no network, no .model_cache.

Registries are imported, never restated: splits from low_floor_sweep.US_SPLITS,
models from rampnet/roster.py. So budapest cannot be pooled here while held out
there, and a leg reaches this page by being registered rather than by someone
remembering to add it.

Aggregation choices, all stated in the doc beside the numbers:

- Macro-mean over the seven pooled US splits. budapest/sao_paulo/manual_gold
  keep their own columns and their HELD_OUT reasons — model_comparison.md says
  outright that budapest must not be averaged into a headline.
- Operating points are per model class and inherited, not chosen here: RampNet
  0.55, YOLO 0.25 (#71), open-vocab at the 0.05 export floor, chat VLMs where
  they are (no confidence to threshold).
- AP is read full-range, and RampNet's pooled AP is flagged as truncated at
  0.55 and NOT comparable to the arms exported at 0.05. Without that flag the
  table reads as YOLO11x beating it.
- The six single-split legs (2 Vistas, 4 Claude) are reported per split in
  their own table. A one-city mean beside a seven-city one is the confusion the
  coverage field exists to prevent, so they are kept out of the pooled headline
  and out of the three aggregate figures.

What pooling makes visible that the per-split tables did not:

- RampNet 0.827, +0.221 over the best challenger, and top score in all ten
  splits.
- The supervised YOLO baseline and the best zero-shot VLM are a dead heat
  (0.604 vs 0.606) — the #51 ablation in one line.
- In-distribution the gap closes: YOLO11x's manual_gold AP (0.931) edges
  RampNet's (0.917) at the same floor, and RampNet's export had flip-TTA while
  YOLO's did not. The 0.20 F1 it gives back out of domain is what the
  architecture buys.
- Precision is not the differentiator; recall is. YOLO11x posts the board's
  best precision (0.969) at 0.416 recall against RampNet's 0.728.

Gaps recorded rather than left blank: gemini-3.1-pro and gemini-3.6-flash have
no published manual_gold detections and none in this workstation's .model_cache
(probed by reconstructing the keys; the same probe hits 124/124 on richmond),
so the F1 0.568 / 0.540 quoted in the log are currently the only numbers in the
comparison that a clean clone cannot re-derive.

Also re-includes analysis_out/scoreboard.json in .gitignore — analysis_out/*
is ignored by default, and an uncommitted derived artifact is the trap this
repo has hit before. Written LF-only so a Windows re-run is a no-op diff.

tests/test_scoreboard.py (26 tests) spot-checks the generated numbers against
the values committed in model_comparison.md, asserts budapest is never pooled,
that pinned legs load their own detections rather than a sibling's, that every
roster provider has a class, and that this page is not stale.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Vistas arm (#126) and the YOLO baseline each added their own result table,
so the count in the opening paragraph was already wrong when it was written.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…hold is chosen on

Two related defects in the first cut, both from reading RampNet's detections
out of records.jsonl.

1. Its AP was truncated and the table said so instead of fixing it. The city
   bundles ARE a production run, so they stop at the deployed 0.55; an AP
   computed from them integrates a curve cut off at the operating point. That
   read 0.720 pooled and put RampNet BELOW the YOLO arms' 0.730 — an artifact
   of the floor, not a result, and a footnote is not the right answer to a
   wrong ordering. analysis_out/op_cache/ is the #54 re-extraction of the same
   panoramas down to 0.05, committed, 928 KB, all ten splits. Read from it,
   RampNet's pooled AP is 0.849 against the best YOLO arm's 0.734.

   Scoped to measured truncation (bundle floor >0.1 above the cache floor),
   not to a list of split names: manual_gold's bundle is already at 0.05, so it
   keeps its own AP. Substituting there would trade that split's flip-TTA
   export for a no-TTA one (0.917 -> 0.904), which is a different change and
   would leave one row's AP and its P/R/F1 describing two configs. P/R/F1 stay
   on records.jsonl everywhere — the published, deployment-faithful point.

2. The page asserted an operating point without showing the curve it sits on.
   Threshold choice is RampNet's question (#54/#55), so the answer belongs
   here. New figure, pooled over the seven US splits: curves for every model
   with a calibrated score, single points for the ones without, and RampNet's
   deployed 0.55 and recommended 0.30 marked on its curve. It makes three
   things visible that a column of F1 cannot — a scored model is a dial and a
   chat VLM is a dot; RampNet's curve dominates over the whole range rather
   than at one point; and the deployed threshold is not the F1 optimum.

   Plus a two-row table of RampNet at 0.55 vs 0.30. Computed from the cache by
   a different code path than low_floor_sweep, and it lands on that document's
   published pooled row to three decimals (0.964/0.722/0.826 deployed, 0.900
   raw precision at 0.30) — now asserted, so a drift in either is a real
   disagreement rather than a silent one.

The sub-0.55 half of the curve is a lower bound and says so: the GT was
assembled from detections at or above 0.55, and #55 measured that 27.2% of the
incremental FPs in [0.30, 0.55) are GT-completeness artifacts. So 0.849 is
conservative.

Also records why 0.55 is still what this page reports — OPERATIONAL_CONFIDENCE
is unchanged in the labeler, sidewalk-auto-labeler#20 is open, and its
2026-08-04 update found the world-space gain after multi-view fusion is +0.4 to
+3.2 rather than the per-panorama +7.4. That is a deployment question about
another repo's product metric; the per-panorama choice this benchmark measures
is settled.

5 new tests (31 total): the AP source per split, the substitution's scope, that
the AP ordering is not floor-dependent, that the marked points agree with
operating_point.md, and that a scoreless model never gets a curve.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two entries from probing claude-opus-5 on richmond (2026-08-18): 6 calls at
$0.0804 and 18 calls at $0.2321, $0.31 total. Committed because the log is
write-once — a cached re-run makes zero API calls and has no usage to report,
so spend not captured on the first pass is unrecoverable. That is how the four
original Claude legs' $28.82 ended up with no committed record.

They land on this branch because it is the one open; they belong to #139, which
has not started.

report_usage opened the ledger without newline="", so those two appends were
CRLF in an otherwise-LF file. Git normalized them away on commit, but that is
the checkout's autocrlf config doing the work, not the writer — the same defect
main just fixed for imagery_manifest (22dd536). Pinned at the writer so it does
not depend on how a contributor's git is configured.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"scoreboard.md" is ambiguous in a docs/ directory whose other results file is
model_comparison.md; the two now read as the pair they are — same benchmark,
one pivoted by split and one by model.

Recorded as a rename, so history follows the file. Every reference moves with
it: the pointer in model_comparison.md, DEFAULT_DOC, the .gitignore comment on
the committed JSON, the module docstrings, the test's staleness message, and the
footer text baked into two of the figures (regenerated — the old name was
rendered into those PNGs, where a stale path is invisible to grep; the other
three figures regenerated byte-identical, which is the determinism check for
free).

scoreboard.py / _render.py / _figures.py, analysis_out/scoreboard.json, and the
scoreboard_*.png figures keep their names: they are unambiguous where they live,
and renaming them would churn the gitignore re-include and every figure path for
no gain.

scoreboard.py --check green; suite 1269 passed.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…says

Review of #141. Three defects in the plumbing, the AP numbers reconciled with
model_comparison.md, and the doc drift the page's own contract is meant to stop.

**analysis_out/scoreboard.json was 7.76 MB, 98% of it unintended.** build()'s
comment says the curve point arrays are kept out of the JSON and .gitignore says
"~80 KB" -- both were true at 624a2da (68.9 KB). 89eda8b added "curves" to the
returned dict with full recalls/precisions and write_json dumps the dict, so
OWLv2 (57,479 points) and Grounding DINO (63,344) went straight into the commit.
scoreboard_render.json_payload now strips the arrays and keeps what the page
cites -- ap, marks, n_points. 7,761,091 -> 80,843 bytes.

**--check never checked the JSON**, though its help said "doc and JSON" and
write_json's docstring said "byte-compared by --check". Falsifying models[0].f1
in the committed file passed with exit 0. It is now byte-compared, which also
pins LF, and test_committed_json_is_current covers the same ground in-process.

**--models silently rewrote the committed page, then crashed.** --models
y11x_pano_h200 spliced a one-row headline -- YOLO11x bolded as winner in every
column -- above prose reading "RampNet wins by 0.221 F1", wrote the JSON and two
figures, then died on StopIteration in fig_headline. A subset run now leaves the
committed artifacts alone unless --doc/--json-out/--figure-dir name somewhere
else, --check refuses --models before scoring, and both figure helpers tolerate
a board with no RampNet row.

**Every number now provably corresponds to model_comparison.md.**
test_every_number_matches_model_comparison parses all ten of the log's per-split
tables and checks P, R, F1 and AP on every row -- 90 (model, split) pairs, all
ten splits, ten models. They already agreed everywhere except the two documented
gemini manual_gold gaps; the point is that they cannot stop agreeing silently.
The one deliberate difference, RampNet's AP, is now carried per row as
ap_bundle, asserted against the log, and printed in a generated AP-provenance
table on the page. model_comparison.md gained the matching note at its AP column.
test_only_rampnets_ap_is_allowed_to_differ_from_the_log scopes the exception.

**Two AP families were being crossed in the prose.** The table's AP is the
macro-mean; the PR figure's legend is micro-pooled. "0.849 to 0.734" paired
macro RampNet with micro YOLO11x (line 186), while line 208 quoted 0.844 against
a table saying 0.849. Both are now labelled, with the consistent pairs stated
(macro 0.849/0.730, micro 0.844/0.734), and the legend carries a
"AP, micro-pooled" title. The scoreless dots on that figure were macro points on
micro axes -- measured at up to 0.010 in P -- and are now micro too.

**The generalization claim cited only the three legs above the diagonal.** Four
of the seven zero-shot legs sit below it (Qwen-8B -0.05, GDINO -0.03, OWLv2
-0.02), all three labelled on the figure directly under the sentence. Restated
as the two-sided +-0.07 scatter it is, which is the stronger #58 result anyway.

Smaller:

- Reproduction said `pip install -r requirements-dev.txt  # numpy + pillow`;
  that file installs torch, timm, transformers, datasets and scikit-image for
  the rest of the suite. Verified in a fresh venv that numpy + pillow alone runs
  --check green, so the doc now says that.
- "four figures" in three places; there are five.
- scoreboard_figures' "one hue does all the work" predates CURVE_COLOR's four.
- The standing=False paragraph started plural and finished singular, reading as
  if the 10/10 cache verification covered all five legs rather than gemini-3.7.
- model_comparison.md's clovis paragraph quoted richmond as 0.960/0.765 against
  its own table's 0.964/0.768 four lines above.
- LF pinning had no regression test anywhere: read_text().splitlines() strips
  \r\n and \n alike, so both report_usage and write_json passed with the bug.
  Both now assert on bytes.
- One substitution gate (uses_low_floor_cache) for the AP column and the PR
  curve, which read it separately before; lru_cache on the op_cache reads (4x
  per split); DEPLOYED_THRESHOLD instead of a hardcoded 0.55 in the figure.
- Contour labels pinned to the right margin -- the auto-placer put "F1 0.6" on
  top of the YOLO markers -- and a halo behind scatter labels, which fixes
  RampNet's label crossing the diagonal without a nudge that cannot fit.

No scored number changed: the refactor reproduces the board exactly. Two full
regenerations are byte-identical on Windows, --check is green on both artifacts,
and the suite is 1282 passed / 1 skipped (13 new tests).

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI on #141 was red before my review commit, on 4a83366 too. The cause is not
this branch: CI tests the PR merged into main, and main has moved 9 commits
(#130/#132) since this branch forked.

eccadda turned on wrap_x in score_pano, so a detection at x=0.99 now matches
ground truth at x=0.01 instead of being treated as ~1,010 px away. That is
correct, and it changes measured results -- but docs/model_comparison.md was
never regenerated for it, so its committed tables have been stale on main ever
since. Nothing on main re-derives those numbers, which is why main is green:
this PR's scoreboard is the first artifact that recomputes them, so it is the
messenger rather than the cause.

Measured across the whole roster: 19 (model, split) pairs each gain one genuine
match, moving 66 cells over six splits. Every one is a chat VLM or open-vocab
detector. RampNet, the three YOLO arms, both Vistas arms and the four Claude
legs are unchanged to three decimals -- which is why the effect was missed when
#132 landed, since it was checked against RampNet.

Nothing reorders. RampNet's pooled F1 stays 0.827 and it is still top on all ten
splits; the best challenger moves 0.606 -> 0.608 and the lead 0.221 -> 0.219.
The largest single move is gemini-3.1-pro on clovis, F1 0.503 -> 0.514.

- model_comparison.md's tables are rewritten in place from the scorer, not by
  hand: P/R/F1/AP and tp/fp/fn, markup and column widths preserved. The AP
  column keeps its documented meaning (the bundle AP) on every row.
- A note at the head of Results records the regeneration, the mechanism, and
  which models are unaffected.
- Three derived prose claims that rode on moved numbers: clovis's lead ~0.30 ->
  ~0.29 (twice), and morgantown's "Gemini-3.1-pro's 0.639 and Molmo's 0.460 are
  their highest anywhere" -> 0.643/0.463, with the "highest anywhere" claim
  corrected too: paterson beat both (0.681/0.511) two splits later, so that had
  been wrong since paterson regardless of the seam.
- score_pano's docstring claimed "wrapping moves no metric on any committed
  split". True for RampNet and YOLO, false for the challengers; rewritten to say
  which, and to point at the test that now enforces it.
- The hardcoded PUBLISHED spot-check in test_scoreboard.py went stale exactly as
  a hand-copy does. Updated, and relabelled as a regression pin rather than the
  contract -- test_every_number_matches_model_comparison reads the log itself.
- model_scoreboard.md prose: the lead, Gemini 3.1 Pro's pooled F1, the challenger
  spread floor, the Qwen pair, and the Vistas gap.

Suite 1302 passed / 1 skipped. Two consecutive regenerations byte-identical;
scoreboard.py --check green on both artifacts.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ucible

CI caught this on the previous commit: 3.12 green, 3.10 red, on
test_committed_json_is_current alone. Every value agreed -- the doc's tables,
which are rounded to three decimals, passed on both. What differed was the file's
bytes: AP comes out of numpy, a different numpy build reorders the last bits of
an accumulation, and repr() changes with them.

So the committed JSON was not reproducible across environments, and the check
added one commit ago to prove it was reproducible is what found that out. The
fix belongs at the writer, not in a looser assertion: seventeen significant
digits of accumulation noise were never meaningful in a file whose whole purpose
is to be diffed by a reviewer. json_payload now rounds every float to six
decimals -- 1,000x finer than anything the page reports, coarse enough to be
identical everywhere.

A byte-compare is still the right check, and it now means something portable.
test_the_json_is_reproducible_across_environments asserts no float on disk
carries more digits than the writer rounds to, so a hand-edit or an older writer
is caught by something other than a platform-dependent diff.

Also drops the file 80,843 -> 72,706 bytes.

Suite 1303 passed / 1 skipped.

🤖 Generated with [Claude Code](https://claude.com/claude-code) (claude-opus-5[1m])

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

Review findings

Deep review of this PR. Everything below is fixed on the branch — the fixes are in
77df9d7, e6f9df5 and 02c5cd7. No scored number changed as a result of the refactor: the
board reproduces exactly, and two full regenerations are byte-identical on Windows.

Plumbing — three defects that made the page's own contract untrue

1. analysis_out/scoreboard.json was 7.76 MB, 98% of it unintended. build()'s comment
says the curve point arrays are kept out of the JSON, and .gitignore says "~80 KB". Both
were true at 624a2da (68.9 KB). 89eda8b added "curves" to the returned dict with full
recalls/precisions, and write_json dumps the dict — so OWLv2 (57,479 points) and
Grounding DINO (63,344) went straight into the commit. scoreboard_render.json_payload now
strips the arrays and keeps what the page cites (ap, marks, n_points).
7,761,091 → 80,843 bytes.

2. --check never checked the JSON, though its help said "doc and JSON" and
write_json's docstring said "byte-compared by --check". Falsifying models[0].f1 in the
committed file passed with exit 0. It is now byte-compared — which also pins LF — and
test_committed_json_is_current covers the same ground in-process.

3. --models silently rewrote the committed page, then crashed.
--models y11x_pano_h200 spliced a one-row headline — YOLO11x bolded as winner in every
column — above prose reading "RampNet wins by 0.221 F1", wrote the JSON and two figures,
then died on StopIteration in fig_headline. A subset run now leaves the committed
artifacts alone unless --doc / --json-out / --figure-dir name somewhere else, --check
refuses --models before scoring, and both figure helpers tolerate a board with no RampNet
row.

Numbers

4. Nothing tied the page's numbers to model_comparison.md.
test_every_number_matches_model_comparison now parses all ten of the log's per-split tables
and checks P, R, F1 and AP on every row — 90 (model, split) pairs, ten splits, ten models.
They already agreed everywhere except the two documented gemini manual_gold gaps; the point
is that they cannot stop agreeing silently. The one deliberate difference, RampNet's AP, is
carried per row as ap_bundle, asserted against the log, and printed in a generated
AP-provenance table on the page; model_comparison.md gained the matching note at its AP
column. test_only_rampnets_ap_is_allowed_to_differ_from_the_log scopes the exception.

5. Two AP families were being crossed in the prose. The table's AP is the macro-mean; the
PR figure's legend is micro-pooled. "0.849 to 0.734" paired macro RampNet with micro YOLO11x
(line 186), while line 208 quoted 0.844 against a table saying 0.849. Both are now labelled,
with the consistent pairs stated (macro 0.849/0.730, micro 0.844/0.734), and the legend
carries an "AP, micro-pooled" title. The scoreless dots on that figure were macro points on
micro axes — measured at up to 0.010 in P — and are now micro too.

6. The generalization claim cited only the three legs above the diagonal. Four of the
seven zero-shot legs sit below it (Qwen-8B −0.05, GDINO −0.03, OWLv2 −0.02), all three now
labelled on the figure directly under the sentence. Restated as the two-sided ±0.07 scatter
it is, which is the stronger #58 result anyway.

Smaller

  • Reproduction said pip install -r requirements-dev.txt # numpy + pillow; that file
    installs torch, timm, transformers, datasets and scikit-image for the rest of the suite.
    Verified in a fresh venv that numpy + pillow alone runs --check green, so the doc now
    says that.
  • "four figures" in three places; there are five.
  • scoreboard_figures' "one hue does all the work" predates CURVE_COLOR's four.
  • The standing=False paragraph started plural and finished singular, reading as if the
    10/10 cache verification covered all five legs rather than gemini-3.7.
  • model_comparison.md's clovis paragraph quoted richmond as 0.960/0.765 against its own
    table's 0.964/0.768 four lines above.
  • LF pinning had no regression test anywhere: read_text().splitlines() strips \r\n
    and \n alike, so both report_usage and write_json passed with the bug. Both now
    assert on bytes.
  • One substitution gate (uses_low_floor_cache) for the AP column and the PR curve, which
    read it separately before; lru_cache on the op_cache reads (4× per split);
    DEPLOYED_THRESHOLD instead of a hardcoded 0.55 in the figure.
  • Contour labels pinned to the right margin — the auto-placer put "F1 0.6" on top of the
    YOLO markers — and a halo behind the scatter labels, which fixes RampNet's label crossing
    the diagonal without a nudge that cannot fit.

Two more found after the review, worth recording separately

model_comparison.md has been stale on main since the #132 seam wrap (e6f9df5). CI on
this PR was red before the review commit, and on 4a83366 too — CI tests the PR merged into
main, and main had moved 9 commits (#130/#132). eccadda turned on wrap_x in score_pano,
so a detection at x=0.99 now matches ground truth at x=0.01 instead of being treated as
~1,010 px away. That is correct, and it changes measured results — but the log was never
regenerated for it. Nothing on main re-derives those numbers, which is why main is green:
this PR's scoreboard is the first artifact that recomputes them, so it is the messenger
rather than the cause. Measured across the whole roster: 19 (model, split) pairs each gain
one genuine match, moving 66 cells over six splits
, every one a chat VLM or open-vocab
detector. RampNet, the three YOLO arms, both Vistas arms and the four Claude legs are
unchanged to three decimals — which is why this was missed when #132 landed, since it was
checked against RampNet. Nothing reorders: RampNet's pooled F1 stays 0.827 and it is still
top on all ten splits; the best challenger moves 0.606 → 0.608 and the lead 0.221 → 0.219.
The log's tables are rewritten in place from the scorer, not by hand.

The committed JSON was not reproducible across environments (02c5cd7) — and the
byte-compare added one commit earlier is what found that out. CI went 3.12 green / 3.10 red
on test_committed_json_is_current alone. Every value agreed; the doc's tables, rounded to
three decimals, passed on both. What differed was the file's bytes: AP comes out of numpy, a
different numpy build reorders the last bits of an accumulation, and repr() changes with
them. Fixed at the writer rather than by loosening the assertion — json_payload rounds
every float to six decimals, 1,000× finer than anything the page reports and coarse enough to
be identical everywhere. test_the_json_is_reproducible_across_environments asserts no float
on disk carries more digits than the writer rounds to, so a hand-edit or an older writer is
caught by something other than a platform-dependent diff. Also drops the file
80,843 → 72,706 bytes.

State

Suite 1303 passed / 1 skipped, --check green on both artifacts, CI green on 3.10 and
3.12, main merged, mergeable.

🤖 Generated with Claude Code (claude-opus-5[1m])

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.

2 participants