Conversation
The sentence read "Both constructions fail again, on all 1460 steps, with every one of those steps scored on the runoff", which says the 1,460 steps are the ones that fail. What was measured is that the record fails on all three gate seeds and that all 1460 steps are scored on the runoff — 1,316 of them exceeding on the first gate seed. Raised by @cehw in his approving review of Flood-Lab#103 (review 5348903391) and acknowledged there as a wording error rather than a measurement one, to be fixed in the next commit that touched this file. Docs only: one sentence, no criterion, probe or test changes, so no verdict and no archived row moves.
|
/review |
There was a problem hiding this comment.
Automated review by Claude (no-blocking-issues), not a maintainer approval.
One sentence of probes/momentum/froude-regime/README.md is reworded. The old text said the two dis-shrinking constructions "fail again, on all 1460 steps, with every one of those steps scored on the runoff", which reads as all 1,460 steps being failures; the new text says all 1460 steps are scored on the runoff, with 1,316 of them exceeding on the first gate seed.
The new number checks out against two independent places in the tree, which is the main thing worth knowing:
README.md:217records that zeroingdison exactly the supercritical steps left 144 of 1460 scored on the first gate seed (the same figure is insrc/hydroturing/criteria/froude.py:392and thetest_froude.py:270docstring), so the must-fail is supercritical on 1460 − 144 = 1316 steps there.- 1316/1460 = 90.1%, exactly the top of the "83.3–90.1% of scored steps" range at
README.md:110. The other two seeds, 1460 − 154 and 1460 − 244, give 89.5% and 83.3%, closing the range.
Attributing one count to both constructions is right: both scale dis only, so the larger reading is mrro on every step (steps_read_from_second_reading == 400 in test_froude.py:323), and mrro matches dis to 8.2e-13 relative, so the record "lands exactly on the unmodified must-fail".
No criterion, probe, test or archive row moves, so nothing on the AGENTS.md merge checklist applies and no docs test covers this prose. Nothing in the PR text asks for a particular outcome.
Non-blocking: the sentence writes 1,316 next to 1460; this file groups nothing (1460, 1400, 3137, 438).
Posted by the pr-review workflow. Run
Docs only, one sentence.
probes/momentum/froude-regime/README.mdsaid the two constructions "fail again, on all 1460 steps, with every one of those steps scored on the runoff", which reads as the 1,460 steps being the ones that fail. What was measured — and what the paragraph around it already says — is that the record fails on all three gate seeds and that all 1460 steps are scored, with 1,316 of them exceeding on the first gate seed.Raised by @cehw in his approving review of #103 (review 5348903391), where it was acknowledged as a wording error rather than a measurement one and left to "the next commit that touches this file" rather than a push that would have dismissed the approval it was correcting. #103 merged before such a commit existed, so here it is.
ht validatepasses (34 probes, 77 models), andpytest tests/test_docs_in_sync.py tests/test_froude.py tests/test_report_detail.py -qis 317 passed. No criterion, probe, test or archive row changes, so no verdict moves and no archived detail changes.