Fix input types, improve readability #369
Conversation
docs/src/api.md
Outdated
| @@ -88,3 +88,9 @@ kernelpdmat | |||
| nystrom | |||
| NystromFact | |||
| ``` | |||
|
|
|||
| ## Optional Utilities | |||
| If the [Kronecker.jl](https://github.com/MichielStock/Kronecker.jl) package is loaded, KernelFunctions also provides the following functions. | |||
There was a problem hiding this comment.
It's always provided but it doesn't work without Kronecker.jl?
There was a problem hiding this comment.
It is in via Requires, so if I understand correctly it is not available without Kronecker.jl?
There was a problem hiding this comment.
Have reformulated, is it clearer now?
Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com>
There was a problem hiding this comment.
I'm happy with this PR, modulo my style-related comments, so I'm happy for this to be merged without me re-reviewing once the things being discussed with @theogf and @devmotion are resolved.
Co-authored-by: willtebbutt <wt0881@my.bristol.ac.uk>
| @@ -88,3 +88,12 @@ kernelpdmat | |||
| nystrom | |||
| NystromFact | |||
| ``` | |||
|
|
|||
| ## Conditional Utilities | |||
| To keep the dependencies of KernelFunctions lean, some functionality is only available if specific other packages are explicitly loaded (`using`). | |||
There was a problem hiding this comment.
also the case for kernelpdmat above, which requires PDMats.jl
There was a problem hiding this comment.
We mentioned it should/could be done in a different PR (also to add kernelkronmat)
Co-authored-by: st-- <st--@users.noreply.github.com>
|
Going to merge this later today if that is ok with everyone? I think remaining issues can be addressing in later PRs? |
Summary
As a follow up for the recent #354 pull request this PR changes the input signatures to prevent unintended behaviour in specialized
kernelmatrixandkronecker_kernelmatriximplementations.Proposed changes
Changing signatures to implicitly enforce same input types (Bug Fix)
Adding a doc string to
kronecker_kernelmatrixWhat alternatives have you considered?
None
Breaking changes
None