Product of scale transforms not supported

[sefffal]

Hello all,
I'm keen to test out this package on my problem once more. I previously was using it in Octofitter, but had to drop it due to Zygote-related precompile errors (which are now fixed, thanks a lot!)

Previously, I was able to approximate a quasi-periodic kernel

kernel = η₁^2 *  
   (SqExponentialKernel() ∘ ScaleTransform(1/(η₂))) *
   (PeriodicKernel(r=[η₄]) ∘ ScaleTransform(1/(η₃)))

with the following approximation, which was compatible with TemporalGPs:

kernel = η₁^2 *  
   (Matern52Kernel() ∘ ScaleTransform(1/η₂)) *  
   (ApproxPeriodicKernel{1}(r=η₄) ∘ ScaleTransform(1/η₃))

Unfortunately, now on the latest stable release I receive the following error:

using AbstractGPs, TemporalGPs
η₁=10.0
η₂=37.6
η₃=11.68
η₄=0.5

kernel = η₁^2 *  
   (Matern52Kernel() ∘ ScaleTransform(1/η₂)) *  
   (ApproxPeriodicKernel{1}(r=η₄) ∘ ScaleTransform(1/η₃))

gp_naive = GP(kernel)
f = to_sde(gp_naive, SArrayStorage(Float64))

x = sort(randn(1000))
y = randn(1000)
v = rand(1000)

fx = f(x,v)
logpdf(fx, y)

ERROR: ArgumentError: Not all kernels in k are safe to product.
Stacktrace:
 [1] lgssm_components(k::KernelProduct{Tuple{…}}, ts::Vector{Float64}, storage::SArrayStorage{Float64})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:403
 [2] lgssm_components(::ZeroMean{…}, k::KernelProduct{…}, t::Vector{…}, storage_type::SArrayStorage{…})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:115
 [3] build_lgssm(f::TemporalGPs.LTISDE{GP{…}, SArrayStorage{…}}, x::Vector{Float64}, Σys::Vector{Float64})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:74
 [4] build_lgssm(ft::AbstractGPs.FiniteGP{TemporalGPs.LTISDE{…}, Vector{…}, LinearAlgebra.Diagonal{…}})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:80
 [5] _logpdf(ft::AbstractGPs.FiniteGP{TemporalGPs.LTISDE{…}, Vector{…}, LinearAlgebra.Diagonal{…}}, y::Vector{Float64})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:67
 [6] logpdf(ft::AbstractGPs.FiniteGP{TemporalGPs.LTISDE{…}, Vector{…}, LinearAlgebra.Diagonal{…}}, y::Vector{Float64})
   @ TemporalGPs ~/.julia/packages/TemporalGPs/Gl8Zv/src/gp/lti_sde.jl:60
 [7] top-level scope
   @ REPL[83]:19
Some type information was truncated. Use `show(err)` to see complete types.

It seems that these two different scale transforms can't be combined any longer. I'm not a sophisticated user of GPs, so I don't have much context for this error.

Is this possible to support once again? Or is there another way to formulate this kernel that is compatible with the newer version of TemporalGPs?

Thanks very much for any help you can offer!

[willtebbutt]

Ah, yes, thanks for opening this issue.

When refactoring I added an additional safety check because there was some risk of getting incorrect gradients with the current implementation of reverse-mode AD for the matrix exponential that this package makes use of. I need to resolve this anyway, so I'll try and sort it asap in Mooncake, and remove the hacky implementation from this package, at which point I'll be able to remove this restriction.

I'll let you know when this is done!

[sefffal]

Thanks very much!

If I want to experiment with it without calculating gradients, is it safe for me to just remove that check for now?

[willtebbutt]

Yes, that should be completely safe.

[willtebbutt]

@sefffal this looks to be resolved for me on v0.7.2, provided that you swap out SArrayStorage for ArrayStorage (this is just a requirement of the ApproxPeriodicKernel). I'm going to close this as completed, but please feel free to re-open if it's not working for you.