-
Notifications
You must be signed in to change notification settings - Fork 28
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
Allocate df with similar, partially fixes GPU support #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any other places where this can be added? It seems like a good solution
Thanks, I seem to remember that I had similar and went away from it for some very particular reason, but I cannot remember why. If I can’t think of why, I’ll merge :) but I’ll give optim and nlsolve a spin with it first. |
Yup, I think I remember. It's the ArrayPartition test in Optim... I have to look into this. Can you check if this doesn't fail for you with your version of NLSolversBase and latest Optim?
I'm in the middle of something else right now, but maybe we can specifically fix this version by branching out if |
Or maybe this is to be considered a bug in RecursiveArrayTools... |
Potentially the other
The test you shared does fail in |
What do you mean here? Can you share the error? |
Maybe this is it
I thought I was getting NaNs in gradients, potentially from reduced precision but I think I was mistaken (I see the NaNs in the existing passing tests). I think the issue is somewhere else.
|
Yeah, it appears that |
The only tests failing are
Others pass fine. So, I think there's an issue in how |
Interesting, let me see. |
Printing info when there are differences before and after the changes is telling: function alloc_DF(x, F)
a = (Base.OneTo(length(F)), Base.OneTo(length(x)))
df = similar(F, a)
fill!(df, NaN)
df_old = fill(eltype(F)(NaN), length(F), length(x))
if !(size(df) == size(df_old))
println("------------------- ")
@show F
@show x
@show typeof(F)
@show typeof(x)
@show eltype(F)
@show eltype(x)
@show df
@show df_old
@show typeof(df)
@show typeof(df_old)
@show eltype(df)
@show eltype(df_old)
println("-------------------")
end
return df
end yields
It's not clear to me how |
Hm, your I have an idea to make this work |
Yeah, IIUC how |
Hessian rarely plays well with these types of arrays, so I'm just going to change the hessian cache allocator. Let me run my tests and then I'll conclude :) Thanks for the help so far btw. |
Change H allocator.
Alright, all tests are passing locally with the new H allocator. @pkofod feel free to merge, or let me know if you want me to squash first. |
This PR allocates
df
usingsimilar
instead ofeltype
to supportCuArrays
.Partially fixes NLsolve.jl's 234.
Script:
test/runtests_gpu.jl
Before (without
CuArrays.allowscalar(false)
):After (without
CuArrays.allowscalar(false)
):Before (with
CuArrays.allowscalar(false)
):After (with
CuArrays.allowscalar(false)
):