Repository navigation
Make tape recording thread-safe (fixes #218) - #311
Merged
Merged
Conversation
`InstructionTape` was a plain `Vector`, so recording from several threads
(e.g. `Threads.@spawn` inside the differentiated function) raced on `push!`.
On Julia 1.10 this corrupts the heap ("double free"), on 1.11+ it throws a
`ConcurrencyViolationError` or silently yields wrong gradients.
`InstructionTape` is now an `AbstractVector` wrapping the instructions and a
`SpinLock` that guards appending and emptying. Reads stay unlocked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The tape is an append-only log written by `record!`. Implement only the iteration interface plus `getindex`, `length` and `isempty`, so generic array functions (`copy`, `similar`, broadcasting, …) don't silently produce unlocked `Vector`s, and the custom `show` is also used for REPL display. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #311 +/- ##
==========================================
+ Coverage 89.18% 89.38% +0.19%
==========================================
Files 19 19
Lines 1933 1978 +45
==========================================
+ Hits 1724 1768 +44
- Misses 209 210 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Recording takes one atomic swap per instruction on a backward-linked list instead of a lock, so tasks recording concurrently don't wait for each other. `finish!` turns the list into the instruction vector; reading a tape that is still recording throws, and so does recording onto a finished tape, until `empty!` starts a new recording. The tape constructors finish their tapes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…compactly `record!` swapped out `Finished()` before throwing, so a rejected record put the tape back into the recording state. Restore it before throwing. The two-argument `show` of tapes and instructions now prints one line, and the verbose `text/plain` form of a tape is truncated like an array. Showing a tape that is still recording no longer throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Resolve the conflicts with the Runic reformat (#312) by keeping this branch's changes, formatting them with Runic 1.11.1, and ending functions with an explicit `return nothing` like master does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`empty!(NULL_TAPE)` (e.g. via `empty!(input.tape)` in `jacobian`) put the shared null tape back into the recording state, after which reading it threw. It now leaves `NULL_TAPE` unchanged. Drop `iterate` and `eltype` for `InstructionTape`: the passes, `compile` and `show` already read `instructions(tp)`, which checks the phase once. Compare the threaded gradients with `≈` instead of an absolute tolerance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devmotion
added a commit
that referenced
this pull request
Oct 2, 2026
Finishes the tape before counting its instructions in the elementwise tests, as recording and reading are now separate phases (#311). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devmotion
added a commit
that referenced
this pull request
Oct 2, 2026
…d, and finish tapes before reading them - `copyto!` into a 0-dimensional `TrackedArray` was ambiguous with `Base`'s 0-dimensional method instead of throwing the destination error. - Replace the tests of `trackresults` and `getpartial` with nestings of ForwardDiff inside and around the broadcast. - The broadcast tests finish their tapes before reading or replaying them (#311). - Name the replayed function `replayf` and clarify two comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devmotion
added a commit
that referenced
this pull request
Oct 22, 2026
* Put broadcasting on Julia's documented custom-broadcast interface
Almost all of ReverseDiff's broadcasting hung off entry points Julia never
documented as extension points, and the documented ones were unused: 2191
`Base.broadcast` overloads, 1777 `Base.broadcast!`, and a 2001-method
`materialize` table keyed on *any* package's style, against five `copy`
methods and no `copyto!`. Because `broadcast` was overloaded directly it
short-circuited the pipeline before a `Broadcasted` was ever built, so `f.(x)`
and `broadcast(f, x)` could diverge, which `Base` guarantees they cannot.
`TrackedStyle` becomes `TrackedStyle{N} <: AbstractArrayStyle{N}`, so it tracks
dimensionality and picks up `Base`'s scalar, tuple and `Unknown` precedence
rules for free. The `Base.broadcast` overloads and the `materialize` table are
deleted, making `f.(x)` and `broadcast(f, x)` the same computation by
construction. `copyto!` now throws a descriptive error instead of failing after
polluting the tape; `.=` with a tracked right-hand side has never worked.
`copy` no longer asks `Core.Compiler.return_type` what a broadcast will produce
in order to pick an implementation. The only question the argument types cannot
answer is whether a tracked value reaches `f` other than through the arguments,
such as a closure capturing one, and `mayhavetracked` already reports that.
What the output turns out to be is settled after the forward pass instead:
anything `f` hands back that is not a `Dual` does not depend on the tracked
arguments, so it carries no partials and is left off the tape. That covers a
`Bool` from a predicate, an `Int` from `exponent`, a non-`Real`, and a function
constant in its argument, with one test. The `:tracker` implementation goes
with it, along with three `@generated` functions.
`∇broadcast` seeds `Dual`s only for the tracked arguments. `DiffRules` gives
`besselj` a literal `NaN` partial for its order, and `NaN * 0.0` is `NaN`, so
dualizing an untracked argument poisoned every partial of the same `Dual`.
Narrowing the seeding also narrows the `Dual` to the number of tracked
arguments and removes the `SVector`/`DiffResults` machinery. The tag is now
derived once for the whole broadcast from the arguments that are actually
seeded, rather than per element from the element types.
Where `f`'s partials are known in closed form, `knownpartials` dispatches on the
operation and its arguments and the cache keeps a `Contract(op, args)` per
tracked argument instead of a `Dual` per element. The entry says that argument
collects `op(seed, args...)` from every element of the output's derivative,
with `args` naming broadcast arguments by position:
`+`, `-`, `identity` `identity` or `-`, applied to the seed alone
`*` `*` with the other argument
`/`, `\` `/` by an untracked denominator
Nothing is stored. A `Contract` is a zero-size type, and an argument a partial
names is read back through `instruction.input`, where `record!` has already
captured it, so the partial cannot go stale on a replay and an untracked array
is not copied a second time. Every `knownpartials` method annotates all of
`f`'s arguments, which is what makes naming one by position safe: those are
exactly the arguments `splitargs` keeps, in order, so an `f` that also receives
arguments held back from the tape matches no method. `op` meets the seed rather
than the partial, so `/` forms no reciprocal, which is how `sum(y ./ 1e-310)`
keeps the finite gradient that #298 gave it.
A tracked denominator keeps a `Dual` per element, its partial `-x/y^2` being no
argument of the broadcast, as does an argument whose shape is not the output's,
which could not be read at the output's own index.
Arrays of `TrackedReal` are differentiated like a `TrackedArray` rather than
traced scalar-wise: 64x faster on `x .* y` and 166x on a fused three-argument
expression, with the tape going from ~3n entries to 2.
Against master, with `gradient` and Chairmarks, best of five alternating runs:
case master.us branch.us t.ratio bytes.ratio
mulscalar_1k 2.41 2.19 0.91 1.00
mulscalar_10k 15.75 14.33 0.91 1.00
mulscalar_100k 159.58 144.62 0.91 1.00
addscalar_10k 15.62 14.13 0.90 1.00
addarray_10k 17.33 15.75 0.91 1.00
mularray_10k 21.58 18.83 0.87 1.00
twoscalars_10k 24.67 23.00 0.93 0.87
sinscalar_10k 207.21 88.46 0.43 1.00
exp_10k 51.75 53.25 1.03 1.00
exp_100k 530.96 535.50 1.01 1.00
`exp` keeps a `Dual` per element either way and is at parity; the spread across
runs is about 3%. Precompilation drops from 10.45s to 6.78s and load time from
2.61s to 2.39s, and `src` loses 417 lines.
Fixes #265: a 0-d broadcast collapsed to a single `ImmutableDiffResult`, whose
`length` was then taken.
Fixes #214: a scalar and a one-element-vector argument now agree, where the
scalar form raised a `ForwardOptimize` ambiguity.
Addresses #226: the counted methods on `broadcast`, `broadcast!` and
`materialize` go from 5969 to 0, and `Test.detect_ambiguities(ReverseDiff)`
from 877 to 581, with the 296 broadcast-related ones all gone. `map`, `hcat`
and `vcat` are untouched and still account for 4567.
`seed!`'s catch-all is narrowed to `::Real`: it silently did nothing for the
arrays and tuples that reach it from a broadcast whose output element type is
not `Real`, and `jacobian` now covers those outputs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Recurse into the element type when looking for hidden tracked values
`fieldcount(Vector{T})` is 0 on Julia 1.10 and 2 on 1.11, where `Array`
gained `ref` and `size`, so asking it whether an array argument could hide
a tracked value answered differently by version. On 1.10 that let
`x[1] * [[x[2], x[3]]]` reach `∇broadcast`, which seeded `x[1]` and left the
enclosing `Dual` to be contracted with the seed in the reverse pass, where a
`Float64` derivative cannot hold it (#140).
Ask the element type instead. An argument is seeded itself or elementwise, so
its own tracked values are in plain sight; below it nothing is seeded, so
recurse into what a container holds rather than ask about the container.
`istypeorclosure` never answered whether its input was a type or a closure,
and `mayhavetracked` is false for a `TrackedReal`, so both become
`mayhidetracked`, which is what they decide.
Also report an argument carrying a perturbation that the tape's derivative
type cannot hold, rather than letting it fail as a `MethodError` from the
reverse pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Cover the `TrackedStyle{Any}`, wrapper, `Type` and `Union` paths
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Leave `map` over a `TrackedReal` to `Base`
`map` iterates a scalar as a one-element collection, so the fused methods
had nothing to fuse, and their reverse pass and replay both threw.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Trace a broadcast with non-`Real` results scalar-wise
A `TrackedArray` holds only `Real`s, so `Complex` or `Tuple` results
were returned untracked and their partials silently dropped. The
inferred element type now sends such broadcasts down the fallback
before `f` is evaluated.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Let `map` stop at its shortest argument again
`map` over arrays of different lengths gives a shorter output, so the
reverse pass iterates over the output and its partials, not the input.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Claim only arrays whose element type fixes the derivative type
`getouttype` reads the derivative type off the element type, so a
`Vector{TrackedReal}` left it `Union{}`. Such arrays now keep `Base`'s
style and are traced with the scalar rules, as before. The
`TrackedArray` methods are subsumed by the `AbstractArray` ones.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Read a known partial's arguments once per reverse pass
`value` of an array of `TrackedReal`s or of an adjoint `TrackedVector`
maps over it, so calling it per element made the reverse pass
quadratic.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Drop a redundant `trackresults` method and two `_maxdim` methods
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Name the closed-form reverse pass `contract_increment_deriv!`
`diffresult_increment_deriv!` took either a partial's index or the
arguments a `Contract` names in the same position. The replay and the
tag check reuse the values `copy` already computed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Link the perturbation-confusion TODO to #45
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Handle uninstantiated broadcasts in `copy` (fixes tests on Julia 1.13)
LinearAlgebra 1.13 forwards a `Diagonal`'s broadcast to its diagonal and
calls `copy` on the result without instantiating it. With a tracked
diagonal, the `Broadcasted` has `TrackedStyle{1}` and no axes, which the
scalar `copy` method matched by its axes type, so it was indexed with
`CartesianIndex()` and threw a `BoundsError`. Decide the scalar route
from `axes` instead, which computes missing axes and still gives `()`
for `TrackedStyle{0}` and 0-dimensional `TrackedStyle{Any}`.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Strip `∇broadcast`'s `Dual`s in `value` (fixes tests on Julia 1.13)
`∇broadcast` now seeds its `Dual`s with a ReverseDiff tag, and `value` strips them as
it strips tracking. So a broadcast of `value`, fused or not, isn't recorded. On Julia 1.13
`map(value, ::Diagonal)` broadcasts over the diagonal, which used to keep `value` of a
tracked `Diagonal` tracked.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Expect `@skip` broadcasts to be untracked, as on master
Since `value` strips `∇broadcast`'s `Dual`s, `SkipOptimize` does too, so a broadcast of
a skipped function is neither differentiated nor recorded. This is what master does
with `broadcast(f::SkipOptimize, t::TrackedArray) = broadcast(f.f, value(t))`, and what
`map` and the scalar rules do.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Read argument values in the reverse pass only for known partials
`value` copies an array of `TrackedReal`s, so the `Dual` path allocated a
copy per argument on every reverse pass without reading it. Also make two
tests exercise what they describe: a `Ref` that shifts argument positions,
and a replay whose named argument is tracked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Include test files by relative path
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Index broadcast partials by tracked argument only
`trackedslots` also returns the position of each tracked argument, so known
partials, bounds and the reverse pass cover tracked arguments alone instead
of carrying `nothing` or `Val(0)` for untracked ones. Known partials are
given per argument, and the tag is keyed on the derivative type.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep tapeless tracked scalars constant and fuse constant tuples
`capture` records a `TrackedReal` without a tape as its value, so the
reverse pass recounting tracked inputs misaligned slots and bounds. The
cache now carries each tracked argument's position, slot and bound from
recording, and the reverse pass skips arguments captured untracked.
Tuple arguments are broadcast elementwise rather than handed to `f`
whole, and only fall back when an element is tracked or may hide one.
Also unroll the vestigial one-element `map` loops, simplify comments, and
make the elementwise test state which functions are left off the tape.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Unwrap structured matrices by dispatch instead of stripping `Dual`s in `value`
`value(::Dual)` for `∇broadcast`'s tag was global, so a broadcast nested in
another stripped the outer perturbation and returned a zero gradient. `value`
now unwraps `LinearAlgebra`'s wrappers of a `TrackedArray` by dispatch, since
`map` over a `StructuredMatrix` broadcasts, and a nested broadcast raises the
descriptive error again. This also replaces `mulargvalue`.
The broadcast tag is keyed on the seeded arguments' value type, as ForwardDiff
keys it, rather than on the derivative type, and `D` is computed where the
output is tracked. Partials are numbered left to right, and known partials
unwrap only the arguments they name.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Differentiate `map` w.r.t. an array of `TrackedReal`s next to a `TrackedArray`
A `ForwardOptimize` `map` over a `TrackedArray` and a plain array treated the
latter as constant, yet the reverse pass read its partial whenever its element
type admits `TrackedReal`, so `map(hypot, x, Real[...])` threw a `BoundsError`,
on master too. Such an array is now differentiated like a second
`TrackedArray`.
The `SkipOptimize` `map` methods taking a `TrackedReal` go: `Base.map` and the
scalar `@skip` rules give the same results and leave the tape empty.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Drop broadcast perturbations in `@skip` only, and unwrap only `Diagonal` by hand
`value` stopped stripping `∇broadcast`'s `Dual`s, which `@skip` relied on: a fused
`x .* @Skip(exp).(x)` differentiated `exp`. The tag wraps `BroadcastTag{F}` again and
`skipvalue` drops it, and nested tracking, at every order.
The rewrapping `value` methods mapped over whole parents, which LinearAlgebra leaves
`#undef` outside a triangle. The generic fallback handles every wrapper but `Diagonal`,
whose `map` broadcasts and would be recorded.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Record broadcasts by inferred type, keep static fallbacks static, and reject buried tracked values
Whether a broadcast was recorded depended on its recorded values: a branchy `f`
whose every element took a constant branch, such as `ifelse.(x .> 0, x, 0.0)`
at negative `x`, was left off the tape, and replays gave wrong gradients. The
decision now uses the element type `combine_eltypes` infers, and the results
are converted to it so that a replay can store the other branch.
The broadcast tag is keyed on every argument's type, so a `Dual` of an
enclosing differentiation on an untracked argument always makes a newer tag;
with an older one, forward-over-reverse threw "Cannot determine ordering".
Arguments with an abstract element type are checked element by element.
The fallback no longer copies values to find the untracked style, and a
`TrackedArray` of a static array is broadcast as a static array of its
elements, which crashed before. `@skip` drops every level of tracking from
arrays too.
A `Dual` wrapping a `TrackedReal` is a perturbation of an enclosing
ForwardDiff call that reached the tape, and was differentiated as a
constant. Such outputs now raise an error, promotion keeps them apart from
`TrackedReal`s rather than cutting them off the tape, and converting one to a
`TrackedReal` raises an error. Reverse-over-forward with such mixed arrays now
works.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Capture the broadcast tag by instance so Julia 1.10 infers the dualized call
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep static broadcast results writable, simplify `_mayhidetracked`, and check abstract argument elements without dispatch
- store the results of a static array via `similar`, which gives a `SizedArray` for a
non-isbits element type where `convert` gives an `MArray` that rejects `setindex!`
- replace the `@generated` `try`/`catch` around `fieldcount` with `isconcretetype`
- branch on `isa` instead of dispatching on every element of an abstractly typed argument
- stop pinning `MethodError`s of the internal `seed!`
- test that the cached partials have a concrete element type
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Reject 0-dimensional `TrackedArray` destinations, test tags end to end, and finish tapes before reading them
- `copyto!` into a 0-dimensional `TrackedArray` was ambiguous with `Base`'s
0-dimensional method instead of throwing the destination error.
- Replace the tests of `trackresults` and `getpartial` with nestings of
ForwardDiff inside and around the broadcast.
- The broadcast tests finish their tapes before reading or replaying them (#311).
- Name the replayed function `replayf` and clarify two comments.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Cover the remaining broadcast paths and tidy up the review leftovers
- Recurse into argument values only through tuples and arrays, so a
non-concrete tuple holding `nothing` no longer throws a `MethodError`
- Capture the broadcast tag rather than its type in the reverse pass
- Drop the `copyto!` methods for `DefaultArrayStyle`, which `.=` never reaches
- Restore master's `return nothing` in `reverse_mul!` and print
`TrackedReal`s without building a string first
- Test `NotTracked` scalars and arguments, the `Union` branch of
`checktags`, and `map` over skipped functions; use `gradient` for the
tapeless `TrackedReal` test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Catch `TrackedArray` broadcast destinations by type, and test the static fallback for `StaticArray`
`.=` hands `copyto!` the source's style, so an untracked source never reached the
`TrackedStyle` methods. A tracked static value is stored as an `MVector`, which the
fallback keeps.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Keep widened broadcast values on replay, drop the dead `Union` branch, and test `show`
`map` narrowed a widened broadcast's value buffer to the recorded values' type, so replaying
`t > 0 ? t : 0` at non-integer inputs threw an `InexactError`. `combine_eltypes` widens to
`Real`, never to a `Union`, so `checktags` loses its unreachable branch. The `nothing`-in-tuple
test now reaches `∇broadcast` instead of the scalar fallback.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Bound a widened broadcast's replayed values by `Real`
`ForwardDiff.valtype` unwraps only a concrete `Dual`, so a widened inferred element type
(`Any` on Julia 1.10, or a `Dual` with free parameters) became the value buffer's element
type, which rejected the values and hid the `DualMismatchError` of a nested tag.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Promote a broadcast's inferred result union instead of widening it
`Broadcast.combine_eltypes` widens `Union{Dual, Float64}` to `Real`, so a function
with a constant branch, such as `ifelse.(x .> 0, x, 0.0)`, cached boxed results
and tracked `Real` values. Promoting the union keeps concrete `Dual`s and
`Float64` values.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Say what handles mismatched shapes in the propagation header
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Sharpen the broadcast tests and say why a broadcast target may be untracked
Use asymmetric values for the tracked scalar factor, let the enclosing and the
held perturbation reach the gradient, pin the nested-broadcast error message,
read `value` without tracing, and test tracked values hidden in a closure or a
`Ref`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Unwrap `NotTracked` before a scalar broadcast and tidy the broadcast tests
A scalar broadcast passed a `NotTracked` argument to `f` still wrapped,
which master unwrapped. Rename the eltype-changing `writable` to
`replayable` next to its only caller, fix two comments, test through
`@forward`/`@skip` and allocations instead of internals, skip the replay
check for untracked results, and cover `.\` with a tracked denominator
and the replay of a known partial naming an array of `TrackedReal`s.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Let `setindex!` reject `TrackedArray` broadcast destinations and fill test gaps
A `.=` into a `TrackedArray` now fails in `setindex!`, whose error carries the
hint, rather than in a `materialize!` override that wrappers bypassed anyway.
The elementwise tests decide whether a result is tracked by inference, as the
implementation does, and known partials are tested with a variadic `+` and at
second order.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Take the scalar rules for perturbations the derivative cannot hold
A broadcast argument whose perturbation the derivative type cannot hold,
such as a `Dual` created inside the differentiated function, now takes
the scalar rules instead of throwing. Reverse-over-forward through a
broadcast therefore works, and forward-over-reverse raises the #45 error.
Also raise a descriptive error for output elements that are not numbers,
cover the abstract-inference path of the reverse pass, and name the
constant DiffTests function explicitly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Test the `besselj` `NaN` partial fused with a tracked factor
`besselj.(nu, y)` alone already passed on master; fused with `.* y`,
the untracked order's `NaN` partial shares a `Dual` with the tracked
partials and gave a `NaN` gradient there.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
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 #218.
Problem
InstructionTapewasVector{AbstractInstruction}, andrecord!calledpush!on it without synchronization. When the differentiated function spawns tasks (Threads.@spawn,@threads, …) that do tracked arithmetic, several threads push onto the same tape concurrently:reallocof the buffer, i.e.double free or corruptionas reported in double free crash with multi-threaded code only when using multiple threads #218, orUndefRefErrorduring the reverse pass.ConcurrencyViolationError("Vector can not be resized concurrently"), or silently wrong gradients.Fix
Lock-free recording.
record!wraps the instruction in a node and links it into a backward-linked list with a single@atomicswapon the tape'slastfield. The swap returns the predecessor, so there is no lock and no second atomic, and recording tasks never wait for each other.Recording and replay are separate phases.
finish!ends recording: it swapslastto aFinished()sentinel and moves the list into aVectoronce. After that:length, the passes, compiling) throws;record!seesFinished()as its predecessor at no extra cost, and restores it before throwing);empty!clears the tape and starts recording again, except forNULL_TAPE(the tape of untracked values), which never records and stays finished.The tape constructors (
GradientTape,JacobianTape,HessianTape) callfinish!, so the public API is unchanged. Code that records onto a bareInstructionTape(internal API) now has to callReverseDiff.finish!(tape)before replaying it.InstructionTapeis no longer anAbstractVector; it implements onlylength, and the recorded instructions are read withReverseDiff.instructions(tape), which checks the phase once.Some packages use this internal API directly: Lux.jl (ReverseDiff training extension), AbstractDifferentiation.jl (
derivative) and Gen.jl (backprop) record onto a bareInstructionTape()and callreverse_pass!or iterate it, so they will need afinish!call. Packages that go throughGradientTapeand friends (e.g. SciMLSensitivity.jl) are unaffected.Printing. The two-argument
showof tapes and instructions prints a single line (7-element InstructionTape,InstructionTape (recording),ScalarInstruction(+)). Thetext/plainform of a tape lists its instructions and is truncated like an array, instead of printing every instruction of a long tape; that of an instruction shows its input, output and cache.Why the order is still valid. The swaps put all recordings in one total order that respects each task's program order and every synchronization between tasks. Every rule records its instruction before its output becomes visible. So for each dependency, the instruction producing a value comes before the one consuming it: by program order within a task, and through the synchronization needed to hand the value over across tasks. That holds per dependency, so it doesn't matter whether a task produces, consumes or both. The same argument orders in-place writes against reads in race-free programs. The order isn't deterministic across recordings, so gradients may differ in the last bits.
Performance
Apple Silicon, minimum times unless noted. "64 tasks" is a function that spawns 64 tasks, each doing 200 tracked scalar ops on one tape (median of 100
gradientcalls).x*ygradientx*ygradientThe extra 32 bytes per op are the list node. They cost about 10% replay time for very large tapes on 1.10 (allocation layout) and nothing measurable on 1.13.
Alternatives I benchmarked and rejected:
Threads.SpinLockaroundpush!: 72 ns per op on 1.10, and 28 / 32 ms for 64 tasks at 8 threads on 1.10 / 1.13, i.e. slower than serial.ReentrantLock: about 2× slower thanSpinLockunder contention on 1.13.Base.Channel, with or without a consumer task: slower than a lock, sinceput!takes a lock itself, and a consumer task competes with the recorders for that lock.Vector): 1.3–2× slower replay due to pointer chasing.setfield!(the instruction type isn't fully inferred at the call site).record!::acquire_releaseperforms the same as the default, and:monotonicis 2.5× slower for 64 tasks at 8 threads on 1.10 (4.1–4.4 vs 1.6 ms, reproducible) and the same on 1.13. The contended swap costs the cache line transfer, not the fence.finish!counting the nodes, thenresize!and filling from the back (instead ofpush!+reverse!): about 2× slower on a 270k-instruction tape, since it walks the list twice.A shared tape still writes one cache line per op from all threads, so recording doesn't reach the speed of fully private per-task tapes (2.8 / 5.2 ms at 8 threads). I'll open a follow-up issue for that.
Tests
test/api/GradientTests.jl: compares the gradient of a function that spawns 64 tasks with a serial reference (repeated, plus a compiled tape). CI now runs withJULIA_NUM_THREADS=4so this is actually exercised.test/TapeTests.jlpins the phase rules (reading while recording throws, recording afterfinish!throws,empty!restarts,NULL_TAPEstays finished).take_recorded!intest/utils.jl).🤖 Generated with Claude Code