ForwardDiff.jl icon indicating copy to clipboard operation
ForwardDiff.jl copied to clipboard

expand NaN-safe mode to cover individual zeros in Partials

Open jrevels opened this issue 6 years ago • 1 comments
trafficstars

This likely further decreases performance when NaN-safe mode is engaged, but I think it's more correct so it's worth the perf hit either way. NaN-safe mode is already the expected slow path so this shouldn't change user expectations too much.

I'll do at least a simple benchmark to get a ballpark performance impact for the docs. Also need to add a test or two.

cc @AndreasNoack this fixes your example; does it help with whatever actual problem you were encountering?

jrevels avatar Feb 12 '19 22:02 jrevels

Interestingly, it looks like this actually improves the performance ratio between NaN-safe mode and non-NaN-safe mode for a simple rosenbrock gradient check (code below). I'm not sure why though, and the current performance quote of 5%-10% is definitely no longer correct; on master, right now, the performance hit is closer to 5x (ouch). It's about 3x with this PR.

At this point, I'm not confident enough to provide a specific estimate of the performance impact in the docs, so I'm going to make the current statement there more vague.

using ForwardDiff, BenchmarkTools

function rosenbrock(x::Vector)
    a = 1.0
    b = 100.0
    result = 0.0
    for i in 1:length(x)-1
        result += (a - x[i])^2 + b*(x[i+1] - x[i]^2)^2
    end
    return result
end

x = rand(1000)
@benchmark ForwardDiff.gradient(rosenbrock, $x)

jrevels avatar Feb 19 '19 19:02 jrevels