JuliaGaussianProcesses / KernelFunctions.jl

Julia package for kernel functions for machine learning
https://juliagaussianprocesses.github.io/KernelFunctions.jl/stable/
MIT License
266 stars 32 forks source link

iskroncompatible #93

Open willtebbutt opened 4 years ago

willtebbutt commented 4 years ago
  1. iskroncompatible appears to be incorrectly implemented in a couple of places. For example, I don't believe that an ExponentialKernel should not be marked as compatible, or have I misunderstood something?
  2. Do we really want to try and do this with a flag method? Would be not be better off introducing a RectilinearGrid input type (that is a subtype of AbstractVector) and then using dispatch to figure out whether it's correct to return a KroneckerMatrix when kernelmatrix is called?
theogf commented 4 years ago

Back to this :

willtebbutt commented 4 years ago

I wouldn't imagine that you would need to introduce a type instability. One would just implement specialised kernelmatrix functions for kernels that are separable when they receive a RectilinearGrid, that spit out Kronecker matrices (e.g. from Kronecker.jl). Everything else in the pipeline would just specialise on that.

Am I missing something?

e.g.

kernelmatrix(k::SqExponentialKernel, x::RectilinearGrid) = Kronecker(...)

and all of the other SqExponentialKernel methods would remain unchanged.

theogf commented 3 years ago

Coming back to this issue:

Additional point we can also define the TensorProduct as "kroncompatible"

willtebbutt commented 3 years ago

I think the RectilinearGrid solution would be too restrictive (I don't think a uniform grid is needed to compute a Kronecker matrix?). I don't see a problem having it, but I think dispatching separately for every kernel able to do it is too much unnecessary work.

It's just unclear to me what other contexts Kronecker product structure arises in. You need to have some kind of cartesian-product-like structure, and a rectilinear grid is the most general manifestation of that in Euclidean space, if I'm not mistaken. Maybe CartesianProductVector would be a better name?

devmotion commented 3 years ago

More structured output, such as Toeplitz or circulant matrices, would be useful also when working with Gaussian random fields.

theogf commented 3 years ago

Yes! It's even in the README! :laughing:

devmotion commented 3 years ago

For Toeplitz matrices, it would be good to fix https://github.com/JuliaMatrices/ToeplitzMatrices.jl/issues/36 :grimacing:

Edit: I opened a PR: https://github.com/JuliaMatrices/ToeplitzMatrices.jl/pull/60