Add Periodic Kernel #50
Conversation
src/distances/sinus.jl
Outdated
| r::Vector{T} | ||
| end | ||
|
|
||
| @inline function Distances._evaluate(d::Sinus,a::AbstractVector{T},b::AbstractVector{T}) where {T} |
There was a problem hiding this comment.
In general, I think one should rather define Distances.parameters and Distances.eval_op (I guess the default definitions of eval_start, eval_reduce, and eval_end should be fine). A Zygote-compatible implementation could then be added in the definition of the custom adjoint.
Here's an example how these functions are used when evaluating the distance between two arrays:
https://github.com/JuliaStats/Distances.jl/blob/master/src/metrics.jl#L181-L210
Using these definitions, one could even just define
(dist::Sinus)(a::Number, b::Number) = Distances.eval_op(dist, a, b, first(dist.r))There was a problem hiding this comment.
Thanks I implemented eval_op only now
There was a problem hiding this comment.
Ah but this would not be taken into account, since UnionMetrics is fixed : https://github.com/JuliaStats/Distances.jl/blob/7f3a28c0d1372e3b3edbcbc28f00ba5645e1bbdb/src/metrics.jl#L107
There was a problem hiding this comment.
Ah sorry I completely missed that 🙈 Somehow I assumed JuliaStats/Distances.jl#126 was fixed already.
There was a problem hiding this comment.
I will leave it as it is for now
There was a problem hiding this comment.
The "UnionMetrics as a Union" issue is now resolved on master of Distances.jl, so @devmotion's suggestion should now be doable. There is a similar minimal example in JuliaStats/Distances.jl#151 (comment).
There was a problem hiding this comment.
@dkarrasch Thanks! We need to wait for a new release of Distances.jl now...
|
I could not put the |
|
Not sure where to post it, so I'll do it here. Distances.jl has released a new version, that allows to subtype |
|
I've already added a lower bound for 0.9 ( KernelFunctions.jl/Project.toml Line 18 in 29b5ad7 |
Following the request #34 I implemented a first version of the periodic kernel described in http://www.inference.org.uk/mackay/gpB.pdf equation 47.
I also added the unconventional Sinus metric to keep the same style as other kernels.