feat: n_jobs for threaded slice aggregation#114
Conversation
Slices share no state, so the loop parallelizes trivially. Measured honestly, the gain is modest today: ~10-13% with 2+ threads and flat beyond, because the hot paths (duration representation, accuracy) are GIL-holding pandas/sklearn loops. A process pool is net negative (spawn + imports + pickling cost more than the parallelism saves). Threads become the right backend once the upstream loops vectorize (FBumann/tsam#49, #50) and release the GIL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Not worth it |
BenchmarksΔ% vs base, one table sorted by biggest change first Full tablebenchmarks/test_bench_aggregate.py
|
Second of the three perf drafts — the parallel-slice experiment, with honest numbers. Supersedes the closed #46 (same design, remeasured on the current stack). Closes #12 if merged.
What
aggregate(..., n_jobs=)threads the per-slice loop:None/1sequential (default, unchanged),-1all CPUs,-2all but one, positive N exact (joblib convention), capped at the slice count. Slices share no state; results are identical to sequential (asserted intest/test_parallel.py).Measured (8 CPUs, min of 3)
~10–13%, flat beyond 2 workers — the hot paths (duration representation, accuracy metrics) are GIL-holding pandas/sklearn loops, so threads mostly wait. A
ProcessPoolExecutorvariant was measured too and is net negative (4.4 s vs 3.9 s sequential — worker spawn/imports + pickling exceed the gains), which also reproduces the conclusion from #46's description and is why this PR does not offer a process backend.The old #46 claimed 2×; that does not reproduce on the current stack/config.
Verdict / recommendation
Parallelism is not the lever today — the same GIL-holding loops filed as FBumann/tsam#49 and FBumann/tsam#50 are the reason. Once those vectorize (numpy releases the GIL), this exact thread pool should scale near-linearly, and the API is already shaped for it. Options: merge now for the modest win + future-proofing, or keep this draft parked until the upstream fixes land and remeasure. Draft until that call is made.
🤖 Generated with Claude Code