Remove input check for vector of vectors#513
Remove input check for vector of vectors#513grero wants to merge 1 commit intoJuliaGaussianProcesses:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #513 +/- ##
===========================================
- Coverage 94.16% 79.85% -14.31%
===========================================
Files 52 52
Lines 1387 1385 -2
===========================================
- Hits 1306 1106 -200
- Misses 81 279 +198 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for opening this PR. I understand why you've taken this approach, but I don't believe it's the correct approach, as whether or not the input-dimension comparison should be performed is a function of the kernel, not the input type. For example, it (should be) perfectly fine to feed a To my mind, a better implementation would be to switch the check on / off depending upon which kernel is employed. @theogf @devmotion @st-- do any of you have thoughts on this problem? I'm starting to wonder whether having this check is more hassle than it's worth, because the fix looks like it's going to be a bit tricky to implement. |
Yes, I agree, that seems to be the best and most general approach. Maybe also |
Summary
This attempts to address #512, which prevents kernels operating on vectors of unequal length from working properly.
Proposed changes
Remove the check on input dimensions when the inputs are vectors of vectors, since there is nothing wrong in principal with having kernels operate on these kinds of inputs.
What alternatives have you considered?
The most obvious alternative would be to implement my own type, wrapping AbstractVector{<:AbstractVector{<:Real}}, but since this change would potentially benefit others with similar needs, a fix in KernelFunctions itself seemed more appropriate.
Breaking changes
The main change is that inputs checks that are currently failing would pass. This is potentially breaking, as it is possible that downstream code could rely on these checks.