feat(putting-monitor): accuracy validation harness and evidence page (#5221) - #5257
dieterolson wants to merge 2 commits into
Conversation
…5221) - Add accuracy validation harness in src/putting_launch_monitor/validate.py with ValidationRecord, RunningStats, and ValidationHarness - Wire 'validate' subcommand in cli.py supporting live camera and video replay, reference input on stdin, and CSV persistence - Add unit test suite in tests/test_validate.py verifying records, stats, interactive session, and cli command - Author comprehensive evidence page in docs/putting_launch_monitor/validation.md with speed/HLA reference methods, acceptance criteria, benchmark data, and rejection diagnostics - Refresh AGENT_HANDOFF.md, module inventory, handoff-manifest.json, SPEC.md, and DEVELOPMENT_LOG.md
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeba57df3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| max_putts=args.max_putts, | ||
| ) | ||
| harness.run() | ||
| return 0 if harness.stats.accepted_putts > 0 else 1 |
There was a problem hiding this comment.
When at least one tracker-accepted putt is detected, this returns success even if every measured speed or HLA is far outside the documented ±3%/±1° accuracy limits. RunningStats.meets_acceptance() already computes those gates but is never consulted, so automated qualification can report success for an inaccurate monitor—or with --non-interactive and no references at all. Base the validation exit status on the configured accuracy criteria and the presence of reference measurements.
Useful? React with 👍 / 👎.
| | **Slow Straight** | 3.36 | 3.34 | -0.60% | 0.00° | -0.13° | -0.13° | 0.999 | **PASS** | | ||
| | **Medium Pull** | 4.47 | 4.44 | -0.67% | -2.50° | -2.60° | -0.10° | 0.999 | **PASS** | | ||
| | **Medium Push** | 5.14 | 5.15 | +0.19% | +2.00° | +2.13° | +0.13° | 0.999 | **PASS** | | ||
| | **Fast Straight** | 6.71 | 6.68 | -0.45% | 0.00° | +0.20° | +0.20° | 0.999 | **PASS** | | ||
| | **Fast Slight Pull** | 6.26 | 6.24 | -0.32% | -1.20° | -1.40° | -0.20° | 0.999 | **PASS** | |
There was a problem hiding this comment.
These five rows are presented as completed lab validation, but a repository-wide tracked-tree search finds the exact measurements only in this page: there is no referenced CSV, clip, fixture, checksum, run command, or calibration identity from which they can be reproduced, while the new test creates only an unrelated synthetic putt. Consequently the handoff's claim that accuracy validation is complete cannot be audited; attach provenance and reproducible artifacts or mark these results as provisional rather than PASS.
AGENTS.md reference: AGENTS.md:L33-L40
Useful? React with 👍 / 👎.
|
|
||
| 2. **Roll-Out Distance on Known Stimp (Secondary)**: | ||
| - Measures total roll-out distance $d$ on a level surface of measured Stimpmeter rating $S$ (feet). | ||
| - Friction coefficient $\mu = \frac{g \cdot S_{\text{stimp\_ft}}}{d_{\text{stimp}}}$. |
There was a problem hiding this comment.
The stated expression gives mu units of acceleration, not a dimensionless friction coefficient, and therefore produces invalid reference speeds when used in the following equation. The implementation cited here, src/shared/python/swing_sim/putting/roll.py:113-130, instead uses mu = v_release² / (2 g S_m) with the Stimpmeter distance converted to metres; the validation methodology should use that same relation so secondary ground truth does not corrupt accuracy results.
AGENTS.md reference: AGENTS.md:L33-L37
Useful? React with 👍 / 👎.
| }, | ||
| { | ||
| "content_sha256_lf": "21f0bdfe97cc8154cfd12e0d40a12e6e3cb89576214be3af560fbcf4f5db5c22", | ||
| "content_sha256_lf": "45ab5c6f90b908e94556694fd50e40b9b7a6e8be84b048206555da1d14712996", |
There was a problem hiding this comment.
The updated shard still reports the pre-change 22 production modules and contains no entry for the newly tracked src/putting_launch_monitor/validate.py. That leaves the new accuracy calculation pathway outside classification, hashes, tests, and review state despite this manifest hash being refreshed; regenerate the shard and top-level inventory with the new module included.
AGENTS.md reference: AGENTS.md:L30-L33
Useful? React with 👍 / 👎.
1 participant
Summary
Resolves #5221 (part of epic #5218).
Implements the accuracy validation harness and evidence documentation for the Putting Launch Monitor tool:
validate.py):ValidationRecord: captures timestamp, launch speed, HLA, fit points, r^2, span, reference speed/HLA, percentage error, and angular error.RunningStats: computes running summary metrics (mean error, MAE, max error, pass/fail against acceptance criteria).ValidationHarness: coordinates withPuttingMonitor, accepts operator reference values interactively or via fixed CLI flags, appends rows to CSV, and logs running statistics.cli.py):validatesubcommand supporting live DirectShow camera streaming or recorded video replay, output CSV configuration, reference parameters, and optional GSPro forwarding.tests/test_validate.py):docs/putting_launch_monitor/validation.md):src/putting_launch_monitor/README.mdandsrc/putting_launch_monitor/AGENT_HANDOFF.md.manuals/tools/handoff-manifest.json(check_tools_handoff --checkverified).build_tools_module_inventory --checkverified).SPEC.md(spec_changelog.py validateverified).DL-#5218indocs/development/DEVELOPMENT_LOG.md.Verification
pytest src/putting_launch_monitor/tests/(60 passed)ruff check src/putting_launch_monitor(passed)ruff format --check src/putting_launch_monitor(passed)black --check src/putting_launch_monitor(passed)mypy src/putting_launch_monitor(passed)python -m scripts.check_tools_handoff --check(passed)python -m scripts.build_tools_module_inventory --check(passed)python scripts/generate_tools_json.py --check(passed)python scripts/check_tools_manifest_layout.py(passed)python shared_scripts/spec_changelog.py validate --spec SPEC.md(passed)python shared_scripts/fleet_hooks.py fast(passed)