Skip to content
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

Product of scale transforms not supported #133

Open
sefffal opened this issue Nov 8, 2024 · 3 comments
Open

Product of scale transforms not supported #133

sefffal opened this issue Nov 8, 2024 · 3 comments

Comments

@sefffal
Copy link

sefffal commented Nov 8, 2024

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
Copy link
Member

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
Copy link
Author

sefffal commented Nov 10, 2024

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
Copy link
Member

Yes, that should be completely safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants