KernelFunctions.jl icon indicating copy to clipboard operation
KernelFunctions.jl copied to clipboard

Add `MOInputHeterotopic` type & helpers

Open thomasgudjonwright opened this issue 2 years ago • 6 comments

Summary

This closes #358 which proposes making a MOInputsHeterotopic type and some supporting infrastructure. It is meant for multi-ouput GPs where not all data is observed at every output.

thomasgudjonwright avatar Oct 27 '21 21:10 thomasgudjonwright

Codecov Report

Merging #393 (24bbeb3) into master (ab30149) will decrease coverage by 0.08%. The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   89.23%   89.14%   -0.09%     
==========================================
  Files          52       52              
  Lines        1198     1207       +9     
==========================================
+ Hits         1069     1076       +7     
- Misses        129      131       +2     
Impacted Files Coverage Δ
src/KernelFunctions.jl 100.00% <ø> (ø)
src/mokernels/moinput.jl 95.12% <77.77%> (-4.88%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ab30149...24bbeb3. Read the comment docs.

codecov[bot] avatar Oct 27 '21 21:10 codecov[bot]

What's the advantage of a separate type MOInputHeterotopic{T} over a Vector{Tuple{T,Int}} & zip(x, output_indices)?

st-- avatar Oct 28 '21 06:10 st--

Actually I just re-discovered prepare_heterotopic_multi_output_data from #353 :)

To clarify what I'm trying to understand: what are the use-cases for a new concrete type? What things could we do (at all, or rather more easily) with it that would not work (or at least not so easily) with only Vector{Tuple{T,Int}} (and perhaps a type alias)?

I'm a bit wary of adding code without concrete use-cases: to accept an increase in the maintenance burden, it'd be good to see the benefits that it brings! So I think it'd be good to demonstrate them, ideally in an example notebook...

st-- avatar Oct 28 '21 07:10 st--

Actually I just re-discovered prepare_heterotopic_multi_output_data from #353 :)

To clarify what I'm trying to understand: what are the use-cases for a new concrete type? What things could we do (at all, or rather more easily) with it that would not work (or at least not so easily) with only Vector{Tuple{T,Int}} (and perhaps a type alias)?

I'm a bit wary of adding code without concrete use-cases: to accept an increase in the maintenance burden, it'd be good to see the benefits that it brings! So I think it'd be good to demonstrate them, ideally in an example notebook...

Hmm this is a good point I hadn't really considered. I suppose most, if not all of this can be done with simply a Vector{Tuple{T,Int}}. Maybe this is unnecessary.

Are we able to create helper methods for an alias? Having support functions explicity for heterotopic data is the only potential benefit I see beyond just having a clear object named for every type of MO-input we can imagine. @willtebbutt do you have any thoughts on this?

thomasgudjonwright avatar Nov 04 '21 15:11 thomasgudjonwright

To be honest, in the absence of a really clear example, I'm not sure what this gets us over just using a Vector{Tuple}, so my inclination would be to stick with that for now.

I could imagine that the internal representation you've got here could be beneficial in some operations (I don't have any good examples though), but in such cases I'd be inclined to just use a StructArray.

willtebbutt avatar Nov 04 '21 16:11 willtebbutt

Maybe we could mark the PR with a (new) label "pending clear need" for now? :slightly_smiling_face:

devmotion avatar Nov 04 '21 18:11 devmotion