-
-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fix support for BigFloat u0 #202
Conversation
Signed-off-by: ErikQQY <2283984853@qq.com>
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
|
We should just use a LazyBufferCache then? |
Signed-off-by: ErikQQY <2283984853@qq.com>
Implemented a special diff cache for the BigFloat case, which may not be the best solution, and some improvement is still necessary, the BigFloat support for MIRK is working now!! |
I don't understand the need for |
We only build the BigFloat cache when the input |
SciML/PreallocationTools.jl#112 should be all you need? |
Hopefully this can close 🤞 and BVP needs to do nothing special. |
Signed-off-by: ErikQQY <2283984853@qq.com>
Signed-off-by: ErikQQY <2283984853@qq.com>
Signed-off-by: ErikQQY <2283984853@qq.com>
src/utils.jl
Outdated
y = similar(x, args...) | ||
eltype(y) <: BigFloat && fill!(y, BigFloat(0)) |
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.
is there a reason to not always fill with zeros?
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.
zeros is better, just change them all. While NonlinearSolve is doing the similar thing like specialize on bigfloat and do __similar
things in SciML/NonlinearSolve.jl#438, maybe we can change to zeros everywhere in NonlinearSolve.jl as well?
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.
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.
Note this will also fix issues with non-fully defined functions. For example, if a user doesn't write to du[i]
, we should still converge now, while before you'd get something random. So a few bugs should be fixed, some bugs avoided, etc. Avoiding uninitialized memory is just a good idea all around and NonlinearSolve needs to do this change too.
Signed-off-by: ErikQQY <2283984853@qq.com>
Fix #43
Unblock #178
Possibly need SciML/PreallocationTools.jl#72
I think
get_tmp
in PreallocationTools.jl doesn't support BigFloat and would causecannot reinterpret `BigFloat` as `ForwardDiff.Dual`, type `ForwardDiff.Dual` is not a bits type
error, similar issue is SciML/PreallocationTools.jl#72@avik-pal
Full stacktrace