Fix Independent MOKernel for single dimension input

[sharanry]

Fails for

N = 100
x = rand(Uniform(-1, 1), N)
X = MOInput(x, 2)
k = IndependentMOKernel(MaternKernel())
k(X, X) #error

[theogf]

Nice catch! Could you also add a test for this?

[sharanry]

@theogf Is this fine?

[theogf]

Looks good! You can use @test_nothrow instead of checking for a matrix type

[devmotion]

You can use @test_nothrow instead of checking for a matrix type

Is this a custom thing we define in KernelFunctions? I have never seen it in Base 🤔 IMO you could just remove @test_nothrow - if the call errors, the tests fail anyways.

[sharanry]

You can use @test_nothrow instead of checking for a matrix type

Is this a custom thing we define in KernelFunctions? I have never seen it in Base thinking IMO you could just remove @test_nothrow - if the call errors, the tests fail anyways.

I think @test_nothrows helps by catching the error and letting the other tests after it run irrespective of whether the error occurs.

Edit: There is no such thing as @test_nothrow 😅

[theogf]

I am sorry! I am at the beach so i did not double check 😅. I meant @test_nowarn

[willtebbutt]

Wait, @sharanry could you explain to me why this is failing? I'm unclear why the previous implementation is incorrect 😬 .

In particular, why is the solution you've implemented the correct one?

[sharanry]

Previously, (κ::IndependentMOKernel) worked with only inputs of the type Tuple{Vector, Int}. However, when working with 1-dim inputs, we might end up with Tuple{<:Real, Int} which wouldn't have been accepted by the old implementation.

[willtebbutt]

Oh, I see. Would the following not be sufficient:

function (k::IndependentMOKernel)(x::Tuple{Any, Int}, y::Tuple{Any, Int})
    if last(x) == last(y)
        return k.kernel(first(x), first(y))
    else
        return appropriate_zero
    end
end

i.e. I'm not sure that we need two methods, or to assume that the wrapped kernel requires vector inputs.

[sharanry]

That should be sufficient. However, we wouldn't have any checks for the first element of the tuple as you point out. I like it as it is definitely a less constrained way.

Shall I start a new PR for this?

[willtebbutt]

That should be sufficient. However, we wouldn't have any checks for the first element of the tuple as you point out. I like it as it is definitely a less constrained way.

Do we need a check on the type of the first element of the tuples? Can they not generally be anything?

Shall I start a new PR for this?

That would be good -- better to address this sooner rather than later :)

[sharanry]

Do we need a check on the type of the first element of the tuples? Can they not generally be anything?

I guess not. This is should make the kernels more flexible. For other non-multi-output kernels we do not really perform any check for the inputs.