Repository navigation
Conversation
…ic arrays 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>
… arrays Compute all batch blocks with `map` and concatenate them once with `reduce(hcat/vcat, ...)` via new internal hooks `stack_batches_col` and `stack_batches_row`, instead of `mapreduce(hcat/vcat, ...)` with an in-loop slice of the last batch. The StaticArrays extension overloads the hooks to concatenate a statically sized tuple of blocks, so that multi-batch Jacobians and Hessians of static arrays are inferred and return static matrices. For dense arrays this avoids the repeated reallocation of pairwise `hcat`/`vcat` (e.g. 10x faster, 5x less memory for a 1000x1000 Jacobian with chunksize 12). Also remove the unreachable unaligned branch in the single-batch Jacobian methods: a single batch is always aligned. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1080 +/- ##
==========================================
+ Coverage 98.21% 98.27% +0.05%
==========================================
Files 143 143
Lines 8246 8274 +28
==========================================
+ Hits 8099 8131 +32
+ Misses 147 143 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…-assembly Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gdalle
left a comment
There was a problem hiding this comment.
Thanks, this looks like a worthwhile improvement! A few things to iron out though
- split `aligned ? M : M[...]` into if/else blocks for coverage - drop the unused `B_last` field from `BatchSizeSettings` - route the B=1 Jacobian methods through `stack_batches_col/row` too, so they also return static matrices for static arrays Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
gdalle
left a comment
There was a problem hiding this comment.
Thanks! I'm good with the changes to the source code, a few more tests and this will be good to go!
| test_differentiation( | ||
| [ | ||
| AutoSimpleFiniteDiff(; chunksize = 2), | ||
| AutoReverseFromPrimitive(AutoSimpleFiniteDiff(; chunksize = 2)), | ||
| ], | ||
| filter(s -> DIT.operator(s) in (:jacobian, :hessian), static_scenarios()); | ||
| logging = LOGGING, | ||
| ) |
There was a problem hiding this comment.
Can we add non-regression type stability tests here, possibly filtering the in-place scenarios?
There was a problem hiding this comment.
I added JET checks (type_stability = :prepared) over all static Jacobian and Hessian scenarios, in-place and out-of-place, for both multi-batch backends. They use chunk size 2, so they cover the aligned path. All four batch methods (B = 1, aligned, unaligned, single batch) are covered by a new @inferred matrix in test/Core/Internals/batchsize.jl: pushforward and pullback Jacobians (out-of-place and in-place, real and complex) and Hessians.
Every JET check passes except the in-place Hessian with AutoReverseFromPrimitive, which fails on main too and is marked broken = true. On main, 44 of these 96 checks fail; with this PR, only those 4 do.
The JET checks are guarded with if JET.JET_AVAILABLE rather than safetypestab, since aviatesk/JET.jl#699 and aviatesk/JET.jl#742 are both closed. Locally, 1.12 with JET 0.12.3 gives the same results as 1.11. The guard has to wrap the whole block, because JET's stubs ignore skip.
These checks exposed a regression in this PR: map over the blocks failed to infer when the block function itself calls map. Fixing it changed the hook you approved: stack_batches_col/row(block, batch_size_settings, seed_example) now takes the block function. The generic method collects the blocks with a comprehension; the StaticArrays method uses ntuple, with no intermediate vector.
A new vec_block uses similar_type, so matrix-shaped static inputs no longer produce SizedMatrix{Matrix}; SMatrix pullback Jacobians and Hessians are now MMatrix. On 1.11, all 224 prepared combinations I benchmarked infer and return the same results as main. Static inputs allocate the same as on main or less; Vector inputs allocate 2 more, for the block vector.
| ) isa DI.BatchSizeSettings{5} | ||
| end | ||
|
|
||
| @testset "Static Hessian with multiple batches" begin |
There was a problem hiding this comment.
Can we test the Jacobian here too?
There was a problem hiding this comment.
I added "Static Jacobian with multiple batches". It covers pushforward and pullback Jacobians, out-of-place and in-place, at chunk sizes that hit every batch method. It goes through AutoForwardFromPrimitive / AutoReverseFromPrimitive, because a plain AutoForwardDiff calls ForwardDiff.jacobian directly. The Hessian testset now also covers the single-batch case (chunk size 6).
Pass a block function to `stack_batches_col/row` instead of a vector of blocks. The generic method collects the blocks with a comprehension, since `map` fails to infer when the block function calls `map` itself (e.g. pullbacks via pushforwards). The StaticArrays methods build the blocks with `ntuple` and allocate no intermediate vector. Flatten blocks with `vec_block`, which uses `similar_type` for static arrays: `vec` of an `MMatrix` is a `SizedVector` view, and concatenating those allocates a `SizedMatrix` backed by a `Matrix`. Tests cover all batch methods (B = 1, aligned, unaligned, single batch) for pushforward and pullback Jacobians (out-of-place and in-place, real and complex) and Hessians, with inference checks, a ForwardDiff Jacobian through `AutoForward/ReverseFromPrimitive`, and JET checks over the static DIT scenarios. The in-place Hessian with a pullback-based inner gradient is marked broken, as it already failed to infer before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stacked on #1078.
Replaces
mapreduce(hcat/vcat, ...)in the multi-batch Jacobian and Hessian paths with new internal hooksstack_batches_col/row(block, batch_size_settings, seed_example), which take the function computing the block of each batch:reduce(hcat/vcat, ...). A comprehension rather thanmap, sincemapfails to infer when the block function callsmapitself.ntupleof blocks, with the number of batches deduced from the seed type. It also overloads the newvec_blockwithsimilar_type, so that flattened blocks of e.g. anMMatrixareMVectors and the result is anMMatrixinstead of aSizedMatrixbacked by aMatrix.Multi-batch Jacobians and Hessians of static arrays are now inferred and return static matrices (turning #1078's
@test_brokeninto passing tests). Tests cover all batch methods (B = 1, aligned, unaligned, single batch) for pushforward and pullback Jacobians (out-of-place and in-place, real and complex) and Hessians, plus JET checks over the static DIT scenarios. The in-place Hessian withAutoReverseFromPrimitivealready fails JET onmainand is marked broken.Why
reduceinstead ofmapreducereduce(hcat, ::AbstractVector{<:AbstractVecOrMat})(andvcat) has a specialized Base method that allocates the result once.mapreduce(f, hcat, v)has no such method and falls back to pairwisehcat, re-copying the partial result at every level.Concatenating
Ablocks of sizem × B(Chairmarks):m× (A×B)mapreduce(f, hcat, v)reduce(hcat, map(f, v))End-to-end, prepared
jacobian(identity, prep, backend, x)with densexandchunksize = 12(Julia 1.11):length(x)AutoSimpleFiniteDiff, 100AutoSimpleFiniteDiff, 1000AutoReverseFromPrimitive(AutoSimpleFiniteDiff), 1000(
stackwas also considered: equivalent for columns, but rows need an extrapermutedimscopy.)🤖 Generated with Claude Code