[WIP] Use scalar parameters with ParameterHandling #397
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| # or just `@noparams GibbsKernel` - it would be safer since there is no | ||
| # default fallback for `flatten` | ||
| function ParameterHandling.flatten(::Type{T}, k::GibbsKernel) where {T<:Real} |
There was a problem hiding this comment.
Is flatten on Function defined? If yes I think it is fine to throw an error and to tell to implement the needed function for the used struct
|
|
||
| See also: [ParameterHandling.jl](https://github.com/invenia/ParameterHandling.jl) | ||
| """ | ||
| struct ParameterKernel{P,K} <: Kernel |
There was a problem hiding this comment.
I chose Parameter because I felt many kernels are parameterized and hence Paramet(e)rizedKernel might be confusing.
There was a problem hiding this comment.
Actually didn't we mention another more meaningful name like KernelLayer or something of this taste? Maybe KernelModel?
There was a problem hiding this comment.
Yes, I think in my initial sketch I used KernelLayer (probably since we discussed its use with Flux at this point) and in the more concrete suggestion I used KernelModel. I thought ParameterKernel was a bit better but I'm not sold on the name and change it to whatever we think is best.
I know, and initially I used it in some places 🙂 However, it requires Julia >= 1.4 and hence does not work with Julia 1.3. |
|
Since this is going to a be a (massively) breaking release, maybe we can drop support < 1.6 |
|
I would be fine with this but I didn't want to make this change without prior discussion. Also I assume Julia 1.6 might be the LTS before this PR is ready and merged 🙂 |
|
|
||
| See also: [ParameterHandling.jl](https://github.com/invenia/ParameterHandling.jl) | ||
| """ | ||
| struct ParameterKernel{P,K} <: Kernel |
There was a problem hiding this comment.
Yes, I think in my initial sketch I used KernelLayer (probably since we discussed its use with Flux at this point) and in the more concrete suggestion I used KernelModel. I thought ParameterKernel was a bit better but I'm not sold on the name and change it to whatever we think is best.
Co-authored-by: Théo Galy-Fajou <theo.galyfajou@gmail.com>
Codecov ReportAttention:
... and 21 files with indirect coverage changes 📢 Thoughts on this report? Let us know! |
This PR implements #299 (comment). See the comment and the additional explanations in the issue for more details.
This PR is still WIP and tests will fail (I assume).