This was generated by AI during triage.
Summary
tests/test_benchmark_overhead.py gates xarray adapter overhead as a ratio against the equivalent NumPy path. That ratio is the wrong shape for the cost it now guards: the calendar validation added in #757 is a fixed per-call cost, so the measured percentage is governed mostly by how cheap the NumPy baseline is, not by whether the adapter regressed.
This is the third operation-specific budget in the file. Each was widened for the same underlying reason, which suggests the metric rather than the numbers is the problem.
Evidence
Current constants in tests/test_benchmark_overhead.py:
The Thornthwaite widening came from a single xr.infer_freq call added by _build_daily_calendar_plan, measured locally at ~0.14 ms against a ~0.4 ms NumPy baseline. That moved the ratio from comfortably under 80% to 85–98% across repeated local runs, with no change to the adapter's per-element work.
At 1.20 the gate now permits roughly 2.2x the NumPy path before failing, which is weak protection against a genuine regression. Meanwhile the operations with the heaviest NumPy baselines are held to 0.80, so the same absolute regression is judged far more strictly on a cheap operation than an expensive one — the opposite of what the gate should do.
Current behavior
_assert_overhead_within_budget computes (xarray_time - numpy_time) / numpy_time and compares it to a per-operation ratio. Because the adapter's fixed costs (coordinate validation, frequency inference, metadata construction) do not scale with the wrapped computation, the ratio inflates as the baseline shrinks. Every time that inflation crosses a threshold, the response has been to widen that operation's budget.
Desired behavior
Guard the invariant that actually matters: the adapter's fixed overhead per call should stay within an absolute budget (e.g. under ~0.5 ms), independent of how cheap or expensive the wrapped NumPy computation is. A ratio check may still be worth keeping for operations where the per-element cost dominates, but it should not be the only signal, and it should not require a new constant each time a fixed cost is added.
Apply the change consistently across all measured operations rather than to Thornthwaite alone, so the file stops accumulating one-off budgets. Retain the existing diagnostic output — both timings and the budget — on failure, as #740 established.
Do not address this by removing the calendar validation from pet_thornthwaite; that check is what prevents a daily coordinate being silently reshaped into 12-month rows. Do not simply raise _OVERHEAD_THRESHOLD for everything.
Context
Follow-up from #757, which made calendar semantics explicit at the xarray seam. Related to #740, which widened the Hargreaves budget for a different cause (GitHub-hosted runner variance) but the same underlying metric weakness.
Summary
tests/test_benchmark_overhead.pygates xarray adapter overhead as a ratio against the equivalent NumPy path. That ratio is the wrong shape for the cost it now guards: the calendar validation added in #757 is a fixed per-call cost, so the measured percentage is governed mostly by how cheap the NumPy baseline is, not by whether the adapter regressed.This is the third operation-specific budget in the file. Each was widened for the same underlying reason, which suggests the metric rather than the numbers is the problem.
Evidence
Current constants in
tests/test_benchmark_overhead.py:_OVERHEAD_THRESHOLD = 0.80(line 35) — shared default_PET_HARGREAVES_OVERHEAD_THRESHOLD = 1.00(line 41) — widened in Harden flaky PET Hargreaves xarray overhead benchmark #740 for runner variance_PET_THORNTHWAITE_OVERHEAD_THRESHOLD = 1.20(line 50) — widened in fix(xarray): make calendar semantics explicit at the xarray seam #757The Thornthwaite widening came from a single
xr.infer_freqcall added by_build_daily_calendar_plan, measured locally at ~0.14 ms against a ~0.4 ms NumPy baseline. That moved the ratio from comfortably under 80% to 85–98% across repeated local runs, with no change to the adapter's per-element work.At 1.20 the gate now permits roughly 2.2x the NumPy path before failing, which is weak protection against a genuine regression. Meanwhile the operations with the heaviest NumPy baselines are held to 0.80, so the same absolute regression is judged far more strictly on a cheap operation than an expensive one — the opposite of what the gate should do.
Current behavior
_assert_overhead_within_budgetcomputes(xarray_time - numpy_time) / numpy_timeand compares it to a per-operation ratio. Because the adapter's fixed costs (coordinate validation, frequency inference, metadata construction) do not scale with the wrapped computation, the ratio inflates as the baseline shrinks. Every time that inflation crosses a threshold, the response has been to widen that operation's budget.Desired behavior
Guard the invariant that actually matters: the adapter's fixed overhead per call should stay within an absolute budget (e.g. under ~0.5 ms), independent of how cheap or expensive the wrapped NumPy computation is. A ratio check may still be worth keeping for operations where the per-element cost dominates, but it should not be the only signal, and it should not require a new constant each time a fixed cost is added.
Apply the change consistently across all measured operations rather than to Thornthwaite alone, so the file stops accumulating one-off budgets. Retain the existing diagnostic output — both timings and the budget — on failure, as #740 established.
Do not address this by removing the calendar validation from
pet_thornthwaite; that check is what prevents a daily coordinate being silently reshaped into 12-month rows. Do not simply raise_OVERHEAD_THRESHOLDfor everything.Context
Follow-up from #757, which made calendar semantics explicit at the xarray seam. Related to #740, which widened the Hargreaves budget for a different cause (GitHub-hosted runner variance) but the same underlying metric weakness.