Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Erratic errors in typed Hessians #29

Closed
rgiordan opened this issue Jul 29, 2015 · 4 comments
Closed

Erratic errors in typed Hessians #29

rgiordan opened this issue Jul 29, 2015 · 4 comments
Labels

Comments

@rgiordan
Copy link
Contributor

ForwardDiff.forwarddiff_hessian intermittently returns hessians that look like they are unset memory - - they are completely wrong. Since this happens intermittently and only in certain settings I haven't been able to narrow down exactly what the problem is, but I have a couple relatively minimal examples that reproduce the problem reliably on my computer.

I happen to have discovered it when using atan. I haven't been able to reproduce it for simple polynomials and haven't systematically tried other functions yet. Here's the simplest example:

using ForwardDiff

function my_fun2{T <: Number}(y::Array{T})
    @assert length(y) == 1
    atan(y[1])
end

my_hess2 = ForwardDiff.forwarddiff_hessian(my_fun2, Float64, fadtype=:typed, n=1)

y = [4.]
original = my_hess2(y);
bad_versions = 0
max_iters = 1000
for iter = 1:max_iters
    this_hess = my_hess2(y)
    if maximum(abs(this_hess - original)) > 1
        println("$iter: ", this_hess)
        bad_versions += 1;
    end
end
bad_versions / max_iters   # Often around 0.04 or so, though sometimes 0.0

I don't know if it's a red herring, but happened to discover it when indexing into the argument using a closure. The following, more complicated example, fails much more often:

# This produces the problem intermittently:
indices = Int64[3, 2, 1]
coeffs = [10., 100., 1000.]
function my_fun{T <: Number}(y::Array{T})
    @assert length(y) == 3
    T[ coeffs[1] * y[indices[3]], coeffs[2] * y[indices[2]] ^ 2, coeffs[2] * atan(y[indices[3]]) ]
end

function get_hess_func_vec(my_fun_arg::Function, K::Int64)
    [ ForwardDiff.forwarddiff_hessian(y -> my_fun(y)[k], Float64, fadtype=:typed, n=K) for k=1:K ]
end

my_hess_funcs = get_hess_func_vec(my_fun, 3);
y = [1., 2., 3.]

# Repeated evaluation of this function gives erratic results:
original = my_hess_funcs[3](y);
bad_versions = 0
max_iters = 1000
for iter = 1:max_iters
    this_hess = my_hess_funcs[3](y)
    if maximum(abs(this_hess - original)) > 1
        println("$iter: ", this_hess)
        bad_versions += 1;
    end
end
bad_versions / max_iters # As low as 0.02 or so, often around 0.5 (of course, the original could be wrong, too)

Please let me know if you have any ideas or want more details.

@jrevels
Copy link
Member

jrevels commented Jul 29, 2015

Good catch!

The FADHessian code in master has a bunch of little bugs in both utility functions (e.g. constructors/conversion) and a couple in the mathematical definitions as well. A large number of tiny bugfixes are included in #27, and the state of testing, functional coverage, and performance is greatly improved. A lot of this has to do with the move to automating the definitions of univariate mathematical functions - I'm not even sure how many bugs got fixed by 548be29, since past test coverage was intermittent, but I suspect quite a few. It's likely that by the time #27 lands (in a week or two), this could issue will be fixed (sorry for the wait in the meantime).

I don't have time tonight to check out for sure that it does fix the above, but I'll check tomorrow and report back.

@rgiordan
Copy link
Contributor Author

That sounds fine, thanks for the quick response. I tried a handful of other trig functions and did not observe the problem, and I can implement atan with exp by hand in the meantime, so no rush on my account.

Thanks!

@jrevels
Copy link
Member

jrevels commented Jul 29, 2015

I ran your examples, and I can say with more certainty that this is fixed by #27 - I'm seeing a bad_versions/max_iter ratio of 0.0, even when cranking up max_iter to 10e6.

The code I used is here; it's basically the code example you gave, but modified to use the new API exposed in #27.

Unless you find anything else, I think this issue can be closed once #27 lands.

@jrevels
Copy link
Member

jrevels commented Aug 14, 2015

This now fixed on master as of the merging of #27.

@jrevels jrevels closed this as completed Aug 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants