[Fastaxx]

Adds the three-argument case of the existing binary rule: the derivatives of hypot(x, y, z) are x / hypot(x, y, z), y / hypot(x, y, z) and z / hypot(x, y, z).

ForwardDiff skips ternary rules and defines its own ternary hypot, and Symbolics differentiates hypot(x, y, z) through Base's implementation, so neither changes.

The new rule gets its own testset inside @testset "DiffRules" : each derivative against central finite differences, in Float32 and Float64, and the type of the result. The full suite passes.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.96%. Comparing base (ab1d7ef) to head (726b8de).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
+ Coverage   95.97%   97.96%   +1.99%     
==========================================
  Files           2        3       +1     
  Lines         174      197      +23     
==========================================
+ Hits          167      193      +26     
+ Misses          7        4       -3     

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

ForwardDiff skips ternary rules and defines its own ternary hypot, and Symbolics differentiates hypot(x, y, z) through Base's implementation, so neither changes.

So why add it?

[Fastaxx]

We are replacing StencilAlgebra.jl's long in-house table of derivative rules with DiffRules : Our builds symbolic derivative formulas from DiffRules' expressions at every arity, and hypot(x, y, z) is common in its formulas: with this rule its derivative comes from DiffRules instead of a rule of its own. If you prefer to keep DiffRules to what the AD packages use, I understand and can close it.

[devmotion]

Thanks! The rule itself looks correct: it matches the binary rule, the derivatives are bounded by 1 in magnitude so there's no over-/underflow (also for inputs around 1e-300/1e300), and Float32 is preserved. At the origin it returns NaN, consistent with the binary rule.

Note that this is the first non-commented ternary rule, so diffrules() returns an arity-3 entry for the first time. I checked downstream packages:

• ForwardDiff (master) and ReverseDiff only handle arity 1 and 2 and silently skip the new rule; gradients of hypot(x, y, z) are unchanged and correct. Tracker also only handles arity 1 and 2.
• JuliaDiff/ForwardDiff.jl#872 skips (:Base, :hypot) for all arities, so it is unaffected as well.
• Symbolics loops over all arities and will now define derivative_rule(::typeof(hypot), ::Val{3}, args, ::Val{i}) methods (loading works). They are not hit in practice since hypot(::Num, ::Num, ::Num) expands via Base's generic implementation. So the sentence in the PR description that Symbolics doesn't change is not quite accurate.

My main comment is about the tests, see below.

[devmotion]

Instead of a separate testset, could we re-enable the (currently commented out) generic elseif arity == 3 branch in the "check rules" loop above and update it analogously to the arity == 2 branch? (:Base, :ifelse, 3) is already excluded there via non_diffeable_arg_functions. Then hypot (and any future ternary rule) is covered automatically, and we check the types of all three derivatives (currently only dx is checked). Something like

elseif arity == 3
    @test DiffRules.hasdiffrule(M, f, 3)
    derivs = DiffRules.diffrule(M, f, :foo, :bar, :goo)
    for T in (Float32, Float64)
        @eval begin
            foo, bar, goo = rand($T), rand($T), rand($T)
            dx, dy, dz = $(derivs[1]), $(derivs[2]), $(derivs[3])
            if !(isnan(dx))
                @test dx ≈ finitediff(z -> $M.$f(z, bar, goo), foo) rtol=1e-2 atol=1e-3
                @test promote_type(typeof(real(dx)), $T) === $T
            end
            if !(isnan(dy))
                @test dy ≈ finitediff(z -> $M.$f(foo, z, goo), bar) rtol=1e-2 atol=1e-3
                @test promote_type(typeof(real(dy)), $T) === $T
            end
            if !(isnan(dz))
                @test dz ≈ finitediff(z -> $M.$f(foo, bar, z), goo) rtol=1e-2 atol=1e-3
                @test promote_type(typeof(real(dz)), $T) === $T
            end
        end
    end

The .+ $T(0.5) offset also shouldn't be needed for hypot, it's smooth away from the origin.