-
Notifications
You must be signed in to change notification settings - Fork 36
Multi-Output helper function #345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Did not go into details but the obvious edge case not considered here is if N == P |
Co-authored-by: st-- <st--@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good to me, but I think it'd be super helpful to include a concrete example with explicit numbers in the docstrings, as even being somewhat familiar with all this it keeps doing my head in. Do doctests count towards code coverage? It might make more sense to move the "consistency checks" to actual tests.
Co-authored-by: st-- <st--@users.noreply.github.com>
Co-authored-by: st-- <st--@users.noreply.github.com>
…ussianProcesses/KernelFunctions.jl into wct/multi-output-helpers
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me; no idea why the doctest fails - it should be caught by the filter! @theogf was going to have a look into that
Just trying something now |
Co-authored-by: st-- <st--@users.noreply.github.com>
It works locally at least! |
Co-authored-by: st-- <st--@users.noreply.github.com>
@theogf your regex magic seems to have sorted it. Have bumped patch -- will merge once CI passes. |
…ussianProcesses/KernelFunctions.jl into wct/multi-output-helpers
Implements a function to make it possible to work with our input types for multi-output GPs more straightforwardly in the isotopic case.
It's entirely possible that we'll want more helper functions as we move forwards, so this should be seen as a first step in that direction, rather than something that will solve all useability problems.
edit: I've added a note to the docs. Unless I'm missing something obvious, we don't currently have a multi-output example, so I'm not going to create one in this PR to showcase this functionality.
edit2: also note that I'm testing only via doctests. It feels a bit weird, but it's not obviously to me that it's obviously a bad idea, so maybe it's okay?