From 232d36d52e1ebed7b13aa87b2c5c4dd529c62a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller-Widmann?= Date: Thu, 6 Aug 2026 11:29:28 +0200 Subject: [PATCH] Fix ternary `hypot` flattening `Dual`s with different tags All seven bodies of `hypot`'s `@define_ternary_dual_op` delegated to a single helper that unwrapped every argument with the one-argument `value`/`partials`, irrespective of its tag. Arguments with an inner tag were therefore flattened and their partials summed into the tag selected by dispatch, which annihilates their perturbations and pollutes the surviving tag's partials. Give each case its own body that only unwraps the arguments known to carry the tag, as is done for `fma` and `muladd`. Since `hypot` is symmetric, two helpers plus permutations of their arguments cover all seven cases. Because the tag is now read off the helpers' signatures instead of being passed as a `::Type{T}` argument, ternary `hypot` also starts working for non-`Type` tags such as `Dual{:t}`, which previously threw a `MethodError`. The tests cover each of the seven bodies twice over: once with the non-tagged arguments as plain `Real`s and once with them carrying an inner tag. Fixes #834 Co-Authored-By: Claude Opus 5 (1M context) --- src/dual.jl | 37 ++++++++++++++++++++++++++----------- test/DualTest.jl | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/dual.jl b/src/dual.jl index a91444b6..17fb7056 100644 --- a/src/dual.jl +++ b/src/dual.jl @@ -599,24 +599,39 @@ end # hypot # #-------# -@inline function calc_hypot(x, y, z, ::Type{T}) where T - vx = value(x) - vy = value(y) - vz = value(z) +# Only the arguments that carry the tag `T` may be unwrapped with `value`/`partials`. +# The remaining ones are constants with respect to `T`, and since `Dual <: Real` they +# are simply passed on to the recursive `hypot` call, which keeps the perturbations of +# their (necessarily inner) tags nested inside the returned `Dual{T}`. +@inline function calc_hypot_xyz(x::Dual{T}, y::Dual{T}, z::Dual{T}) where T + vx, vy, vz = value(x), value(y), value(z) h = hypot(vx, vy, vz) p = (vx / h) * partials(x) + (vy / h) * partials(y) + (vz / h) * partials(z) return Dual{T}(h, p) end +@inline function calc_hypot_xy(x::Dual{T}, y::Dual{T}, z::Real) where T + vx, vy = value(x), value(y) + h = hypot(vx, vy, z) + return Dual{T}(h, (vx / h) * partials(x) + (vy / h) * partials(y)) +end + +@inline function calc_hypot_x(x::Dual{T}, y::Real, z::Real) where T + vx = value(x) + h = hypot(vx, y, z) + return Dual{T}(h, (vx / h) * partials(x)) +end + +# `hypot` is symmetric in its arguments, so the remaining cases are permutations @define_ternary_dual_op( Base.hypot, - calc_hypot(x, y, z, Txyz), - calc_hypot(x, y, z, Txy), - calc_hypot(x, y, z, Txz), - calc_hypot(x, y, z, Tyz), - calc_hypot(x, y, z, Tx), - calc_hypot(x, y, z, Ty), - calc_hypot(x, y, z, Tz), + calc_hypot_xyz(x, y, z), + calc_hypot_xy(x, y, z), + calc_hypot_xy(x, z, y), + calc_hypot_xy(y, z, x), + calc_hypot_x(x, y, z), + calc_hypot_x(y, x, z), + calc_hypot_x(z, x, y), ) # fma # diff --git a/test/DualTest.jl b/test/DualTest.jl index 67d9c9f7..ce91af34 100644 --- a/test/DualTest.jl +++ b/test/DualTest.jl @@ -617,6 +617,14 @@ ForwardDiff.:≺(::Type{OuterTestTag}, ::Type{TestTag}) = false @test dual_isapprox(hypot(FDNUM, FDNUM2, FDNUM), sqrt(2*(FDNUM^2) + FDNUM2^2)) @test dual_isapprox(hypot(FDNUM, FDNUM2, FDNUM3), sqrt(FDNUM^2 + FDNUM2^2 + FDNUM3^2)) + # every argument position has to be checked: only the arguments carrying the tag may + # be unwrapped, so each case needs its own body + @test dual_isapprox(hypot(FDNUM, FDNUM2, PRIMAL3), sqrt(FDNUM^2 + FDNUM2^2 + PRIMAL3^2)) + @test dual_isapprox(hypot(FDNUM, PRIMAL2, FDNUM3), sqrt(FDNUM^2 + PRIMAL2^2 + FDNUM3^2)) + @test dual_isapprox(hypot(PRIMAL, FDNUM2, FDNUM3), sqrt(PRIMAL^2 + FDNUM2^2 + FDNUM3^2)) + @test dual_isapprox(hypot(FDNUM, PRIMAL2, PRIMAL3), sqrt(FDNUM^2 + PRIMAL2^2 + PRIMAL3^2)) + @test dual_isapprox(hypot(PRIMAL, FDNUM2, PRIMAL3), sqrt(PRIMAL^2 + FDNUM2^2 + PRIMAL3^2)) + @test dual_isapprox(hypot(PRIMAL, PRIMAL2, FDNUM3), sqrt(PRIMAL^2 + PRIMAL2^2 + FDNUM3^2)) @test all(map(dual_isapprox, ForwardDiff.sincos(FDNUM), (sin(FDNUM), cos(FDNUM)))) @@ -734,6 +742,35 @@ end @test ForwardDiff.derivative(x -> sum(1 .+ x .* (0:0.1:1)), 1) == 5.5 end +@testset "ternary hypot" begin # issue #834 + # Arguments carrying an inner tag must not be unwrapped: their perturbations have + # to stay nested inside the returned `Dual` instead of being flattened into (and + # summed with) the outer tag's partials. `sqrt` of the sum of squares is built + # from binary operations only and hence serves as a reference. + # `hypot` is symmetric, so every argument position has to be checked. Both tag layouts + # have to be checked as well: dispatch settles on one tag, and a different body runs + # depending on whether one or two of the arguments carry it. + for i in 1:3, args in ((x, y) -> ntuple(j -> j == i ? y : j * x, 3), # one `y`, two `x` + (x, y) -> ntuple(j -> j == i ? j * x : j * y, 3)) # one `x`, two `y` + f(x) = ForwardDiff.derivative(y -> hypot(args(x, y)...), 2.0) + g(x) = ForwardDiff.derivative(y -> sqrt(sum(a -> a^2, args(x, y))), 2.0) + @test f(3.0) ≈ g(3.0) + @test ForwardDiff.derivative(f, 3.0) ≈ ForwardDiff.derivative(g, 3.0) + @test ForwardDiff.derivative(f, 3.0) != 0 + end + + # all three tags distinct, so dispatch has to single out the outermost one + d3(f) = ForwardDiff.derivative( + x -> ForwardDiff.derivative(y -> ForwardDiff.derivative(z -> f(x, y, z), 4.0), 3.0), + 2.0, + ) + @test d3(hypot) ≈ d3((x, y, z) -> sqrt(x^2 + y^2 + z^2)) + @test d3(hypot) != 0 + + # `hypot` must not form squares, otherwise the partials overflow + @test ForwardDiff.gradient(v -> hypot(v[1], v[2], v[3]), fill(1e200, 3)) ≈ fill(1 / sqrt(3), 3) +end + @testset "Givens rotations: consistency with `LinearAlgebra.givensAlgorithm` for zero partials (no duals)" begin # Test different branches in `LinearAlgebra.givensAlgorithm` for f in [randexp(), -randexp()], g in [0.0, f / 2, 2f, -f / 2, -2f]