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

KernelSum and KernelProduct with Vector of Kernel fails #497

Open
theogf opened this issue Mar 28, 2023 · 5 comments · Fixed by #547
Open

KernelSum and KernelProduct with Vector of Kernel fails #497

theogf opened this issue Mar 28, 2023 · 5 comments · Fixed by #547
Labels
bug Something isn't working

Comments

@theogf
Copy link
Member

theogf commented Mar 28, 2023

Right now these composites will not work if the container is not a Tuple.
See

k = SqExponentialKernel()
kk = KernelSum([k, k])
kk(2, 3) # Errors
@theogf theogf added the bug Something isn't working label Mar 28, 2023
@devmotion
Copy link
Member

From looking at the code, it seems we just forgot to add a fallback for _sum?

_sum(f, ks::Tuple, args...) = f(first(ks), args...) + _sum(f, Base.tail(ks), args...)
We explicitly supports constructors and summation for KernelSum with AbstractVector, so it should work. I'm surprised there's no test for it.

@theogf
Copy link
Member Author

theogf commented Mar 28, 2023

Yes sorry I started with the title, found that the code was almost right and forgot to update.

I will edit the issue name now

@theogf theogf changed the title Refactoring KernelSum and KernelProduct KernelSum and KernelProduct with Vector of Kernel fails Mar 28, 2023
@theo-brown
Copy link

Has this been fixed?

@theogf
Copy link
Member Author

theogf commented Feb 6, 2024

No the issue still seems to be there. The MWE is wrong, I will adjust it.

@theogf
Copy link
Member Author

theogf commented Feb 9, 2024

Reopening as the KernelProduct still has the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants