Skip to content
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

Example notebook: deep kernel learning #322

Closed
wants to merge 6 commits into from

Conversation

st--
Copy link
Member

@st-- st-- commented Jul 1, 2021

No description provided.

@st-- st-- mentioned this pull request Jul 1, 2021
using Flux
using Distributions, LinearAlgebra
using Plots
using ProgressMeter
using AbstractGPs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also introduces a circular dependency and hence leads to the same problems as mentioned in #316 (comment). It might be better to move it to AbstractGPs or the JuliaGaussianProcesses webpage. We can always link to these examples from the documentation, so it would still be possible to make it discoverable from the KernelFunctions docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that each example has its own project environment & pinning, shouldn't this no longer be a concern? The example depends on KernelFunctions and AbstractGPs, and AbstractGPs depends on KernelFunctions, but neither KernelFunctions nor AbstractGPs depend on the example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractGPs depends on KernelFunctions

This is exactly the problem and the circular dependency I was referring to. If we make breaking changes in KernelFunctions, the example will break - there is no version of AbstractGPs that is compatible with this version yet. Hence either we have to switch to an old version of KernelFunctions and there will be a disconnect between the version of KernelFunctions discussed in the documentation and used in the example until AbstractGPs is updated and the example is fixed or we have to remove the example for a while. Both alternatives seem a bit annoying.

This problem is the main reason why I think it is unfortunate to use downstream packages in the documentation of KernelFunctions and these examples should rather go into AbstractGPs where we would not have to deal with these problems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A breaking change doesn't imply the example must break, just that it's not guaranteed to not break... In any case, I'm fine with moving this notebook over to AbstractGPs. Just wanted to save the current state out of #234.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Often the example itself won't break but this is not relevant: AbstractGPs is just not compatible with KernelFunctions before and when (and possibly for some time after) a breaking release of KernelFunctions is made. A new release of AbstractGPs is needed with updated compatibility bounds and possibly some fixes, even if the example itself does not require any changes.

@st-- st-- added the help wanted Extra attention is needed label Jul 1, 2021
@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #322 (e1d1a57) into master (6033b56) will decrease coverage by 40.06%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master     #322       +/-   ##
===========================================
- Coverage   83.21%   43.14%   -40.07%     
===========================================
  Files          52       52               
  Lines        1251     1240       -11     
===========================================
- Hits         1041      535      -506     
- Misses        210      705      +495     
Impacted Files Coverage Δ
src/basekernels/sm.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/gabor.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/cosine.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/constant.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/periodic.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/rational.jl 0.00% <0.00%> (-100.00%) ⬇️
src/basekernels/nn.jl 0.00% <0.00%> (-97.62%) ⬇️
src/basekernels/wiener.jl 0.00% <0.00%> (-92.86%) ⬇️
src/basekernels/piecewisepolynomial.jl 0.00% <0.00%> (-89.48%) ⬇️
src/kernels/gibbskernel.jl 0.00% <0.00%> (-88.89%) ⬇️
... and 33 more

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 6033b56...e1d1a57. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants