Use nansafe branch for ForwardDiff #165
Conversation
test/Project.toml
Outdated
| @@ -18,7 +18,7 @@ AxisArrays = "0.4.3" | |||
| Distances = "0.9" | |||
| FiniteDifferences = "0.10.8" | |||
| Flux = "0.10, 0.11" | |||
| ForwardDiff = "0.10" | |||
| ForwardDiff = "nansafe" | |||
There was a problem hiding this comment.
This doesn't seem to help as suggested here.
There was a problem hiding this comment.
The other suggested solution is including this information in the test/Manifest.toml. But I am not sure if we want to do that.
https://discourse.julialang.org/t/specifying-a-branch-of-a-package-in-project-toml/32101/3
There was a problem hiding this comment.
I would be surprised if there would be a way to specify in the Project.toml, and in your links it is explicitly stated that it does not work currently.
An alternative would be to add something like
import Pkg; Pkg.add(url="https://github.com/JuliaDiff/ForwardDiff.jl", rev="nansafe")to test/runtests.jl (or the Travis script, but I guess it would be better to add it to runtests.jl since otherwise tests would fail locally).
There was a problem hiding this comment.
Looks like Pkg is not available in the Travis test environment for runtests.jl to use. https://travis-ci.com/github/JuliaGaussianProcesses/KernelFunctions.jl/jobs/383271248#L421
We might be forced to add this in the Travis script.
There was a problem hiding this comment.
You just have to add it to the test dependencies, then the lines above will work.
There was a problem hiding this comment.
@devmotion Do you mean we need to add Pkg to test dependencies?
There was a problem hiding this comment.
Yes.
I'm not sure yet though if it's really a good idea to use the nansafe branch for testing.
|
It seems tests on Travis still yield NaN values even with this branch - it doesn't fix (all of) the AD problems? |
Locally it seems to fix problems only for FBM, Gabor and Wiener Kernels. |
|
@devmotion @sharanry Can we close this PR? I does not look possible to use a specific branch for testing and does not generally look like a good idea :D |
|
Yes, JuliaDiff/ForwardDiff.jl#539 (or something similar) and switching to |
Related issue and comment: #116 (comment)
Fix proposed here: JuliaDiff/ForwardDiff.jl#451
This uses the
nansafebranch of ForwardDiff in order to fix issues with FBM kernel.