KernelFunctions.jl
KernelFunctions.jl copied to clipboard
Make ARDTransform a subtype of LinearTransform?
As discussed in https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/pull/106, the ARDTransform could be viewed as a special LinearTransform with a diagonal matrix. Do we still want to keep these separate types?
I would strongly push for keeping ARDTransform separate from LinearTransform.
It is indeed a special case but it is a very used one. In the same sense that SqExponentialKernel is just a special case of the GammaSqExponentialKernel
I agree that users should have easy access an ARD transform, but I think it's very debatable whether or not this means we need to maintain an entirely separate transform from the LinearTransform unless there are good performance reasons to do so.
We could just alias const ARDTransform = LinearTransform{<:Diagonal} (or whatever the correct parametrisation is) and no longer have to maintain the corresponding code as a consequence.
For me at least, the difference between this and the SqExponentialKernel case is that here whether or not something is a ARD is determined by the type of its argument, so we know everything at compile time and can specialise appropriately using dispatch.
I agree. IMO there's no particular reason for not unifying the underlying implementation (which a user shouldn't deal with or be concerned about). As @willtebbutt says, one can introduce an alias and still provide constructors for vectors to users, even if one makes ARDTransform a subtype of LinearTransform.
I am not sure I see the point of it but okay...