Merge infra/bench-variance (#54)

.gitea/workflows/bench-variance.yml

@@ -0,0 +1,91 @@
+# Measure the CI runner's own benchmark variance.
+#
+# #54 asks whether benchmark regressions can be gated. The threshold is the
+# whole problem: too tight and CI goes red on noise, which trains people to
+# re-run until green; too loose and it never fires. Which of those is possible
+# depends on a number nobody has measured — how much this runner's results move
+# between identical runs.
+#
+# So: run one unchanged benchmark ten times and report the spread. If it is
+# ~15%, a fixed-threshold gate is dead and the answer is a tracker; if it is
+# ~2%, a gate at 10% is meaningful.
+#
+# Manual only. It takes ten benchmark runs and answers a question that is asked
+# once, not every push.
+name: Benchmark variance
+
+on:
+  workflow_dispatch:
+    inputs:
+      runs:
+        description: How many repeats
+        required: false
+        default: "10"
+
+jobs:
+  variance:
+    name: runner variance on one benchmark
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: Swatinem/rust-cache@v2
+
+      # `joint_factorise_480_appearances` is the right probe: ~9 ms, so it is
+      # long enough not to be dominated by timer overhead, and it is the
+      # measurement this crate most wants protected — the dense factorisation
+      # #52 is about replacing.
+      - name: Warm up
+        run: cargo bench --bench joint -- joint_factorise_480_appearances --warm-up-time 1 --measurement-time 3
+
+      - name: Repeat the same benchmark
+        run: |
+          set -euo pipefail
+          for i in $(seq 1 "${{ inputs.runs || '10' }}"); do
+            echo "== run $i =="
+            cargo bench --bench joint -- \
+              joint_factorise_480_appearances --warm-up-time 1 --measurement-time 3 \
+              2>&1 | tee -a raw.txt
+          done
+
+      - name: Report the spread
+        run: |
+          set -euo pipefail
+          # Criterion prints `time:   [lo mid hi]` with a unit after each. Take
+          # the midpoints. `sort -n` rather than awk's `asort`, which is a gawk
+          # extension the runner's mawk does not have — that failed on the
+          # first try here.
+          grep -oE 'time:[[:space:]]+\[[^]]+\]' raw.txt \
+            | sed -E 's/.*\[[^ ]+ [^ ]+ ([0-9.]+) ([^ ]+).*/\1 \2/' > mids.txt
+          echo "--- midpoints ---"
+          cat mids.txt
+          # Criterion picks a unit per run, so mixed units would have us
+          # comparing 9 ms against 9 us as if they were the same number — the
+          # plausible-looking wrong answer this crate keeps removing. Refuse.
+          if [ "$(cut -d' ' -f2 mids.txt | sort -u | wc -l)" -ne 1 ]; then
+            echo "runs reported different units; the spread would be meaningless"
+            cut -d' ' -f2 mids.txt | sort | uniq -c
+            exit 1
+          fi
+          sort -n mids.txt | awk '{ v[NR]=$1; u=$2; s+=$1 }
+            END {
+              if (NR == 0) { print "no samples parsed - see the raw.txt artifact"; exit 1 }
+              printf "n      = %d\n", NR
+              printf "min    = %.4f %s\n", v[1], u
+              printf "median = %.4f %s\n", v[int((NR+1)/2)], u
+              printf "max    = %.4f %s\n", v[NR], u
+              printf "mean   = %.4f %s\n", s/NR, u
+              printf "spread = %.2f%%  (max-min)/min\n", 100*(v[NR]-v[1])/v[1]
+              print ""
+              print "Read it against #54: a spread near 15% kills both"
+              print "fixed-threshold options and the answer is a tracker;"
+              print "a spread near 2% makes a gate at 10% meaningful."
+            }'
+
+      - uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: bench-variance-raw
+          path: |
+            raw.txt
+            mids.txt

benches/joint.rs

@@ -18,8 +18,14 @@ fn fitted() -> History<String> {
         .beta(1.0)
         .score_sigma(2.0)
         .drift(ConstantDrift::new(0.05))
+        // `max_iter: 30` was here, and this fixture needs more: `converge`
+        // reported `NotConverged { iterations: 30, final_step: (4.5e-4, 0.0) }`
+        // once it stopped returning short fits silently. The benchmark measures
+        // the factorisation, whose cost depends on the fit's *shape* rather
+        // than its exactness — but measuring it on an unconverged fit is still
+        // measuring something nobody would run.
         .convergence(ConvergenceOptions {
-            max_iter: 30,
+            max_iter: trueskill_tt::ITERATIONS,
             epsilon: 1e-10,
             alpha: 1.0,
         })