Repository navigation
fix(xtest): show the faster-tail p-value beside an improvement - #643
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: displaying the faster-tail p-value for benchmark improvements. Linked Issues check The only directly linked issue is #621. Issue#621is closed and supplies historical context only. No active directly linked issue provides coding requirements for this pull request.Out of Scope Changes check The changes stay within the stated benchmark-report scope. xtest/perf/report.pyselects the one-sided tail and adds the footer note.xtest/test_bench_report.pytests the rendering cases. `.github/…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the p-value line
One tail for ratios under one
Another when they rise above
Tests hop through each fallback case
The workflow runs them in the sun
Comment @coderabbitai help to get the list of available commands.
1d7ca04 to
d9609b0
Compare
|
2 participants
Problem
The benchmark report's
p (BH)column always showed the slower-tail p-value (p_adjusted/p_value). For a real speedup that tail is ~1, so anIMPROVEDrow read e.g.ratio 0.50 … p 1.000 … IMPROVED— the p-value contradicted the verdict on the same row. The verdict itself was correct (it already usedp_adjusted_faster); only the rendering was wrong.Fix
_p_cellnow shows the one-sided tail in the direction of the observed ratio: the faster tail whenratio < 1, the slower tail otherwise, each preferring the BH-adjusted value and falling back to unadjusted (controls stay outside the BH families). The table footer notes that p is one-sided.Ported from the closed #621 branch. Stage-1 follow-up to #625 (DSPX-4372).
Tests
New offline
xtest/test_bench_report.py(added to thecheck.ymlharness list): improvement → faster tail, slowdown/no-change → slower tail, unadjusted fallback,<0.001floor, NaN →-.Note: the
check.ymllist edit touches the same lines as #640 (which addstest_bench_config.py); whichever lands second needs a trivial rebase.Summary by CodeRabbit