Repository navigation
Speed up calibrator hot paths 12–21× with bit-identical results - #20
Conversation
Profiling showed each update spent over 90% of its ~550 us inside scikit-learn's IsotonicRegression wrapper (input validation, dataframe sniffing, interpolator construction) around a PAV step on B numbers. - Fit with scipy.optimize.isotonic_regression (C++ PAVA, SciPy >= 1.12). Update and half-life calibrate: 11-27x faster. - Large batches locate bins and interpolate by direct calculation on the evenly spaced grid, corrected against the same edge arrays, instead of a binary search: O(1) per value, identical to np.digitize/np.interp. Small batches keep NumPy's lower fixed cost. - Lazy-load names that need scikit-learn (PEP 562), so importing the streaming calibrator no longer loads it: 1.1 s to 0.1 s. - BatchCalibrator keeps history in chunks joined at refit, with one-byte labels: no-refit updates stop copying all history (265x at 1M rows) and history drops from 16 to 9 bytes per row. - compare_prequential scores batches with sklearn's exact two-column Brier expression instead of re-validating each one: 18x at batch 1. Every output was checked bit for bit against main on 1.2M values from randomized workloads; tests/test_kernels.py keeps the equivalences in CI and benchmarks/speed.py reproduces the timings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011pXaJ5DLex7BrdUyfTqjL4
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: e992eb5ae3
ℹ️ 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".
| while excess > 0: | ||
| oldest = self._probability_chunks[0].size | ||
| if oldest <= excess: | ||
| del self._probability_chunks[0], self._outcome_chunks[0] |
There was a problem hiding this comment.
When a rolling calibrator receives small updates and refit_every exceeds window_size, the full window can contain one chunk per update. Once full, every subsequent update executes these front deletions, which shift all remaining list entries and make each nominally constant-size update O(window_size); dropping many chunks for a large incoming batch can become quadratic. Use a deque, a moving head index, or periodic chunk compaction so window eviction does not shift every retained chunk.
Useful? React with 👍 / 👎.
| return sum( | ||
| chunk.nbytes for chunk in self._probability_chunks + self._outcome_chunks | ||
| ) |
There was a problem hiding this comment.
When a rolling window removes less than half of its oldest chunk, _drop_head intentionally leaves a view backed by the larger original array, but this property counts only the view's nbytes. For example, inserting 10 rows into a window of 8 keeps 90 bytes allocated while reporting 72, so the public memory diagnostic understates retained storage. Either copy after trimming or traverse each chunk's base allocation as retained_array_bytes does.
Useful? React with 👍 / 👎.
- Score each prequential batch with brier_score again rather than a private copy of its formula. - Drop the chunked BatchCalibrator history. Evicting the oldest chunk was O(window) per update, and per-chunk object overhead could exceed the data it held. Keep only one-byte labels (16 -> 9 bytes per row). - Replace the string-built import probe in benchmarks/speed.py with a real function run via --probe-import. Import NumPy inside main() and probe before loading anything heavy: on Linux a forked child's peak RSS starts from the parent's, which had inflated the earlier figure. - Remove an unneeded __future__ import. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011pXaJ5DLex7BrdUyfTqjL4
soodoku
changed the title
2 participants
Summary
A profile-driven pass over time, space and import cost. Every output is bit-for-bit identical to
main.What profiling found. A single
updatetook ~550 µs whether it saw 1 label or 100. Over 90% of that went to scikit-learn'sIsotonicRegressionwrapper: fivecheck_arraycalls, dataframe sniffing, parameter validation, a sort of already-sorted bins, and building an interpolator, all around a PAV step on 20 numbers.scipy.optimize.isotonic_regression(SciPy's public PAV function, C++)np.digitize/np.interpexactly. Below 1,024 / 2,048 values the NumPy routines are used, because they're faster there.StreamingIsotonicCalibratornever imports scikit-learn.BatchCalibratorstores labels asuint8Changes after review:
compare_prequentialis back tomainexactly.BatchCalibrator. Evicting the oldest chunk was O(window) per update, and per-chunk overhead could exceed the data it held.Receipts: speed
python -m benchmarks.speed(new, reproducible),mainvs this branch on the same machine, BLAS pinned to one thread. Values are medians; the machine is shared, so expect ±20% noise.update, B=20, 1 labelupdate, B=1000, 10k labelscalibrate, B=100, 100 valuescalibrate, B=1000, 10k valuescalibrate, 1–100 valuesimport streamcalReceipts: space
import streamcal+ first updateBatchCalibratorhistory per observationStreamingIsotonicCalibratorstateCorrection. An earlier version of this description said peak memory was unchanged at 159 MB. That was a measurement error: on Linux a forked child's peak RSS starts from its parent's, and the benchmark had loaded scikit-learn before spawning its probes.
benchmarks/speed.pynow probes first.Receipts: identical outputs
mainand this branch in separate processes, and every output was compared bitwise. That's 1,199,512 values, 0 differences. It covers:compare_prequential: every score fieldtests/test_kernels.py(new) keeps this in CI:Compiled code and .pyc
Measured, not adopted:
.pyc: no measurable effect on import.maintook 5.2 s instead of 1.1 s. Production images should build withUV_COMPILE_BYTECODE=1.Validation
ruff check,ruff format --check,pyright src,pydoclint srcall clean.pytest: 228 passed.sphinx-build -Wpasses anduv buildsucceeds.🤖 Generated with Claude Code
https://claude.ai/code/session_011pXaJ5DLex7BrdUyfTqjL4