Skip to content

Give tracked reals with a free origin a zero and a one (fixes #172), and pin #126 - #302

Merged
devmotion merged 2 commits into
masterfrom
dmw/regressions-126-172
Sep 22, 2026
Merged

devmotion merged 2 commits into
masterfrom
dmw/regressions-126-172

Conversation

@devmotion

Copy link
Copy Markdown
Member

#301 triaged #126 and #172 as already passing on master. #126 does; #172 does not, so this branch pins the first and fixes the second.

#126 — test only

gradient(x -> x[], fill(1.0)) was ambiguous between the AbstractRange... and Colon... methods of getindex, because a bare vararg matches zero arguments. 4a50321 fixed it incidentally while fixing #202, by requiring a leading index. Nothing pinned it, and it would come back unnoticed with any future widening of those signatures.

#172 — fix and test

mapping over a tracked array mixes elements that carry an origin with ones that do not, and the resulting eltype leaves the origin free. reducedim_init allocates a reduction's accumulator by asking that eltype for its zero, or one for prod, which reached valtype — defined only for a pinned origin:

julia> ReverseDiff.gradient([1.0, 2.0]) do x
           sum(sum(map(i -> isodd(i) ? x[i] : 2 * x[i], eachindex(x)); dims = 1))
       end
ERROR: MethodError: no method matching valtype(::Type{TrackedReal{Float64, Float64}})

That is the element type in the issue's stack trace. Narrower spellings of "different types" (Real[], Any[]) already worked, which is most likely what produced the triage in #301.

Neither identity depends on the origin, so both are deducible from the type alone, as zero requires. Nothing is the origin zero has always used, and TrackedReal{V,D,Nothing} is an instance of TrackedReal{V,D}, so the documented zero(T) isa T holds. The signature matches that type exactly rather than its subtypes: a Union of two origins is also a subtype, and no single member of it could be returned.

The zero method is the one #173 added and #177 reverted wholesale over #175, whose cause — increment_deriv! indexing into untracked elements — was fixed by #300. one is its counterpart, which #173 never added.

Derivatives still route through the tape, not the origin: the accumulator is a fresh value, never an aliased array slot. Checked against ForwardDiff across randomized sum/prod reductions, jacobian and hessian, and on tape replay with a compiled tape — the committed test uses prod so that a replay reusing stale values would be caught.

detect_ambiguities(ReverseDiff) is unchanged at 877, none involving zero or one. Removing the two methods makes every new #172 assertion fail with the MethodError above, and leaves the #126 ones passing.

🤖 Generated with Claude Code

devmotion and others added 2 commits September 22, 2026 15:17
`gradient(x -> x[], fill(1.0))` threw a `MethodError` for an ambiguity between
the `AbstractRange...` and `Colon...` methods of `getindex`: a bare vararg
matches zero arguments, so `t[]` matched both equally.

4a50321 fixed it incidentally while fixing #202, by rewriting every
`getindex(t::TrackedArray, i::T...)` as `i1::T, is::T...`. Requiring a leading
index sends `t[]` down `Base`'s generic zero-index `AbstractArray` path, which
lands on the `Integer` method via linear indexing -- so, like any scalar
`getindex`, it aliases its parent through the origin and records nothing.

Nothing pinned that, and the ambiguity would come back unnoticed with any
future widening of those signatures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`map`ping over a tracked array mixes elements that carry an origin with ones
that do not, and the resulting eltype leaves the origin free. `reducedim_init`
allocates a reduction's accumulator by asking that eltype for its `zero`, or
`one` for `prod`, which fell through to `convert(T, 0)` and then to `valtype`,
defined only for a pinned origin:

    MethodError: no method matching valtype(::Type{TrackedReal{Float64, Float64}})

Neither identity depends on the origin, so both are deducible from the type
alone, as `zero` requires. `Nothing` is the origin `zero` has always used, and
`TrackedReal{V,D,Nothing}` is an instance of `TrackedReal{V,D}`, so the
documented `zero(T) isa T` holds. The signature matches that type exactly
rather than its subtypes: a `Union` of two origins is also a subtype, and no
single member of it could be returned.

The `zero` method is the one #173 added and #177 reverted wholesale over #175,
whose cause -- `increment_deriv!` indexing into untracked elements -- was fixed
by #300. `one` is its counterpart, which #173 never added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.93%. Comparing base (cd0a06d) to head (4146188).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #302      +/-   ##
==========================================
+ Coverage   86.92%   86.93%   +0.01%     
==========================================
  Files          19       19              
  Lines        1950     1952       +2     
==========================================
+ Hits         1695     1697       +2     
  Misses        255      255              

☔ 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.

@devmotion
devmotion merged commit 55159d7 into master Sep 22, 2026
7 of 8 checks passed
@devmotion
devmotion deleted the dmw/regressions-126-172 branch September 22, 2026 14:22
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.

1 participant