-
Notifications
You must be signed in to change notification settings - Fork 34
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 gradient issues with kernelmatrix_diag and use ChainRulesCore #208
Conversation
This makes me a little nervous. The benefit of calling I would have thought that a better solution would be to require edit: see e.g. what I do in Stheno. |
Actually, I'm surprised that this problem is happening at all. Looking into it now. |
@willtebbutt It seems your fix on defining methods for SimpleKernel fixed the issue. I added AD test for kerneldiagmatrix now |
Co-authored-by: David Widmann <[email protected]>
It seems the error is not fixed? |
Yep, I tried @willtebbutt suggestion but it does not solve the problem... |
Back to this annoying problem! |
I guess the correct way would be to define our own |
Current fails are just random, it's ready to review/merge |
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.
Looks good overall! I had a few questions, and I think we should use ChainRulesTestUtils to properly test the ChainRules definitions.
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Should we make a patch version bump followed by a minor version bump where we remove the deprecation warnings? |
Sorry @theogf , which deprecations are you referring to? |
|
Oh I see. Yeah, patch followed by minor release seems reasonable to me. |
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.
Looks good to me 👍
We are encountering following warning during Julia REPL startup if we include `KernelFunctions` in the default SYSIMG, this commit fixes that issue ``` ┌ Warning: Error requiring `PDMats` from `KernelFunctions` │ exception = │ SystemError: opening file "/home/bmharsha/.julia/packages/KernelFunctions/AxuTC/src/matrix/kernelpdmat.jl": No such file or directory │ Stacktrace: │ [1] systemerror(p::String, errno::Int32; extrainfo::Nothing) │ @ Base ./error.jl:168 │ [2] #systemerror#62 │ @ ./error.jl:167 [inlined] │ [3] systemerror │ @ ./error.jl:167 [inlined] │ [4] open(fname::String; lock::Bool, read::Nothing, write::Nothing, create::Nothing, truncate::Nothing, append::Nothing) │ @ Base ./iostream.jl:293 │ [5] open │ @ ./iostream.jl:282 [inlined] │ [6] open(f::Base.var"JuliaGaussianProcesses#326#327"{String}, args::String; kwargs::Base.Iterators.Pairs{Union{}, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}) │ @ Base ./io.jl:328 │ [7] open │ @ ./io.jl:328 [inlined] │ [8] read │ @ ./io.jl:434 [inlined] │ [9] _include(mapexpr::Function, mod::Module, _path::String) │ @ Base ./loading.jl:1166 │ [10] include(mod::Module, _path::String) │ @ Base ./Base.jl:386 │ [11] include(x::String) │ @ KernelFunctions ~/.julia/packages/KernelFunctions/AxuTC/src/KernelFunctions.jl:1 │ [12] top-level scope │ @ ~/.julia/packages/KernelFunctions/AxuTC/src/KernelFunctions.jl:124 │ [13] eval │ @ ./boot.jl:360 [inlined] │ [14] eval │ @ ~/.julia/packages/KernelFunctions/AxuTC/src/KernelFunctions.jl:1 [inlined] │ [15] (::KernelFunctions.var"JuliaGaussianProcesses#209#215")() │ @ KernelFunctions ~/.julia/packages/Requires/7Ncym/src/require.jl:99 │ [16] err(f::Any, listener::Module, modname::String) │ @ Requires ~/.julia/packages/Requires/7Ncym/src/require.jl:47 │ [17] (::KernelFunctions.var"JuliaGaussianProcesses#208#214")() │ @ KernelFunctions ~/.julia/packages/Requires/7Ncym/src/require.jl:98 │ [18] withpath(f::Any, path::String) │ @ Requires ~/.julia/packages/Requires/7Ncym/src/require.jl:37 │ [19] (::KernelFunctions.var"JuliaGaussianProcesses#207#213")() │ @ KernelFunctions ~/.julia/packages/Requires/7Ncym/src/require.jl:97 │ [20] listenpkg(f::Any, pkg::Base.PkgId) │ @ Requires ~/.julia/packages/Requires/7Ncym/src/require.jl:20 │ [21] macro expansion │ @ ~/.julia/packages/Requires/7Ncym/src/require.jl:95 [inlined] │ [22] __init__() │ @ KernelFunctions ~/.julia/packages/KernelFunctions/AxuTC/src/KernelFunctions.jl:123 └ @ Requires ~/.julia/packages/Requires/7Ncym/src/require.jl:49 ```
This PR aims at solving the AD issues coming from
kernelmatrix_diag
, add tests for it and incidentally solve AD issue for whenx==y
.This solves the issue on #203