Skip to content

Resolve NaN in hypot near zero #669

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

timholy
Copy link
Contributor

@timholy timholy commented Oct 13, 2023

This checks whether hypot of the value component is zero,
and if so switches to a next-order method.

This may be slightly controversial since it exploits the difference between 0.0 and -0.0 to give the correct sign behavior at the origin. This exploits the difference between 0.0 and -0.0 to provide the same behavior exhibited for abs:

julia> ForwardDiff.derivative(abs, 0.0)
1.0

julia> ForwardDiff.derivative(abs, -0.0)
-1.0

# and now:
julia> f(x) = hypot(x, 0, 0)
f (generic function with 1 method)

julia> ForwardDiff.derivative(f, 0.0)
1.0

julia> ForwardDiff.derivative(f, -0.0)
-1.0

This implementation is consistent with the limit -> 0, e.g.,

julia> hypotvec(v) = hypot(v...)
hypotvec (generic function with 1 method)

julia> ForwardDiff.gradient(hypotvec, [0.0, -0.0, 0.0])
3-element Vector{Float64}:
  0.5773502691896258
 -0.5773502691896258
  0.5773502691896258

# for comparison:
julia> ForwardDiff.gradient(hypotvec, [1e-12, -1e-12, 1e-12])
3-element Vector{Float64}:
  0.5773502691896257
 -0.5773502691896257
  0.5773502691896257

Note this only covers the 3-arg version. For the 2-arg version I think a similar fix must be made in DiffRules.

This checks whether `hypot` of the value component is zero,
and if so switches to a next-order method.
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (d300209) 89.65% compared to head (482dde0) 87.41%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #669      +/-   ##
==========================================
- Coverage   89.65%   87.41%   -2.25%     
==========================================
  Files          11       10       -1     
  Lines         967      898      -69     
==========================================
- Hits          867      785      -82     
- Misses        100      113      +13     
Files Coverage Δ
src/dual.jl 80.73% <100.00%> (-1.42%) ⬇️

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant