Wire the committed TrackMan captures into CI so carry accuracy is measured on every PR

[cvrt-jh]

Description

While reading the ballistics code I noticed the repo already has almost everything needed to guard carry accuracy — session_logs/OpenFlight-Test.Normalized.csv is committed, and scripts/analysis/validate_ballistics.py scores the model against it properly (including the 1.184 kg/m³ air-density match for TrackMan's "Flat" normalization, which is a nice detail). The one missing piece is that nothing in tests/ or .github/workflows/ calls it, so the check is manual-only and carry accuracy isn't measured on PRs.

grep -rn "OpenFlight-Test\|comparison_2026" tests/ .github/ returns nothing.

The existing carry tests are deliberately broad ranges — tests/test_ballistics.py:87 asserts 250 <= carry <= 300 — so they catch gross breakage but not a systematic offset. Running your validator on the committed capture, with TrackMan's own launch conditions fed in:

OVERALL  n=24  bias=-5.05 yd  rmse=24.52 yd  mae=20.36 yd  max|d|=52.29 yd
  club              n     bias    rmse     mae   max|d|
  7-iron            9   +11.00   11.64   11.00    16.25
  driver            8   -37.18   38.60   37.18    52.29
  pitching wedge    7   +11.02   13.56   13.16    16.54

The per-club spread is interesting on its own (the driver column looks like a low-spin-parameter Cl fit issue — CL_HALF_SP = 0.15 sits above the driver's whole realistic Sp range). I'd rather not conflate that with this issue though; happy to file it separately if useful.

What did you expect to happen?

A change that moves carry accuracy in the wrong direction should show up in CI.

Proposed solution

I have a PR ready — one new test file, no source changes — that replays the committed capture through the production resolve_launch/simulate path and asserts per-club RMSE against a budget seeded at the current measured error rather than at zero, so it blocks new error immediately without blocking anyone on the existing offset. It imports your load_trackman/validate_tm_inputs/_stats rather than reimplementing them, so the test and the tool can't drift apart, and skips cleanly if the capture file is absent.

I checked it catches things the current tests don't. CL_SATURATION 0.32→0.34 with CL_HALF_SP 0.15→0.14 passes all 21 existing ballistics tests, but fails here — overall RMSE actually improves (24.5→21.2) while 7-iron (11.6→16.0) and wedge (13.6→15.5) get worse. That's the case per-club budgets are for.

Full suite with it added: 1326 passed, 8 skipped. Pylint 9.70, ruff clean.

Two things I'd rather have your call on than guess at:

• budgets inline in the test module, or a committed JSON alongside the capture?
• gate on RMSE (what I did), or on bias, or both?

Happy to adjust either way, or to drop it if you'd rather solve this differently.

Area

Ballistics / physics model, CI

[cvrt-jh]

PR with the proposed test is up: #219. Happy to rework the budget storage or the gating metric per the two questions above.

[cvrt-jh]

Thanks for merging #219 so quickly — and for the kind word.

Post-merge CI on main (fc1e32f) confirms the one thing I couldn't verify locally: the test runs green on the full 3.10/3.11/3.12 matrix, and all 5 cases genuinely execute rather than skipping (suite went to 1327 passed, 8 skipped — same 8 skips as before, so nothing regressed).

@jewbetcha two follow-ups, whenever you have a moment:

1. Should this issue stay open? My PR description said "Addresses #218" rather than "Fixes #218", so GitHub didn't auto-close it. The CI gap itself is closed now. Happy for you to close it, or I can leave it open as the tracking issue for the two design questions below.

2. The two open questions — the merged version picked defaults, and I'd rather match your preference than leave mine baked in:

• Budget storage: currently inline constants in the test module (RMSE_BUDGET_YARDS). A committed JSON next to the capture would make ratcheting a data change rather than a code change. Either is a small follow-up PR.
• Gating metric: currently per-club RMSE. Bias would catch a systematic offset more directly (RMSE conflates offset and scatter), so gating on both is defensible.

3. Next up, if you want it: I'd like to take on the aero constants — the driver column in the table above (-37.18 yd bias fed TrackMan's own launch conditions). CL_HALF_SP = 0.15 sits above the driver's entire realistic spin-parameter range (Sp ≈ 0.08), so Cl lands near 0.11 where Bearman & Harvey / Kensrud & Smith put it at ~0.18-0.25 — the lift curve never leaves its low-lift regime for a driver, while irons at Sp ≈ 0.27 are fine. That matches the symptom exactly: driver short, irons/wedges long.

A quick hand-fit to CL_SATURATION=0.26, CL_HALF_SP=0.06 took overall RMSE from 24.52 to 11.37 yd and driver bias from -37 to -10. I'm not proposing those numbers — I'm currently running your sweep_ballistic_coeffs.py to convergence and would use its output, with the before/after validator table in the PR body and the now-merged regression test as the guard.

One thing I'd flag: test_spin_decays_over_flight hardcodes 2300 < final_spin < 2900 on an assumed ~6s flight, so a correctly-lifted ball trips it. That fixture needs updating as part of the change — wanted to surface it rather than quietly widen the assertion.

Also worth noting #169 (altitude air density) touches the same file and has been open since Aug 1. It doesn't touch the lift/drag constants so there's no logical conflict, but whichever lands second needs a trivial rebase. Happy to sequence behind it if you'd prefer that one first.

[jewbetcha]

thanks for the detailed write up @cvrt-jh !

I think we can close this one and open new issues as necessary. Aero constants sounds great, that would be a huge upgrade for us. Feel free to take it on.

The project has drifted quite a bit since #169, so it is probably worth it to start a new branch

[HuggeK]

Wouldn’t it be better to have it written in meters? Precision can be lost and it is easier to work with in the backend if everything is just meters.

[cvrt-jh]

@HuggeK on metres — I agree with the principle and it's already half-true, but I'd argue against changing it in this test specifically.

ballistics.py already integrates in SI internally: BALL_MASS_KG, BALL_RADIUS_M, AIR_DENSITY_STD in kg/m³, velocities in m/s, and it only converts to yards/mph at the boundary via M_TO_YD / MPS_TO_MPH when building the Trajectory. So the physics core is metres already, which is the part where precision would actually matter.

The reason the regression test asserts in yards is that it compares against TrackMan's Carry Flat - Length column, which the reference CSV publishes in yards. Converting to metres would mean multiplying the measured truth by 0.9144 before comparing — that adds a conversion step to the reference value rather than removing one, so it can only lose precision, never gain it. The budgets are yard thresholds against yard measurements.

Where your point does land, and I think it's a real finding: the backend has four different literals for the same conversion. MPS_TO_MPH = 2.23694 in rolling_buffer/processor.py, MPH_PER_MS = 2.23694 duplicated in both iwr6843/club.py and iwr6843/lcmf.py, 2.23693629 in src/analysis/analyze_capture.py, plus bare * 0.44704 in launch_monitor.py and camera/launch_angle.py. A single src/openflight/units.py would be a clean, low-risk PR that doesn't touch behaviour. Want me to open an issue for that?

@jewbetcha — aero constants PR is up: #229. Cut from current main rather than rebasing #169, per your note about drift. Headline is 24.52 -> 3.97 yd overall RMSE and driver bias -37.18 -> -0.45, produced with your own sweep_ballistic_coeffs.py; I've included leave-one-out cross-validation and a comparison against an independent published 1040-shot fit in the PR body, plus the limitations I think you should weigh before merging.

One thing I found while validating it that's worth its own issue: the Cl Hill form is monotonic, so it structurally cannot represent the measured Cl peak near S≈0.5. Swapping it for a quadratic reaches 1.99 yd overall (vs 3.97) and 3.33 yd on wedges (vs 6.26) on the same 24 shots — which is why wedges stay the weak column in #229 no matter how the constants are tuned. I'll file that separately rather than expand the PR.

Happy for you to close this issue whenever — the CI gap it described is fixed.

[cvrt-jh]

Closing per @jewbetcha's note above - the CI gap itself is closed by #219, and new issues get opened as needed (#230 is the first of those).

The two design questions from the original post are unresolved but not blocking; happy to open a separate issue for them if either becomes worth changing:

• Budget storage: inline constants in the test module vs a committed JSON next to the capture (would make ratcheting a data change rather than a code change).
• Gating metric: currently per-club RMSE; bias would catch a systematic offset more directly, so gating on both is defensible.