Skip to content

Make the seed settable, pre-register its reading, and launch both seed-variance campaigns (#51, #135) - #155

Merged
jonfroehlich merged 5 commits into
mainfrom
exp/seed-variance-51-135
Sep 4, 2026
Merged

Make the seed settable, pre-register its reading, and launch both seed-variance campaigns (#51, #135)#155
jonfroehlich merged 5 commits into
mainfrom
exp/seed-variance-51-135

Conversation

@jonfroehlich

@jonfroehlich jonfroehlich commented Sep 3, 2026

Copy link
Copy Markdown
Member

Makes the training seed settable in both trainers, pre-registers how its variance will be read, and launches the two campaigns that measure it. Campaign A has been running since 2026-09-03; Campaign B was relaunched 2026-09-04 after its first three jobs died one second after submission on a bash parse error (see the review comments below). Job IDs below.

Why now

#51 and #135 hit the same wall from opposite sides.

Neither trainer could vary the seed at all: train.py hardcoded manual_seed(42) with no flag, and run_yolo_train_tillicum.slurm never passed one to Ultralytics.

Everything downstream inherits this. With no noise floor, no RampNet 2.0 improvement can be called real either.

The half that was easy to get wrong

Stage 2 has two sources of run-to-run randomness and only one was ever set. DistributedSampler carries its own seed (default 0) and derives each epoch's permutation from seed + epoch inside set_epoch(), independent of torch.manual_seed. A sweep that moved only the torch seed would reuse one data order across every replicate — understating the spread, and doing it silently, since no log line distinguishes the two cases.

So both seeds move together, with one exception that has to be exact: at the historical seed 42 the sampler must stay at its historical 0, or the default stops reproducing published runs. That pairing is rampnet/seeding.py::sampler_seed_for, tested rather than commented.

Why the klone launcher is a new file

train.py writes best_model.pth and latest_checkpoint.pth to the current directory, not to --checkpoint-dir. Three replicates launched from one directory would overwrite each other's best model and — worse — each other's resume state: a resume file from another seed loads silently as if it were this run's own, converging the arms onto one lineage with nothing in the log to say so. run_train_seed.slurm gives each replicate its own RUNDIR and cds into it. run_train.slurm stays untouched as the preserved record of the published run and #135's rungs, the same reasoning that kept the Tillicum YOLO launcher separate from its klone original.

Pre-registered, before any replicate finished

docs/seed_variance_51_135.md fixes the decision rule on the sample SD s of the three Campaign A replicates:

s reading
≤ 0.010 0.039 is ≈4σ — the architecture advantage is small but real
≥ 0.020 0.039 is inside 2σ — #51 closes with "the supervised baseline is statistically indistinguishable from RampNet at matched operating points"

Superseded by Amendment 1 (2026-09-04), ratified by Jon the same day, before any replicate was scored. The rule above divides 0.039 by Campaign A's SD alone, but 0.039 is a difference of two single-seed runs, so its standard error is sqrt(s_A² + s_B²). At s_A = s_B = 0.010 it is 2.8σ, not ≈4σ; at s_B = 0.020 it is 1.7σ, inside the band this same table calls indistinguishable. Campaign B also had no reading pre-registered at all. Both are corrected in docs/seed_variance_51_135.md under Amendment 1, with this original kept verbatim so the change is auditable. (#154 also revises 0.039 to 0.041 bundle-equivalent, which does not change the direction.)
| 0.010–0.020 | ambiguous; report the interval, claim neither, combine with Campaign B |

The second branch makes our own headline smaller and is accepted in advance.

What is running

Campaign A — YOLO seed variance, Tillicum. Three replicates of y11x_tiles at seeds 1/2/3, config identical to the as-run args.yaml in every respect except the seed. Jobs 274367 (running, verified seed=1 in its log header), 274369, 274370, each with CHAIN=5 to cover the 24 h normal-QoS ceiling. ~3.0 h/epoch on one H200; ~$21.60 per 24 h link, ≤$389 for all three, against a $1,500/month cap with $0 used this cycle.

Tillicum was chosen over free klone for a measured reason: the tiles arm consumes 8.5 MB/s there against a filesystem measured at 8.3–11.8 MB/s — it sits on the ceiling — so three concurrent replicates would contend for the wall itself. On Tillicum the same arm uses 4% of available bandwidth and is genuinely GPU-bound.

Campaign B — RampNet seed variance, klone. Three replicates of the committed recipe (1 epoch / 9,378 steps, constant lr 1e-5, global batch 16) at seeds 1/2/3. Jobs 39583887/39583890/39583891 on ckpt-all (relaunched 2026-09-04; the original 39515025/26/27 died at submit time on the apostrophe parse bug fixed in 2b7471c), free and preemptable, resumed by --requeue plus train.py's own latest_checkpoint.pth. Calendar, not money, is the risk: ckpt-all's duty cycle was 3.9% in 2026-08.

One deliberate deviation: save_period=1

The #51 arms ran save_period: -1, which is exactly why the epoch-curve follow-up had to be retracted — no per-epoch weights exist for any arm and they cannot be recovered. Keeping every epoch costs ~20 GB against a 1 TB allocation and buys back an analysis that is currently foreclosed.

Tests

tests/test_seeding.py (21 tests) checks the plumbing at the source level, because every failure mode here is silent:

  • the default still maps to sampler seed 0, so reproductions are unchanged
  • all three RNGs follow the flag, and no 42 literal survives
  • the launcher refuses to default SEED — an unset replicate would be a silent duplicate of the published run
  • per-seed RUNDIR isolation, and no checkpoints under klone's 10 GB home quota
  • the Tillicum heredoc's positional list still lines up with its sys.argv unpack — an off-by-one there shifts imgsz into epochs and trains a wrong model that finishes green

Full suite: 1,343 passed, 1 skipped. --seed verified end to end in the real klone sidewalkcv2 environment, and seed=1 confirmed in job 274367's Ultralytics arg dump.

Stated limitations

  • n=3 gives a wide interval on the SD itself (95% CI on σ spans ~0.5σ̂–3.7σ̂). A 4th and 5th replicate are ~$119 each and are the pre-registered response if the result lands in the ambiguous band.
  • Campaign A's replicates are Tillicum H200; the existing seed: 0 arm is klone L40S. The SD is over the three same-hardware replicates only; the seed-0 arm is a separate cross-hardware check, not a fourth sample.
  • Only y11x_tiles is replicated — the measured SD is not automatically the other arms'.
  • Seed variance is not the same as training-run variance; Campaign B has no deterministic=True guarantee, so its spread is an upper bound on seed effect alone.

Related: #154 (the parity result this exists to adjudicate), #51, #135.

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

…ad to mean (#51, #135)

Both issues are blocked on the same missing number. #51's matched-operating-point read
put the RampNet-vs-YOLO residual at 0.039 F1, and #51's own rule is that differences
under ~0.02 should not be read -- but every arm in that comparison is seed 0, the
ultralytics default, in all eight args.yaml. #135's power analysis reached the same wall
from the other side and said so outright: the binding limit is unmeasured seed variance,
n=1. Nothing downstream can be called real without it, RampNet 2.0 included.

Neither trainer could vary the seed at all. train.py hardcoded manual_seed(42) with no
flag; run_yolo_train_tillicum.slurm never passed one to ultralytics.

THE HALF THAT WAS EASY TO GET WRONG. Stage 2 has TWO sources of run-to-run randomness,
and only one was ever set. DistributedSampler carries its own seed (default 0) and
derives each epoch's permutation from seed + epoch in set_epoch(), independent of
torch.manual_seed. A sweep that moved only the torch seed would reuse one data order
across every replicate, understating the spread -- and would do it silently, since no
log line distinguishes the two cases.

So both seeds move together, with one exact exception: at the historical seed 42 the
sampler must stay at its historical 0, or the DEFAULT stops reproducing published runs.
That pairing is rampnet/seeding.py::sampler_seed_for, tested rather than commented.

Also load-bearing, and the reason the klone launcher is a new file rather than a flag on
run_train.slurm: train.py writes best_model.pth and latest_checkpoint.pth to the CURRENT
DIRECTORY, not to --checkpoint-dir. Three replicates launched from one directory would
overwrite each other's best model and, worse, each other's resume state -- a resume file
from another seed loads silently as if it were this run's own, converging the arms onto
one lineage with nothing in the log to say so. run_train_seed.slurm gives each replicate
its own RUNDIR and cd's into it.

PRE-REGISTERED, before any replicate finished: docs/seed_variance_51_135.md fixes the
decision rule. SD <= 0.010 and the architecture advantage is real at ~4 sigma; SD >= 0.020
and #51 closes with "the supervised baseline is statistically indistinguishable from
RampNet at matched operating points"; between is ambiguous and needs both campaigns. The
second branch makes our own headline smaller and is accepted in advance.

One deliberate deviation from the #51 protocol: save_period=1. The arms ran -1, which is
exactly why the epoch-curve follow-up had to be retracted -- no per-epoch weights exist
anywhere and cannot be recovered. ~20 GB against 1 TB buys back that analysis.

tests/test_seeding.py (21) checks the plumbing at the source level, because every failure
here is silent: the default still mapping to sampler seed 0, all three RNGs following the
flag, the launcher refusing to default SEED, per-seed RUNDIR isolation, and the Tillicum
heredoc's positional list still lining up with its unpack -- an off-by-one there shifts
imgsz into epochs and trains a wrong model that finishes green.

Full suite: 1,343 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jonfroehlich added a commit that referenced this pull request Sep 3, 2026
… B is decided against (#135)

#135 exists to decide whether to spend 1,675-3,350 GPU-hours on the annealed arm. Every
input to that decision has been in place since 2026-08-29 -- the rung completed, the LR
schedule verified over 100% of the run, manual_gold scored and tied at every epoch -- but
the gate written into the pre-registration was never actually applied to the numbers, so
the issue stayed open with the decision unmade.

scripts/analysis/run_b_gate_135.py applies it from the committed summaries rather than
restating it in prose, and writes docs/data/run_b_gate_135.json:

  PRIMARY   max-F1(cosine ep8) - max-F1(Run A ep8) = +0.002994
            |z| = 1.02 to 1.86        -> not significant
  SECONDARY Run A ep3->ep8 -0.006624 ; cosine -0.005158, arrested? NO
            difference of declines +0.001466, |z| = 0.50 to 0.91 -> not significant
  VERDICT   JUDGMENT CALL

The primary misses significance AT THE FAVOURABLE END of the measured s.e. bracket, so the
reading does not depend on which value inside it is chosen. That robustness is the reason
the bracket is honest enough to decide on: a true paired bootstrap between the two arms
needs both arms' per-pano detections and the cosine arm's are not committed, so this uses
the s.e. #138 measured across 28 Run A epoch pairs on the same panos and GT. Stated in the
script's own docstring, not buried.

The pre-registration says a tie on both is explicitly NOT an automatic cancellation, so the
decision is recorded as the judgment it is. Not running Run B, because:

- The mechanism that justified it is the one that failed. Run B's case was #51's annealed
  tail; the rung tested exactly that at matched budget, seed and data order and moved
  manual_gold by nothing measurable. A 30-epoch arm changes length AND schedule, so it
  could not attribute a difference even if it found one.
- The gain is real and in the wrong place: up to 3.98% better auto-label val loss, none of
  it reaching human F1. That is #84's exchange rate replicating.
- THE ARGUMENT THAT WAS NOT AVAILABLE WHEN RUN B WAS SPECIFIED: Run B is n=1, and this
  issue itself established seed variance as the binding limit. The plausible effect (~0.003
  of late-epoch damping) sits below the ~0.01 that is "measured but not attributable"
  without a seed control, so a single 30-epoch run is uninterpretable at any length. A
  readable Run B is 3 seeds = 5,025-10,050 GPU-h, ~$4,500-9,000.
- Opportunity cost, measured: #151 just produced a +0.115 F1 rig effect on the same model.

What is NOT claimed is written down too: not that annealing does nothing (the ~0.003 damping
is unresolved, not refuted), not that a 30-epoch run would fail (it was never run), and not
epoch 7's +0.0042 -- the largest gap anywhere, which WOULD clear 1.96 at the favourable end
of the bracket but is not the pre-registered comparison. The artifact flags it so nobody
quotes it as the result.

Reopening condition is concrete and already in flight: the seed campaign (PR #155, klone
39515025/26/27) prices it directly. Seed SD <= ~0.002 max-F1 and a 0.003-0.008 effect
becomes readable at n=1.

Two amendments so the repo does not carry contradictory advice:
- stage2_run_b_power_135.md's "run the 30-epoch arm" recommendation predates the rung's
  results; marked SUPERSEDED in place rather than rewritten.
- stage2_epoch_curve_84.md's "the gate does not cancel Run B" section gets the later
  outcome inline, since that doc is where a reader looks for Run A/Run B.

Also states a deliberate omission: #135 asked for the benchmark splits as well as
manual_gold, and only manual_gold was scored -- because #138 measured the nine city splits
pooled at an unpaired MDE of 0.0219 against manual_gold's 0.0117, so they cannot resolve
what manual_gold cannot.

tests/test_run_b_gate_135.py (10) covers the branches that did NOT fire, since an
implementation that only ever emits the observed verdict is untestable by its own output.

Full suite: 1,371 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ncher in CI (#51, #135)

Jobs 39515025, 39515026 and 39515027 were submitted on 2026-09-03 at 15:01 and were all
dead one second later, FAILED with exit 2. The stdout files are zero bytes; the whole
record of the failure is 99 bytes of stderr:

    slurm_script: line 46: unexpected EOF while looking for matching `}'

Line 46 was the required-SEED guard, whose message read "the replicate's seed". Inside
${VAR:?...} bash parses the word for quoting even within double quotes, so the lone
apostrophe opened a quote and the closing brace was never found. Removing it is the
entire fix; the guard still aborts on an unset SEED, which is tested rather than assumed.

WHAT IT COST. Nothing in dollars -- klone ckpt is free -- and everything in calendar.
The free half of the campaign produced no GPU-seconds for 27 hours while the paid
Tillicum half ran normally beside it, so the asymmetry pointed the wrong way: the arm
that measures RampNet's own seed spread, which #135 named as the binding limit, is the
one that had not started. /gscratch/scrubbed/jfroehli/seedvar/ did not exist.

WHY NOTHING CAUGHT IT. Nothing in the suite reads these files as shell. test_seeding.py
asserts on their content with regexes, which a syntactically broken script passes
happily, and a launcher is the one artifact whose failure is invisible locally -- you
learn about it on the cluster, hours later, from an empty log. tests/test_slurm_scripts.py
now runs bash -n over every tracked .slurm (23 of them, ~1.5 s, no cluster and no
network) and fails on exactly this. Verified by reintroducing the apostrophe: two tests
fail, and they name the file.

The glob prunes dot-directories deliberately -- .claude/worktrees/ holds whole nested
checkouts, and collecting their launchers would scale this test with however many
branches happen to be on disk.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich

Copy link
Copy Markdown
Member Author

Campaign B never started: all three klone replicates died one second after launch

Checked the cluster 2026-09-04 ~19:30 UTC, 28 hours after launch. Campaign A (Tillicum, paid) is healthy. Campaign B (klone, free) produced nothing.

job state elapsed start
39515025 FAILED (exit 2:0) 00:00:01 2026-09-03T15:01:54
39515026 FAILED (exit 2:0) 00:00:01 2026-09-03T15:02:50
39515027 FAILED (exit 2:0) 00:00:01 2026-09-03T15:02:50

The stdout files are zero bytes. The entire record is 99 bytes of stderr:

slurm_script: line 46: unexpected EOF while looking for matching `}'

Line 46 was the required-SEED guard, and the message read the replicate's seed. Inside ${VAR:?...} bash parses the word for quoting even within double quotes, so the lone apostrophe opened a quote and the closing brace was never found. /gscratch/scrubbed/jfroehli/seedvar/ did not exist — no RUNDIR was ever created.

Fixed in 2b7471c: the apostrophe is gone, the guard still aborts on an unset SEED (tested, not assumed), and tests/test_slurm_scripts.py now runs bash -n over all 23 tracked .slurm files in ~1.5 s with no cluster and no network. Verified by reintroducing the apostrophe — two tests fail and they name the file. test_seeding.py could not have caught this: it asserts on the launcher's content with regexes, which a syntactically broken script passes happily.

Resubmitted from the updated checkout at seeds 1/2/3: 39583887, 39583890, 39583891, all PENDING (Resources) as of this comment. Note Slurm capped --time to 9:05:00 rather than the requested 24 h, so --requeue plus latest_checkpoint.pth resume is now load-bearing on ckpt-all rather than merely insurance.

Campaign A is fine, and its TIMEOUTs are by design

274367/274369/274370 each hit the 24 h normal ceiling exactly as the CHAIN mechanism anticipates; the afterany links picked them up without intervention. Links 2 (274368/274372/274373) were 3 h 17 m in, links 3 (276163–65) queued. Seeds 1/2/3 on y11x_tiles, yolo11x.pt, imgsz 1024, batch 12, 60 epochs — matching the pre-registration. Spend to that point ≈ 82 GPU-h ≈ $74, derived from elapsed × 1 GPU × $0.90, against the ≤$389 ceiling.

What this cost, and the asymmetry worth noting

Nothing in dollars — klone ckpt is free — and 28 hours of calendar. The pointed part is which half stalled: the arm measuring RampNet's own seed spread, which #135 named as the binding limit, is the one that had not started, while the paid YOLO half ran normally beside it. squeue showed an empty queue for a day and that is indistinguishable from "finished". As in #135: only sacct -D tells you which.

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

@jonfroehlich

Copy link
Copy Markdown
Member Author

Review of the whole diff (first review of this PR)

The plumbing is right where it counts: both RNG sources move together, the historical
pairing (42, 0) is preserved by a named function rather than a magic number, the
per-seed RUNDIR correctly closes the best_model.pth/latest_checkpoint.pth
cross-contamination hole, and the Tillicum positional list does line up with its unpack
(13 passed, sys.argv[1:14]). The 2b7471c fix is correct and bash -n over every
launcher is the right generalisation of it.

What I do not think holds up is the guarding and the reading. Two of the three
assertions this PR points to as its safety net are vacuous — I verified both by
mutation, and in each case the mutation is the exact silent regression the docstring
names. And the pre-registered decision rule divides 0.039 by the wrong standard error
and leaves Campaign B with no reading at all, so as written half the campaign produces
a number whose interpretation is still free after the fact.

Nothing here argues for touching the in-flight jobs.


High

H1. tests/test_seeding.py:77-88 — the test guarding the campaign's central invariant is vacuous.
test_train_sampler_seed_is_derived_not_hardcoded asserts "sampler_seed_for(args.seed)" in src
against the whole file, and stage_two/train.py:305 already contains that string inside a
print(). So the assertion is satisfied by the log line alone.

Verified: I deleted seed=sampler_seed_for(args.seed) from the
DistributedSampler(train_dataset, ...) call at stage_two/train.py:282-283, leaving
everything else intact — 21/21 tests still pass. That regression is precisely the failure
rampnet/seeding.py:17-21 exists to prevent: all three replicates would share one data order,
the campaign would measure initialization variance only, s would come out too small, and the
first branch of the decision table would fire and publish "the architecture advantage is small
but real". Nothing in any log distinguishes the two cases.
(sampler_line on line 81 is also computed and never used.)

H2. docs/seed_variance_51_135.md:87-93 — the decision rule divides 0.039 by the wrong SD.
The rule is stated "on the sample SD s of the three Campaign A replicates", and the bands
read 0.039/s. But 0.039 is a difference between two single-seed runs — RampNet 0.843 minus
y11x_tiles 0.804 (PR #154's parity table). The standard error of that difference is
sqrt(s_A^2 + s_B^2), not s_A.

Concretely: at s_A = 0.010 the table asserts "0.039 is approx 4 sigma". If Campaign B
returns s_B = 0.010, the gap is 2.8 sigma; at s_B = 0.015 it is 2.2; at s_B = 0.020 it is
1.7 — i.e. inside the band the table calls "statistically indistinguishable". So the published
conclusion can invert while every input to the stated rule is unchanged. This is the same
assumption (the RampNet side is noiseless, n=1 is fine) that #135 said was unjustified, and
Campaign B is measuring s_B concurrently but the rule only invokes it in the middle band.

H3. docs/seed_variance_51_135.md:73-96 — Campaign B has no pre-registered reading.
"The reading, fixed in advance" gives bands for #51 only. Nothing states what s_B will be
read to mean, what closes #135, or how s_B relates to #135's measured paired MDE of 0.0063.
A pre-registration that leaves half its campaign uninterpreted lets that half's interpretation
be chosen after the numbers land — the thing the Status banner at the top claims to have
prevented. It also makes the middle branch unexecutable, since "combine with Campaign B's SD
for a two-sample test" names neither the test nor its threshold.


Medium

M4. stage_two/run_train_seed.slurm:57 — the campaign's only artifact lands on purge-scheduled storage.
RUNDIR defaults under /gscratch/scrubbed, and because train.py writes to the CWD that is
where best_model.pth and latest_checkpoint.pth go. docs/stage2_epoch_curve_84.md:119
records that /gscratch/scrubbed purges on a ~21-day idle window, which is exactly why #84 put
its checkpoints on /gscratch/makelab (purchased, never purged). This doc simultaneously says
(lines 110-113) that Campaign B's calendar is unbounded at a 3.9% duty cycle. Failure scenario:
a replicate finally runs, writes best_model.pth, then sits unscored past the purge window
because the other two are still pending — the deliverable is gone and nothing warned.
This one needs Jon's decision: changing the default would put a manual resubmission in a
different place from its already-queued siblings, so I have documented the required copy-out
rather than changed it.

M1. docs/seed_variance_51_135.md:12 and scripts/model_comparison/run_yolo_train_tillicum.slurm:92 — dead link to the source of the headline number.
docs/operating_point_parity_51.md, scripts/analysis/operating_point_parity_51.py and
docs/data/operating_point_parity_51.json exist only on fix/yolo-label-cache-rescue-51
(PR #154), which is open and based on main — it is not a parent of this branch. If #155
merges first, main carries a pre-registration whose 0.039 and whose scoring tool cannot be
found from a clean clone. 0.039 appears nowhere else in the repo.

M2. docs/seed_variance_51_135.md:119-132 — "Reproducing" reproduces the training, not the reading.
Neither campaign has a written path from the artifact it produces to the pre-registered
statistic. For A, operating_point_parity_51.py reads a fixed leg list out of
docs/data/yolo_geometry_51/*.txt, so three new replicates need sweep dumps plus a change to
that script. For B, a fresh best_model.pth needs a new analysis_out/op_cache/ over eight
splits before any threshold can be selected. Standing rule: the run instructions live in the
repo as exact commands in order.

M3. docs/seed_variance_51_135.md:125-128 — the Campaign A reproduce command omits PYTHON=.
run_yolo_train_tillicum.slurm:155 defaults PYTHON=python, which on Tillicum has no
ultralytics — the as-run invocation in docs/tillicum.md:499 sets
PYTHON=/gpfs/projects/makelab/$USER/envs/rampnet-yolo/bin/python. Someone following the
committed block gets a ModuleNotFoundError rather than a replicate.

M5. tests/test_seeding.py:136-154 — the positional-argument test is blind to braced variables.
n_passed counts "\$[A-Z_]+" only. Verified: inserting "${LR0}" between "$YOLO_CKPT" and
"$YOLO_DATA" — which shifts data to imgsz to epochs and so on, verbatim the failure the
docstring names — leaves the suite green.

M6. stage_two/run_train_seed.slurm:63 — no interpreter escape hatch.
source activate sidewalkcv2 is hardcoded. Every other klone launcher here either takes a
PYTHON= override (run_yolo_train.slurm:71-76, run_gold_bundle.slurm:47-51) or an env
prefix (run_train_epoch_curve.slurm:60-75, whose header says calling the env's torchrun by
absolute path is "the pattern already proven on this cluster"). #84's env was built at a
prefix/gscratch/scrubbed/jfroehli/envs/sidewalkcv2, docs/stage2_epoch_curve_84.md:140
— which source activate <name> cannot resolve. Under set -e the job then dies at line 63
with no other diagnosis, the same shape as the failure 2b7471c just fixed.

M7. rampnet/seeding.py:41-43sampler_seed_for is not injective.
sampler_seed_for(0) == sampler_seed_for(42) == 0. A later sweep that includes seed 0 — the
natural first choice, and the Ultralytics default every #51 YOLO arm used — silently reuses the
published run's data order, and test_every_other_seed_maps_to_itself parametrizes 0 and
asserts that as correct. Harmless for seeds 1-3; undefended and silent for anyone extending.


Low

L1. docs/seed_variance_51_135.md:91-93 — the decision bands overlap at their endpoints.
The three rows all contain 0.010 and 0.020. A pre-registration has to be unambiguous at the
boundary, and s = 0.0100 is a realistic landing point.

L2. docs/seed_variance_51_135.md:80-85 — "best-val epoch" does not name the metric.
docs/tillicum.md:507-511 establishes that this Ultralytics build selects on mAP50-95
alone
, not the 0.1/0.9 fitness blend (the retarget dry run matched the klone arm's ep21
mAP50-95 to five decimals). Two readings of "best-val" pick two different epochs.

L3. docs/seed_variance_51_135.md:83, 101-103, 127 — the budget and the schedule length disagree.
At the doc's own 3.0 h/epoch, 60 epochs is 180 GPU-h, about $162/replicate, but line 102 quotes
~$119 — which is 44 epochs, not 60. And CHAIN=5 buys six 24 h slices = 144 GPU-h, about 48
epochs before startup overhead, so the replicates will in fact stop near epoch 48 rather than
running "the full epochs=60 schedule". Nothing is invalidated (the LR curve is still the
60-epoch one, which is the real justification, and the read is at epoch <= 44), but both stated
numbers are wrong.

L4. stage_two/train.py:277 — "reproduces those runs byte for byte" overclaims.
cuDNN autotuning, AMP loss scaling and DDP allreduce ordering are not seeded, and
torch.use_deterministic_algorithms is not set. The true claim is "same seeds, same data
order". docs/seed_variance_51_135.md:134-135 repeats it as "exactly".

L5. stage_two/run_train_seed.slurm:37-38, 60mkdir -p "$REPO/logs" cannot create the log directory.
Slurm resolves --output=logs/seedvar_%j.out against the submit directory before the script
runs, so if logs/ is missing the job fails to start and the mkdir never executes; and if
submitted from anywhere but $REPO, the mkdir creates a directory Slurm is not writing to.

L6. tests/test_seeding.py:69-74 — one assertion is satisfied by another line.
assert "random.seed(args.seed)" in src is a substring of np.random.seed(args.seed).
Verified: deleting the bare random.seed(args.seed) line — the one that seeds the
horizontal-flip augmentation at stage_two/train.py:217 — leaves the suite green.

L7. stage_two/run_train_seed.slurm:72-73 — no world-size guard.
run_train_epoch_curve.slurm:108-111 warns when WORLD_SIZE != 16, because train.py is
batch_size=1 per rank so world size is the global batch. This launcher computes
WORLD_SIZE and only prints it, so a replicate that ever ran at a different node/GPU count
would silently be a different optimisation regime rather than a seed replicate.


Not findings, checked and cleared

  • The Tillicum positional list and unpack do match (13 passed, sys.argv[1:14]), and seed
    reaches YOLO.train().
  • set -euo pipefail alongside source activate is fine on klone — run_yolo_train.slurm
    has shipped both since the Train a supervised YOLO baseline (YOLO11 / YOLO26) on the RampNet dataset — isolate architecture vs. data #51 grid.
  • DATA_ROOT's default /gscratch/scrubbed/$USER/rampnet_dataset matches the staged dataset
    in docs/stage2_epoch_curve_84.md:158, and YOLO_DATA's /gpfs/scrubbed/... matches
    docs/tillicum.md:473.
  • The ~$50/replicate Tillicum fallback for Campaign B is right (16 GPUs x 3.5 h x $0.90).
  • bash -n does catch the 2b7471c bug class, and SLURM_SCRIPTS finds 21 files against its
    >= 15 floor.

…ding rule (#51, #135)

Review of PR #155. Three findings verified by mutation, all silent.

The test that guards the campaign's central invariant did not guard it.
test_train_sampler_seed_is_derived_not_hardcoded asserted
"sampler_seed_for(args.seed)" in src against the whole file, and train.py
already contains that string in a log line. Deleting seed= from the
DistributedSampler call -- which is exactly the regression rampnet/seeding.py
exists to prevent, and which would make all three replicates share one data
order -- left the suite green. It is now asserted on the parsed statement.

Two smaller versions of the same mistake: "random.seed(args.seed)" in src is
satisfied by np.random.seed(args.seed), so the stdlib call that drives the
horizontal-flip augmentation could be deleted unnoticed; and the Tillicum
positional-argument count matched only "$VAR", so inserting "${VAR}" mid-list
shifted every later field by one and passed. Both now fail as intended.

The pre-registered decision rule divided 0.039 by Campaign A's SD alone. That
gap is a difference between two single-seed runs, so its standard error is
sqrt(s_A^2 + s_B^2). At s_A = 0.010 the table read 4 sigma; with s_B = 0.010 it
is 2.8, and with s_B = 0.020 it is inside the band the same table calls
indistinguishable -- the conclusion could invert with no change to any input the
rule looked at. Campaign B was measuring s_B concurrently and had no reading of
its own at all, leaving half the campaign free to be interpreted afterwards.
Amendment 1 corrects the sigma, makes the bands disjoint at their endpoints,
gives Campaign B a rule against #135's measured 0.0063 paired MDE, and says what
happens if B does not finish in time. The original table is kept verbatim. No
result from either campaign had been scored when this was written.

Also: the doc's headline number, its source document and the script that
computes its statistic all live on PR #154, not on main, which is now stated
rather than left as a dead link; neither campaign had a written path from its
checkpoints to the statistic, and both gaps are named; the Campaign A reproduce
command omitted the PYTHON= that Tillicum needs; and Campaign B writes its only
artifact to /gscratch/scrubbed, which purges on a ~21-day idle window while the
same document says the campaign's calendar is unbounded.

Launcher changes are additive only, since both campaigns are in flight: an
optional RAMPNET_ENV escape hatch (default path unchanged), a world-size warning,
and header notes. The RUNDIR default is deliberately NOT moved off scrubbed --
that would put a resubmission somewhere different from its queued siblings, so
the required copy-out is documented instead and the move left as a decision.

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

Copy link
Copy Markdown
Member Author

Review fixes

Pushed as 455e1c4. Full suite
1368 passed, 1 skipped (the pre-existing skip); bash -n clean on both touched
launchers. No Slurm job was submitted, cancelled or modified, and no launcher default or
argument-passing behaviour was changed — every launcher edit is additive.

Fixed

H1 — tests/test_seeding.py. test_train_sampler_seed_is_derived_not_hardcoded now
parses train.py with ast, locates the train_sampler = DistributedSampler(...)
statement, and asserts the seed= kwarg is present and is exactly
sampler_seed_for(args.seed). Split out test_val_sampler_is_unshuffled_and_therefore_needs_no_seed.
Dead sampler_line removed. Re-ran the mutation: deleting the kwarg now fails at
test_seeding.py:144.

H2 / H3 / L1 — the reading rule. Added ## Amendment 1 (2026-09-04) to
docs/seed_variance_51_135.md, written before any replicate was scored and dated as such.
It (a) corrects the σ to s_gap = sqrt(s_A² + s_B²) with the worked numbers showing the
conclusion can invert, (b) restates the three bands on s_gap with disjoint endpoints,
(c) says what happens if Campaign B does not deliver s_B in time — apply to s_A alone
and report it in those words as an upper bound on significance, never as the finding,
(d) gives Campaign B its own decision rule against #135's measured 0.0063 paired MDE, and
(e) adds A1.3, the missing artifact-to-statistic path for both campaigns, with the two
code changes that are still needed named as gaps. The original table is kept verbatim with
a pointer, so the amendment is auditable rather than a rewrite.

Band edges and the Campaign B rule are the one place I made a scientific call rather
than a mechanical correction — please ratify or replace them.
The σ arithmetic is not a
judgment; where the cut points sit, and whether s_B ≥ 0.0063 should be what closes #135,
are.

M1 — added a "Where the inputs live" section naming PR #154 as the home of
operating_point_parity_51.md, operating_point_parity_51.py and the JSON artifact, with
a † marker on the affected links; same note added at
run_yolo_train_tillicum.slurm:92.

M2 — A1.3, above.

M3 — the Campaign A reproduce block now carries
PYTHON=/gpfs/projects/makelab/$USER/envs/rampnet-yolo/bin/python and says why.

M4 — documented, not changed. The launcher header and a new limitations bullet spell
out that best_model.pth lands on /gscratch/scrubbed, that it purges on ~21 idle days,
and that the copy-out to /gscratch/makelab is part of the protocol; "Reproducing" carries
the cp. The RUNDIR default is deliberately left alone — see below.

M5 — the positional count now matches "$VAR" and "${VAR}". Re-ran the mutation:
inserting "${LR0}" mid-list now fails at test_seeding.py:219.

M6run_train_seed.slurm gained an optional RAMPNET_ENV conda-prefix path that
calls the env's own torchrun by absolute path (the run_train_epoch_curve.slurm pattern).
Unset — the default — behaviour is byte-identical to before: source activate sidewalkcv2,
torchrun off PATH.

M7 — documented in sampler_seed_for's docstring as a .. warning::, plus
test_seed_zero_shares_the_published_data_order pinning the collision as known. The
function is unchanged; changing it would change train.py's behaviour.

L2 — "best-val" now names metrics/mAP50-95(B) from results.csv, citing the
tillicum.md measurement that this build selects on mAP50-95 alone.

L3 — cost corrected: ~$130/replicate as launched (CHAIN=5 = 144 GPU-h ≈ ep48), ~$119
only if stopped at ep44; the old $119-with-60-epochs pairing is called out as inconsistent.
The doc now says the runs stop near ep48, not ep60, and that the LR curve is the real
requirement.

L4train.py:277 no longer says "byte for byte"; it says same seeds and same data
order, and names cuDNN autotuning, AMP and DDP allreduce as unseeded. Same fix in the doc's
closing paragraph.

L5 — the launcher header now says logs/ must exist before submit, because Slurm
opens --output against the submit directory before the script runs, and explains what
the mkdir is actually for. Added to the reproduce block.

L6 — the three RNG assertions are line-anchored. Re-ran the mutation: deleting the bare
random.seed(args.seed) now fails at test_seeding.py:123.

L7 — added the WORLD_SIZE != 16 warning, matching run_train_epoch_curve.slurm:108.
Echo only; no control flow.

Needs your decision — would affect in-flight runs, so not changed

M4, the RUNDIR default. Moving it from /gscratch/scrubbed to the purchased
/gscratch/makelab is the right long-run answer, but a manual resubmission of a failed
klone replicate would then land somewhere different from its already-queued siblings, and
/gscratch/makelab was at 93% bytes / 96% inodes when last measured. Left as-is with the
copy-out documented; worth changing for the next campaign.

H2/H3, the amended band edges — flagged above.

L3, CHAIN=5. Six slices reach roughly ep48 of a 60-epoch schedule. That is fine for
the pre-registered read at ≤ ep44, so I did not touch it — but if you want the replicates
to actually complete 60 epochs for a later question, that is CHAIN=7, and it has to be
set on a fresh submission, since chain links inherit their args from last.pt.

Skipped as not real

  • set -euo pipefail + source activate — I initially suspected the klone jobs would
    die on conda under nounset, on the strength of the "deliberately no set -u" note in
    run_train_epoch_curve.slurm:55. run_yolo_train.slurm has shipped both together since
    the Train a supervised YOLO baseline (YOLO11 / YOLO26) on the RampNet dataset — isolate architecture vs. data #51 grid, so the combination is proven on this cluster. Not a finding.
  • scontrol … | head -n 1 under pipefail — four hostnames fit the pipe buffer, so
    scontrol exits before head closes it and there is no SIGPIPE. Not a finding.
  • Missing #SBATCH --account — jobs 39515025/26/27 were accepted by sbatch and died
    at runtime, which proves the default association can submit to ckpt-all.
  • Chain links running past an early stop — plausible that a residual afterany link
    allocates a GPU to do nothing but final_eval, but I could not verify Ultralytics'
    behaviour on a completed resume without running it, and it is main's behaviour, not
    this PR's. Left unclaimed rather than guessed.
  • Per-rank augmentation streams — all ranks seed random identically, so rank i and
    rank j draw the same flip sequence for their n-th sample. Pre-existing, unrelated to
    the seed flag, and it does not bias the SD across seeds.

🤖 Generated with Claude Code (claude-opus-5)

jonfroehlich added a commit that referenced this pull request Sep 4, 2026
…e bundle truncation travels with its numbers (#135)

Two review passes on this PR (2026-08-18 and 2026-09-03) left thirteen findings open,
and the gate commit 4171a6e added an artifact that contradicts the document it sits
beside. This is the fixes pass those reviews said would follow.

THE HEADLINE CLAIM WAS READ AGAINST THE WRONG BAR

The pre-registration says |delta| / s.e. >= 1.96 per pair. The Results section instead
compared the largest delta to #138's MDE of 0.0063, which is 2.80 x s.e. at 80% power --
1.43x looser -- and concluded "tied at every epoch". Those are different bars: an effect
below the MDE is not thereby non-significant. run_b_gate_135.json, added in 4171a6e,
already records the consequence: epoch 7's +0.0042 gives |z| = 1.45 to 2.64 over the
measured s.e. bracket, and the artifact flags it as clearing 1.96 at the favourable end.

The accurate statement is narrower and is now what the document says. The pre-registered
primary (epoch 8, +0.0030, |z| = 1.02 to 1.86) is a tie at both ends of the bracket --
which is the reading the decision rests on and which does not depend on which s.e. inside
it is chosen. Six of the other seven epochs are ties at both ends. Epoch 7 is undetermined,
not a tie, and resolving it needs the cosine arm's per-panorama detections, which are not
committed. The deviation from the pre-registration is stated where the numbers are, along
with the fact that the pre-registration's own "Exact commands" could not have run the test
as written.

THE DECISION IS UNCHANGED, AND WAS NEVER LOAD-BEARING ON THE TIE

Run B at n=1 cannot be told apart from a seed draw at any length. That argument holds
whether or not epoch 7 separates, which is why overstating the tie bought nothing.

THE NINE CITY BUNDLES ARE CUT AT 0.55, AND EVERY UNPAIRED ROW INHERITS IT

Measured: all nine hold 0.0% of their detections below 0.55 (minima 0.5501-0.5607) while
manual_gold reaches 0.0501 with 28.8% below. So "the #54 operating point of 0.30" is a
no-op on nine of ten splits, max-F1 there peaks on an already-truncated curve (max_f1 ==
f1 exactly on eight of nine cities and on POOLED cities), and the pooling gain is
understated. Measured against the one untruncated arm in committed data
(--reference rampnet_1pass, all ten splits to 0.05): POOLED-all MDE 0.0105 against
manual_gold's 0.0121, a 14% gain rather than 7%. That arm is single-pass and missing seam
detections, so 14% bounds the correction rather than being it; there is no clean
uniform-0.30 RampNet arm in the repo, which is now stated beside the number. The
conclusion is unchanged on either reading: pooling is not a lever.

The artifact now records reference_min_confidence and protocol_threshold_binds per split
and truncated_members per pooled row, so this is visible rather than inferable -- it sat
undetected through two reviews precisely because the only evidence was indirect.

benchmark_power_135.json regenerates with ZERO changed values: the additions are new keys
plus self_pair's max-F1, which is now null. Every number the documents quote still stands.

REST OF THE FIX LIST

- self_pair max-F1 was identically zero by construction (max-F1 re-picks its own
  threshold, so shifting the read-out point cannot move it). It read as a measured zero
  with zero uncertainty in all 24 rows; it is now null with a note. That block bounds F1.
- The headline table's rows 1 and 4 were F1 standard errors under a max-F1 header
  (0.0042/0.0117 and 0.0039/0.0109). Now max-F1: 0.0041/0.0114 and 0.0039/0.0108.
- Four-decimal drift against the artifact, all corrected: +0.1119 -> +0.1117,
  +0.1492 -> +0.1489, c = 123 -> 124, "9 of 356" -> "10 of 357", discordance ranges to
  2.50-4.26 / 2.50-6.66 / 6.48-6.66, MDE upper 0.0081 -> 0.0082, and the rung doc's
  0.961/3.979 -> 0.962/3.980 (the trap that file's own provenance note warns about).
- The recall table printed six of the nine pairs the script computes, and the three it
  dropped included the only "not resolvable" verdict. All nine now print, and the table's
  verdicts carry the conditional the max-F1 table already had: they rest on a discordance
  range taken from three cross-detector pairs, none of them epoch-vs-epoch.
- "Every derived number in this document is in benchmark_power_135.json" was false. Four
  classes of exception are now named, and test_committed_json_matches_the_doc_headline_
  numbers pins every headline value so the prose cannot drift again.
- dump_peaks_from_cache.py hardcoded a run_a_epoch_N label whatever --summary-csv it was
  given, so a second arm's dumps were either invisible to the reader or overwrote Run A's
  committed ones. Adds --label-prefix; --verify now fails loudly on a fingerprint the
  summary does not contain instead of silently checking nothing; exclude_border is read
  from the extractor rather than restated; the docstring no longer recommends the one
  output directory the same file's help text forbids.
- stage2_epoch_curve_84.md's status block still stated the superseded curve shape. It now
  carries the #135 amendment: the plateau is 2-6, not 2-8.
- The rung's pre-registration claimed nothing above Results had been edited; two commits
  had. Says what was edited and what was not.
- Provenance gaps stated rather than implicit: the 21 restarts and 35.06 h come from
  sacct -D with no dump committed (a clean clone can count 18 event files, 11 with steps);
  the 560.9 GPU-hours are pending #147's compute_log.jsonl; the LR verification block gets
  its one-line reproduce command; whether the rung's eval cache survived is unknown and is
  now said so.
- The reopening condition pointed at docs/seed_variance_51_135.md, which is not on this
  branch; it points at PR #155. #151's +0.115 F1 is marked as not re-derivable here. The
  "~0.01 measured but not attributable" threshold the decision's third reason turns on is
  marked as a working assumption rather than a measurement.
- Run A's epoch-7 max-F1 differs between two documents in this PR (0.9110 from
  summary.csv, 0.9107 post-#140). Both arms were scored under the pre-#140 matcher so the
  comparison is internally consistent; that is now said, in both places.
- Dead Scored.gt_pano removed. observed_and_se(paired=...) asserts shared panorama order,
  the way mcnemar already asserts its own. The test's (prediction_confidence(p) or -1e9)
  mapped a legitimate 0.0 confidence to -1e9; it checks for None.
- Deliberately NOT changed, with the reason recorded in the code: the single Generator
  threaded through every group, and the redundant res_a/res_b bootstraps. Either change
  shifts the draw stream and moves every standard error these documents quote, for no gain
  in correctness. The --splits caveat is documented instead.
- train.py's epoch-boundary checkpoint window is described in the ResumeSkipSampler
  docstring rather than fixed: latest_checkpoint.pth is written after validation, so a
  preemption there loses the whole pass (the committed events show it happening twice).

Suite: 1,373 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jonfroehlich and others added 2 commits September 4, 2026 14:31
… about the wrapper

#138 and #154 merged while this branch was open, and #138 rewrapped the train sampler as
ResumeSkipSampler(DistributedSampler(...)). Three conflicts in stage_two/train.py, all
additive on both sides except one that is genuinely semantic:

  train_sampler = ResumeSkipSampler(
      DistributedSampler(..., shuffle=True, drop_last=True,
                         seed=sampler_seed_for(args.seed)))

The seed has to sit on the INNER DistributedSampler. On the wrapper it would be accepted
and inert -- the sweep would run with one data order across all three replicates and
nothing in any log would say so, which is the exact failure this branch exists to prevent.
The other two conflicts are the argparse block and the startup log line; both sides added
different things and both are kept.

Two tests then failed, and both were right to:

- tests/test_seeding.py asserted the assignment was a DistributedSampler call. It is now a
  wrapper. Rather than loosen the assertion to a substring -- which is what made this test
  vacuous in the first place -- it walks into the wrapper via _unwrap_to() and fails loudly
  if the target is ambiguous or absent.
- tests/test_resume_skip_sampler.py executes parse_args in a restricted namespace, which
  now needs HISTORICAL_SEED injected.

Re-verified by mutation, both caught: deleting seed= from the inner sampler, and moving it
to the wrapper where it would be inert. The second is a NEW failure mode that only exists
because of this merge.

Full suite: 1,481 passed, 1 skipped. bash -n clean on the launcher.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An amendment to a pre-registration is only worth the audit trail attached to it. This
records who ratified it and the one fact that decides whether it is legitimate: it was
ratified on 2026-09-04, before any replicate from either campaign had been scored.

Nothing about the rule changes. The cut points remain 0.010 and 0.020 as originally
written; the amendment applies them to sqrt(s_A^2 + s_B^2) rather than s_A alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonfroehlich
jonfroehlich merged commit 2916ba5 into main Sep 4, 2026
2 checks passed
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