Exploit symmetry in the Hessian - #844
Open
devmotion wants to merge 14 commits into
Open
Conversation
instead of relying on the jacobian of gradient for the hessian explicitly seed dual numbers and only calculate the upper triangular part when chunking, gives ~2x speedup as input length becomes big
Carries #837 unmodified. Its two commits are already master-based -- the merge base is v1.4.5 and master's only newer commit touches test/QATest.jl and Project.toml, neither of which #837 changes -- so this merge is clean and their content and authorship are preserved exactly. The commits that follow are #843's follow-ups, rebuilt on master instead of on #840.
`HessianConfig` wrapped a `JacobianConfig` and a `GradientConfig` because the Hessian
was `jacobian(gradient(f), x)`: the outer sweep seeded the Jacobian config's buffer and
the inner `gradient` seeded the gradient config's. The symmetric sweep seeds both layers
of the one nested buffer, so it reads three things -- the two seed tuples and the nested
buffer -- and never touches the Jacobian config's buffer again after the constructor
derives the nested element type from it.
Holding those three directly drops the dead buffer. At `length(x) == 1000` and a chunk
size of 12:
HessianConfig(f, x) 1.422 MB -> 1.312 MB
HessianConfig(f, result, x) 1.531 MB -> 1.312 MB
The result-aware constructor allocated two dead buffers rather than one, since it built
the `f!(y, x)` `JacobianConfig`. Nothing about the work buffers depends on `result`, so
it forwards to the plain constructor and the two now return the same type -- which is
what the tests asserting the two configs interchangeable already implied.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diagonal blocks read `outer <= inner`, off-diagonal blocks the other triangle, and the StaticArrays path reads `outer <= inner` throughout. Reading an entry with i in the outer layer rounds differently from reading it with j there, so the result was not reproducible: at `n = 16` and `f = log(sum(exp, z))`, chunk sizes 1, 2, 3, 5, 7 and 11 each differed from chunk 16 and from the `SVector` path by ~3e-18. Swapping which layer block q carries fixes it. Same number of evaluations and seed writes, and q is still seeded once outside the loop; afterwards every chunk size is bitwise identical to the `SVector` path. `log(sum(exp, z))` is the objective in the new test because its mixed partials actually round differently in the two orders -- `sum(z)^3`, `exp(sum(z))`, `prod(z)` and `sum(sin, z) * sum(cos, z)` all give bitwise equal results either way, so none of them would have caught this. It also makes the existing `symmetric_static == hessian(symmetry_f, x)` assertion robust rather than accidental: that only passed because `n = 9` is below DEFAULT_CHUNK_THRESHOLD, so the array path ran a single block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`seed_hessian_chunk!` materialised both zeros even when both seeds were supplied. Free
for an isbits value type, but for `BigFloat` at `N = 3` it cost 288 bytes per call,
identical for all four seed combinations. Each `=== nothing` is a compile-time constant,
so deciding per layer folds away:
iseeds oseeds before after
given given 288 0
given - 288 192
- given 288 96
- - 288 288
New coverage: the extension's `reshape` branch and its two `HESSIAN_ERROR` throws, and
empty inputs on both paths. Also notes why the `Partials{0}` method of `extract_hessian`
is load-bearing -- for a constant `f` the generic method would build a `0 × length(x)`
result, not `length(x) × length(x)`.
Not added: the mixed seed forms in `SeedTest`. Which layer a seed lands in is enforced by
the types -- `oseeds` only fits the outer `Dual` -- and both mixed forms run in every
multi-block sweep, so the bitwise chunk-size test covers them with a failure mode that a
unit test would only relocate. This is unlike `seed_zero_partials!`, whose testset exists
because over-clearing is invisible through the public API.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
For a structured input the result mixed two coordinate systems: rows were linear indices
of `x`, columns were structural positions, so `hessian(f, UpperTriangular(3x3))` was 9x6
and a caller could not say what `H[4, 5]` referred to without reading
`structural_eachindex`. Chunk mode did not get that far, throwing a `DimensionMismatch`
from `reshape_jacobian`, and `hessian!(DiffResults.HessianResult(x), f, x)` failed to
broadcast.
Both axes are now the linear indices of `x`, with hard zeros in the rows and columns of
the structural zeros. That is the smaller change relative to master, which already used
linear indices for the rows and only got the columns wrong, and it makes
`DiffResults.HessianResult(x)` the right buffer, since it allocates `length(x)^2`:
size(hessian(f, UpperTriangular(randn(3, 3)))) # 9x9, rows/cols 2, 3, 6 zero
size(hessian(f, Diagonal(randn(n)))) # n^2 x n^2
The cost is `Diagonal`, whose Hessian is now quartic in the size of the diagonal;
differentiating with respect to the diagonal vector is the better choice there.
`structural_linearindices` gives the positions of `structural_eachindex` as linear
indices, computed once per sweep. `Base.OneTo` and a `StepRange` for the dense and
`Diagonal` cases, so those stay allocation-free; the triangles build one position vector
per call, against a sweep that evaluates `f` B(B+1)/2 times. Its two-argument form takes
the config's buffer and the input, and performs the size check `structural_eachindex`
performs for seeding, which the sweep no longer goes through.
`extract_hessian_gradient_chunk!` no longer delegates to `extract_gradient_chunk!`, which
takes its positions from the result rather than from `x` (#838): `HessianResult` hands
back a dense `size(x)` gradient buffer even for a structured `x`, so delegating scattered
the derivatives into the first `structural_length(x)` linear positions.
Seeding a structured buffer now walks it by linear index rather than by `CartesianIndex`,
which is also what lets a position double as a Hessian row. `Base._unsetindex!` is
implemented for `Array` alone -- for a linear index its `AbstractArray` fallback recurses
forever, where a `CartesianIndex` merely had no method -- so the unassigned-entry path
goes through a wrapper that raises an `ArgumentError` naming the entry instead. That also
removes a pre-existing `StackOverflowError` for a `Diagonal` of a non-bits element type,
whose positions `structural_eachindex` already gave as linear indices.
Not addressed here, and unchanged from master: `gradient!`/`jacobian!` extraction (#838,
#839) and detecting a config reused across structures (#842).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The symmetric sweep writes the result entry by entry, which lost the validation that
`reshape_jacobian` and the broadcast in `extract_jacobian!` used to provide. A vector
result was still checked by `reshape`, a matrix one no longer was:
hessian!(fill(NaN, 4, 4), f, rand(3)) # no error, row/col 4 left NaN
hessian!(fill(NaN, 4, 4), f, SVector(1., 2., 3.))
`reshape_hessian` mirrors `reshape_jacobian`, down to its `DiffResult` method, so the
`DiffResult` path is checked too -- it holds a buffer no entry point ever passes to
`require_one_based_indexing`, hence the extra call here. The non-matrix method checks the
length itself rather than leaving it to `reshape`, whose message names neither the Hessian
nor the input.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #844 +/- ##
==========================================
+ Coverage 90.68% 92.16% +1.48%
==========================================
Files 11 11
Lines 1052 1200 +148
==========================================
+ Hits 954 1106 +152
+ Misses 98 94 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`check_structural_size` and the `_unsetindex!` fallback were the only lines of the sweep left untested: the first needs a `HessianConfig` reused across sizes, the second a work buffer that is not an `Array`, so the existing `BigFloat` test with a `Vector` takes the `Base._unsetindex!` branch instead. Every `@test_throws` the sweep added asserted only the exception type. `HESSIAN_ERROR` is new here, but master threw `DimensionMismatch` at the same calls with the inner gradient's message, so those assertions passed on master too. They now match the message, with the type prefix so the type stays pinned. The chunk size guard named `ForwardDiff.structural_length`, which is internal.
`reshape_hessian` restored the check the sweep lost for the Hessian buffer, but the gradient
buffer of a `DiffResult` was still written unchecked. Master validated it indirectly: the
`f!(y, x)` `jacobian!` called `require_one_based_indexing` on it, and `reshape_jacobian`
compared the Hessian's row count against `length(ydual)`, which followed the buffer.
hessian!(DiffResult(0.0, fill(NaN, 4), fill(NaN, 3, 3)), f, rand(3))
# was: no error, gradient == [g1, g2, g3, NaN]
hessian!(DiffResult(0.0, fill(NaN, 2), fill(NaN, 3, 3)), f, rand(3))
# was: BoundsError naming neither the gradient nor the input
hessian!(DiffResult(0.0, fill(NaN, 4, 3), fill(NaN, 9, 9)), f, UpperTriangular(randn(3, 3)))
# was: no error, the six derivatives written to linear positions 1,4,5,7,8,9 of a 4x3 buffer
The last one is why this matters: silently wrong values in a buffer the caller believes was
filled, and reachable only because indexing the Hessian by the linear indices of `x` made the
`DiffResult` path work for structured inputs at all.
`structural_eachindex(grad, x)` is the check the seeding utilities already apply to a work
buffer, so no new helper is needed and `require_one_based_indexing` comes with it. It compares
linear indices where both arrays are `IndexLinear`, which keeps a flat gradient buffer for a
matrix `x` working as it did on master -- `length` is the requirement, not `size`, since the
sweep writes by linear index of `x`.
The new `@test_throws` assert the type alone: the message is Base's, not ours.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`structural_linearindices` returned a comprehension over a flattened generator for the two
triangular wrappers, so the vector grew by `push!`: 512 bytes per sweep at n = 6, where the
vector itself is 168. Writing into a `Vector{Int}` of the right length is one allocation of
that length, and carrying the linear index along the columns -- advancing by `n - j` for the
upper triangle and by `j` for the lower at the end of column j -- drops the multiplication as
well.
n = 6 512 -> 224 bytes
n = 20 6832 -> 1840 bytes
Dense inputs and `Diagonal` are unaffected, both still allocation-free.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`(sum(abs2, z) + sum(z)^2) / 2` has second derivative `1 + (a == b)` on the structural entries, so `expected` was uniform off the diagonal and any bijection of the structural positions maps the diagonal to the diagonal: the Hessian assertion was invariant under every permutation of them, and could not catch a wrong row or column in the off-diagonal blocks -- the one thing the structured testset exists to check. Only the gradient assertion pinned the order, and the gradient comes from the diagonal blocks alone. `dot(w, z)^2 / 2` has second derivative `w[a] * w[b]`, and `w * transpose(w)` for distinct `w` is reproduced by no permutation, since it would need `P * w == w`. With `w` the small integers on the structural positions and zero elsewhere the entries stay exactly representable, so the assertions remain `==` rather than `isapprox`, and `dot(w, x)` reads the same linear indices the sweep writes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The PR's headline is that `hessian!` into a preallocated matrix allocates nothing, and nothing in the suite held it: `hessian_allocs()` from #720 covers the StaticArray `ImmutableDiffResult` path only. The sweep is allocation-free for a dense input because `structural_linearindices` returns a `Base.OneTo` and `structural_chunk` a `UnitRange` rather than a view; either of those turning back into an array would show up here and nowhere else. Both chunk shapes are covered, since a partial final block takes different branches: n = 40 with chunk 6 gives seven blocks with a four-wide last one, n = 10 with chunk 10 a single block. The seeding testset has covered `seed_hessian_chunk!` since the sweep landed, so its name no longer described it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`HessianConfig` handed one `Tag(f, V)` to both layers of `Dual{T,Dual{T,V,N},N}`,
leaving them type-indistinguishable. `value`/`partials` inside `f` could not tell
them apart, the ordering machinery had nothing to compare, and the result depended
on the code path and the chunk size (#845).
The outer tag is now derived from the inner one as `outer_tag(T, Dual{T,V,N})`, and
registered after it so that `T ≺ TO`. A result that carries only the inner layer
then reads as a gradient with a vanishing Hessian, with no special casing, and the
array path keeps the gradient master produced for such an `f`.
Blocks whose derivatives the result cannot carry are no longer evaluated, and
`extract_hessian` dispatches on the result rather than on its partials, which no
longer matches a method when the result carries only an enclosing tag (#846).
Zeros written for a derivative that vanishes take their element type from `ydual`,
as `extract_gradient!` does, so both paths treat a plain buffer alike.
Fixes #845. Fixes #846.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #253. Fixes #836. Fixes #845. Fixes #846.
Relation to #837, #840 and #843
x, and store those indices in the config #840, whose structured-array design is still under discussion.x, and store those indices in the config #840. Same sweep, same fixes.gradient,jacobian,GradientConfigandJacobianConfigare untouched, andgradient!writes to the wrong entries for structured inputs #838, Chunkedjacobianthrows forDiagonal/LowerTriangular/UpperTriangularinputs #839 and Reusing aGradientConfig/JacobianConfigwith a differently structured input silently computes wrong derivatives #842 are out of scope — reusing a config across structures stays exactly as wrong as on master.kc/symmetric_hessianis untouched.Numbers
Suite 9759/9759.
hessian!into a preallocated matrix, best of 200, Julia 1.12.7:Over 4 functions ×
n ∈ {1,2,3,5,8,13}× chunk sizes × 4 result kinds, 372 of 432 results are bitwise identical to master. The 60 that differ are exactly the 60 master returned asymmetric, by at most one ulp.#840's seeding speedup, confined to the Hessian.
seed_hessian_chunk!takes its window from stored positions rather than walkingIterators.drop, so its cost stops growing withn. Seeding the final chunk:xVectorn=40 / n=200UpperTriangular13² / 25²Diagonal40 / 200End-to-end that is 1.15x at
n = 40and 1.21x atn = 80withchunk = 2, and nothing atchunk = 12. #840's owngradient!/jacobian!gains are not included — those paths are untouched here.Fixes
n = 16withlog(sum(exp, z)), chunks 1, 2, 3, 5, 7, 11 each differed from chunk 16. Every chunk size is now bitwise equal to the StaticArrays path.hessian!stopped checking the result shape when the sweep replacedextract_jacobian!:hessian!(fill(NaN, 4, 4), f, rand(3))silently left row/column 4 untouched.reshape_hessianrestores it.hessian!(::DiffResult, …)stopped checking the gradient buffer, which master validated throughreshape_jacobian's row count. Worst case was silent and wrong rather than incomplete: with a4×3buffer for anUpperTriangular(3, 3)input the six derivatives went to linear positions 1, 4, 5, 7, 8, 9 of a four-row array.structural_eachindex(grad, x)— the check the seeding utilities already apply to a work buffer — restores it, and still accepts a flat gradient buffer for a matrixxas master did.jacobian(gradient(f), x)and was not exactly symmetric either.hessian(prod, ::SMatrix{3,3}): 195 ns → 61 ns.HessianConfighanded oneTag(f, V)to both, sovalue/partialsinsidefcould not tell them apart and the ordering machinery had nothing to compare.f(z) = sum(abs2, z) + ForwardDiff.value(z[1]) * z[2]at[1.0, 2.0, 3.0]gave master three different Hessians and two different gradients depending on the path and the chunk size — and the chunk-size reproducibility above does not hold for such anfunder one tag. The outer tag is now derived from the inner one asouter_tag(T, Dual{T,V,N}), registered after it so thatT ≺ TO. A result carrying only the inner layer then reads as a gradient with a vanishing Hessian, with no special casing.extract_hessiandispatched onpartials(T, ydual)(hessian!(::ImmutableDiffResult, f, ::StaticArray) errors when f does not depend on its argument #846), which is aPartialsonly when the result carriesT. For a result carrying only an enclosing tag it is a bareDualand matched no method, soForwardDiff.derivative(a -> ForwardDiff.hessian(z -> a * 2.0, SVector(1.0, 2.0, 3.0))[1,1], 1.0)errored. Dispatch is now on the result itself, which also subsumes thePartials{0}shortcut for a constantfand an emptyx. Zeros written for a derivative that vanishes take their element type fromydual, asextract_gradient!does, so the array and StaticArrays paths treat a plain result buffer alike.fwhose result has no outer perturbation the off-diagonal blocks are known to be zero, so evaluations drop fromnblocks(nblocks+1)/2tonblocks, and to one when no gradient is requested.Breaking
Structured inputs return
length(x) × length(x), both axes the linear indices ofx, zeros in the rows and columns of the structural zeros.hessian(f, UpperTriangular(rand(3,3)))is 9×9 with rows/columns 2, 3, 6 zero.Master returns 9×6 — linear rows, structural columns — which cannot be indexed without reading
structural_eachindex; chunk mode throwsDimensionMismatch; andhessian!(DiffResults.HessianResult(x), f, x)fails to broadcast. The new convention is the smaller change (master already had linear rows) and makesHessianResult(x)the right buffer. The cost isDiagonal, now quartic in the diagonal's size:hessian!forDiagonal(40)spends its time on a 1600×1600 result rather than a 40×40 one.HessianConfigholdsiseeds,oseeds,dualsinstead of aJacobianConfigand aGradientConfig, and carries the outer tag, soHessianConfig{T,V,N,DG,DJ}becomesHessianConfig{T,TO,V,N,D}andeltypebecomesDual{TO,Dual{T,V,N},N}— any code that writes the type out, not just code that reads the fields, has to change.Tstays first and stays the inner tag, socfg::HessianConfig{T}signatures keep working. The sweep never reads the Jacobian config's buffer.HessianConfig(f, x): 1.422 → 1.312 MB atlength(x) == 1000; the result-aware constructor 1.531 → 1.312 MB. Separate commit, easy to drop.Both of these need a 2.0.
Results change for an
fthat reaches into the layers of its argument, which is the point of the #845 fix rather than a cost of it.ForwardDiff.value(z[1])now yields the inner-layer value instead of one the outer layer re-absorbs, so the example above gives[4.0, 5.0, 6.0]— what master's array path returned — from every path and every chunk size, where master's StaticArrays path gave[2.0, 5.0, 6.0]. Anfthat does not reach into layers is bitwise unchanged: overDiffTests.VECTOR_TO_NUMBER_FUNCS× chunk sizes 1, 2, 3, 5, all 60 results are identical to the one-tag computation.Implementation notes
structural_linearindicesgivesstructural_eachindex's positions as linear indices, computed once per sweep rather than cached on the config —OneTo/StepRangefor dense andDiagonal(allocation-free), one small vector per call for the triangles.Seeding a structured buffer now walks it by linear index, which is what lets a position double as a Hessian row.
Base._unsetindex!exists forArrayalone — for a linear index itsAbstractArrayfallback recurses forever, where aCartesianIndexmerely had no method — so the unassigned-entry path now raises anArgumentErrornaming the entry. That also removes a pre-existingStackOverflowErrorfor aDiagonalof a non-bits element type.Beyond the suite: 3 wrappers ×
n ∈ {1,2,3,5,8}× every chunk size × 4 result kinds against a closed-form reference — 1269 checks, all passing.🤖 Generated with Claude Code