Skip to content

fix: avoid double dynamic dispatch in pick_batchsize with run-time batch size - #1076

Merged
gdalle merged 3 commits into
mainfrom
dmw/pick-batchsize-runtime
Oct 12, 2026
Merged

gdalle merged 3 commits into
mainfrom
dmw/pick-batchsize-runtime

Conversation

@devmotion

Copy link
Copy Markdown
Member

Fixes #1075.

BatchSizeSettings{B}(N) with a run-time B did two dynamic dispatches. This PR replaces it (and the Val{N} variant) with BatchSizeSettings(B::Int, N::Int), which computes all three type parameters first and constructs the concrete type once. Static-array calls still infer via constant propagation (existing @inferred tests pass).

before after
pick_batchsize(AutoForwardDiff(), N) 1.39 µs, 8 allocs 0.15 µs, 1 alloc
pick_batchsize(AutoForwardDiff(; chunksize = 1), N) 1.00 µs, 4 allocs 0.15 µs, 1 alloc
sparse jacobian! from the issue (n = 8) 1.96 µs 0.78 µs
sparse prepare_jacobian from the issue 1.81 µs 0.63 µs

(medians, Chairmarks)

🤖 Generated with Claude Code

@devmotion
devmotion requested a review from gdalle as a code owner October 5, 2026 14:14
devmotion added a commit that referenced this pull request Oct 5, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…batch size

Replace `BatchSizeSettings{B}(N)` and `BatchSizeSettings{B}(::Val{N})` with
`BatchSizeSettings(B::Int, N::Int)`, which computes all type parameters
before constructing the concrete type.

Fixes #1075

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devmotion
devmotion force-pushed the dmw/pick-batchsize-runtime branch from 92da381 to b1eb273 Compare October 5, 2026 14:15
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.21%. Comparing base (bef2881) to head (5922314).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1076      +/-   ##
==========================================
+ Coverage   97.40%   98.21%   +0.80%     
==========================================
  Files         143      143              
  Lines        8300     8236      -64     
==========================================
+ Hits         8085     8089       +4     
+ Misses        215      147      -68     
Flag Coverage Δ
DI 98.04% <100.00%> (+0.13%) ⬆️
DIT 98.67% <ø> (+2.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gdalle gdalle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!
Can we fix the missing coverage too?

Comment thread DifferentiationInterface/src/utils/batchsize.jl
devmotion and others added 2 commits October 5, 2026 20:42
…pick_batchsize`

Move the `singlebatch`/`aligned` computation into a foldable helper
`batchsize_flags`, used by the run-time constructor and directly by the
StaticArrays extension, so static-array inference relies on concrete
evaluation rather than const-prop heuristics.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
All arguments are constants in the StaticArrays case, which forces
const-prop regardless of method size, and the flag computation is
loop-free foldable arithmetic, so the helper gave no observable
inference benefit on Julia 1.10, 1.12 or 1.13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devmotion

Copy link
Copy Markdown
Member Author

Added direct pick_batchsize tests for the HyperHessians extension (6f2056b); those lines were never reached through the Hessian path.

@gdalle gdalle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Docs failure is unrelated

@gdalle
gdalle merged commit 16b8302 into main Oct 12, 2026
208 of 225 checks passed
@gdalle
gdalle deleted the dmw/pick-batchsize-runtime branch October 12, 2026 08:59
gdalle pushed a commit that referenced this pull request Oct 13, 2026
…ic arrays (#1078)

Follow-up to #1076: lock in that `pick_batchsize` and `prepare_jacobian`
infer the batch size from the type for `SVector`, `SMatrix` and `MVector`,
with adaptive and fixed batch sizes. Mark multi-batch `jacobian` inference
as broken, since `mapreduce(hcat, ...)` changes the static size at every step.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pick_batchsize spends ~1 µs constructing BatchSizeSettings{B}(N) with a run-time B

2 participants