Rename kerneldiagmatrix[!] -> kernelmatrix_diag[!]

[st--]

Addresses #233

[devmotion]

Looks good roughly but it seems you forgot to add the deprecations.

[st--]

Looks good roughly but it seems you forgot to add the deprecations.

So I did! I added the tests of the deprecation and apparently was so used to writing code first I then didn't think anymore about actually deprecating them. Thanks for pointing that out 😆

[devmotion]

The deprecations are not correct, please see my comments.

[st--]

I noticed that the in-place kernelmatrix[_diag]! forms only tested the return value, not whether it correctly got written to the passed-in array, so I've added those as well. (The tests of utils.jl and pairwise.jl in contrast only test the passed-in array and don't check the return value, should we add those as well?)

[devmotion]

Oh, good that you added these other tests as well!

The tests of utils.jl and pairwise.jl in contrast only test the passed-in array and don't check the return value, should we add those as well?

Probably, but in my opinion it should be done in a separate PR.

[devmotion]

Thanks, looks good!

Can you bump the version? Then it can be merged and released, if tests pass.

[st--]

While we're looking at this: what's the use-case for kernelmatrix_diag(k, x, y) (with x != y but same shape)? I can't think of any place I'd actually use that code, so is it worth incurring the maintenance cost (like we've had now)? What for or where would you use it?

[devmotion]

I haven't used kerneldiagmat in any downstream code but I would have assumed that the two argument version is more useful. For e.g. translation-invariant kernels kerneldiagmat(k, x) is just trivial and returns a constant vector (usually of ones for unscaled kernels) whereas kerneldiagmat(k, x, y) = [k(xi, yi) for (xi, yi) in zip(x, y)] is a more interesting non-constant vector.

[st--]

I haven't used kerneldiagmat in any downstream code but I would have assumed that the two argument version is more useful. For e.g. translation-invariant kernels kerneldiagmat(k, x) is just trivial and returns a constant vector (usually of ones for unscaled kernels)

In Gaussian process models it's exactly the kernelmatrix_diag(k, x) that's needed all the time - it may be trivial for stationary kernels (but also has the advantage it's super fast to compute), but it's also common to use non-stationary kernels (e.g. linear, arc-cosine, change-points).

whereas kerneldiagmat(k, x, y) = [k(xi, yi) for (xi, yi) in zip(x, y)] is a more interesting non-constant vector.

In contrast, I've not come across any use-case for kernelmatrix_diag(k, x, y) with y!=x (but having to have the same length) which is why I asked.

If we can't think of any use-cases, would it not be better to save the maintenance cost? What's the value of keeping around functions just because we can write them if we have no use for it?

[st--]

The one failing CI run seems to be some blip (if anyone can explain why that would happen intermittently, I'd be curious to find out). So I'm going to go ahead and merge it as is. I've moved the open question to a separate issue (#250).

[theogf]

While we're looking at this: what's the use-case for kernelmatrix_diag(k, x, y) (with x != y but same shape)? I can't think of any place I'd actually use that code, so is it worth incurring the maintenance cost (like we've had now)? What for or where would you use it?

@willtebbutt motivated this as he needs it for Stheno

[st--]

I've bumped the version. Is there anything else that needs doing to release?

@willtebbutt can you point me to the concrete use? would be curious to understand what you do with it.

[theogf]

I've bumped the version. Is there anything else that needs doing to release?

Yes, I just did. You can check it out all you need is to write a comment to the merging commit with @JuliaRegistrator register : 1983d1d#commitcomment-46427428

[st--]

Ah, neat. The docs suggest that given TagBot is installed in this repo's workflow it should automatically add a tag for the new release. How long does it usually take for that to happen? I can't see a new tag yet..

[theogf]

So first it needs to be registered in the Registry and that takes around 20 min (there is a cycle of automerge every 15 min or so) and then TagBot will automatically create a new tag with closes issues / pull request merged, that we can then edit
You can see the Registry PR here : JuliaRegistries/General#28772