Stable version of KernelProduct and added test_type_stability

[theogf]

Summary
This adresses #485 by bringing a similar solution from #459. It goes slightly differently by avoiding a closure entirely.
This also add tests for checking that on the culprit kernel, the type stability is maintained.
Regarding performance, it seems to not change anything (maybe marginally better).

master:

julia> @btime kernelmatrix($nested_k, $x);
  5.005 μs (23 allocations: 10.94 KiB)

this PR:

julia> @btime kernelmatrix($nested_k, $x);
  4.936 μs (18 allocations: 10.69 KiB)

This also refactors a part of TestUtils.jl to run with test_with_type so that we can reuse this functionality for different tests.

Breaking changes
Non-breaking

[codecov]

Codecov Report

Base: 94.32% // Head: 82.64% // Decreases project coverage by -11.68% ⚠️

Coverage data is based on head (f647b40) compared to base (eac3538).
Patch coverage: 65.45% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #486       +/-   ##
===========================================
- Coverage   94.32%   82.64%   -11.69%     
===========================================
  Files          52       52               
  Lines        1358     1371       +13     
===========================================
- Hits         1281     1133      -148     
- Misses         77      238      +161     

Impacted Files	Coverage Δ
src/TestUtils.jl	75.67% <55.00%> (-19.41%)	⬇️
src/kernels/kernelproduct.jl	56.66% <87.50%> (-43.34%)	⬇️
src/kernels/kernelsum.jl	57.14% <100.00%> (-42.86%)	⬇️
src/kernels/gibbskernel.jl	0.00% <0.00%> (-88.89%)	⬇️
src/kernels/normalizedkernel.jl	0.00% <0.00%> (-82.50%)	⬇️
src/kernels/neuralkernelnetwork.jl	0.00% <0.00%> (-77.56%)	⬇️
src/kernels/overloads.jl	25.00% <0.00%> (-75.00%)	⬇️
src/kernels/scaledkernel.jl	41.17% <0.00%> (-47.06%)	⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[devmotion]

What's the root issue here? reduce with a generator? hadamard? Depending on what exactly the problem is, I wonder if one should fix something upstream instead.

[theogf]

It's really hard to say, when going with Cthulhu through the calls, it seems that the only type unstable calls are due to the DimensionMismatch errors thrown by Distances.jl

[devmotion]

Hmm errors shouldn't cause type instabilities. Otherwise you could basically never throw errors in any function.

[theogf]

I honestly don't know how to find the underlying issue... if you have a method please share 😊

[willtebbutt]

Glad to have this fix in. It's possible it will make more of a difference on the reverse-pass of AD anywy.

I think I just want slightly more general testing of the kernelmatrix-related functions (sorry to ask for more stuff) -- just seems silly not to make slightly more general use of your new functionality.

[theogf]

Hmm errors shouldn't cause type instabilities. Otherwise you could basically never throw errors in any function.

I was thinking of an eventuality:

When using a generator or Base.Fix2 we create a closure, which x is part of. Maybe it's just too much for the compiler to treat in such nested cases?

[willtebbutt]

@theogf any progress on this on your end? I'm happy to finish up the PR if you're busy

[theogf]

@theogf any progress on this on your end? I'm happy to finish up the PR if you're busy

I was thinking of finishing up this week-end.

[willtebbutt]

Cool. I'll leave you to it then :)

[willtebbutt]

Broadly LGTM. Just some formatting issues to resolve, then I think we're good to go

edit: also, sorry for taking a few days to review this -- have been busy with work etc :(

[theogf]

Broadly LGTM. Just some formatting issues to resolve, then I think we're good to go

edit: also, sorry for taking a few days to review this -- have been busy with work etc :(

No worries I think we are all on the same boat now 😆

[willtebbutt]

Once you've got CI passing, happy for this to go in. Thanks for sorting it out!