Examples in the docs: base PR for cleaning up notebook workflow #303
Conversation
…ing blacklisted right now)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
docs/make.jl
Outdated
| const BLACKLIST = [ | ||
| "deepkernellearning", | ||
| "kernelridgeregression", | ||
| "svm", | ||
| ] |
There was a problem hiding this comment.
[JuliaFormatter] reported by reviewdog 🐶
| const BLACKLIST = [ | |
| "deepkernellearning", | |
| "kernelridgeregression", | |
| "svm", | |
| ] | |
| const BLACKLIST = ["deepkernellearning", "kernelridgeregression", "svm"] |
There was a problem hiding this comment.
This is really nice, but showing how I could batch-format the code before committing/pushing would be even nicer. :) @devmotion
(one of the things I would like to cover in #251)
There was a problem hiding this comment.
in case you need it:
using JuliaFormatter # you should add it to your global env
format(".")There was a problem hiding this comment.
Yeah it's in the Makefile in #251:)
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…KernelFunctions.jl into st/examples-base
| @@ -0,0 +1,35 @@ | |||
| # How to build the docs locally | |||
There was a problem hiding this comment.
Happy to get improvements to the workflow. :)
docs/Project.toml
Outdated
| @@ -1,6 +1,12 @@ | |||
| [deps] | |||
| Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f" | |||
There was a problem hiding this comment.
I think it would be cleaner and probably easier to maintain (less conflicting dependencies etc) if docs/Project.toml only contains the dependencies for the actual documentation and each example uses a separate project environment with its own pinned dependencies.
There was a problem hiding this comment.
I have played around with this in https://github.com/devmotion/CalibrationErrors.jl.
There was a problem hiding this comment.
Yes, I agree - I was wondering how you would make that work but the Pkg.activate() context manager is really neat, so I'll try to add that :)
There was a problem hiding this comment.
Mhm, that would mean having to run the Pkg.develop in every single example subdirectory if you want to work on them though, and would that not make it harder to check the examples run consistently with any future changes to KernelFunctions (in a PR)?
There was a problem hiding this comment.
Mhm, that would mean having to run the Pkg.develop in every single example subdirectory if you want to work on them
Yes, the main point is to have independent environments. In this way you can avoid that dependencies of different examples are conflicting and it is clear which packages and which versions were used to run the individual examples.
They are re-run every time the documentation is built so one would just check that the build passes and the webpage looks fine.
There was a problem hiding this comment.
Hm, for the examples to actually pull in the in-development version of the package I think your make.jl would need to also include a Pkg.develop(path=joinpath(@__DIR__, "..")), but with that I think it should work:)
I think it'd still be good to provide a Makefile or some other way of making it easy for contributors to locally run tests, formatter, etc..
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…KernelFunctions.jl into st/examples-base
IMO this is a motivation for splitting the tests into different groups that are run in parallel (similar what we do in the SciML ecosystem: https://github.com/SciML/OrdinaryDiffEq.jl/blob/master/test/runtests.jl) rather than running the examples in serial. |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: David Widmann <[email protected]>
This comment has been minimized.
This comment has been minimized.
…KernelFunctions.jl into st/examples-base
This reverts commit 64d5ef7.
| #md # notebook can be viewed in [nbviewer](@__NBVIEWER_ROOT_URL__/examples/@__NAME__.ipynb). | ||
| #nb # HTML output can be viewed [here](https://juliagaussianprocesses.github.io/KernelFunctions.jl/dev/examples/@__NAME__/). | ||
| #md # The corresponding notebook can be viewed in [nbviewer](@__NBVIEWER_ROOT_URL__/examples/@__NAME__.ipynb).* | ||
| #nb # The rendered HTML can be viewed [in the docs](https://juliagaussianprocesses.github.io/KernelFunctions.jl/dev/examples/@__NAME__/).* |
There was a problem hiding this comment.
How difficult would it be to change the dev to whichever build it actually is?
There was a problem hiding this comment.
Not difficult but a bit annoying it seems. Literate automatically derives the url for the repo and nbviewer when running it with Github actions which are used to replace @__NBVIEWER_ROOT_URL__ and @__REPO_ROOT_URL__ after the custom user-defined replacements. However, we can't use these urls or subparts of them in our preprocessing and there are no similar "magic" variables for the github.io webpage (which seems reasonable somehow since one can use e.g. custom domains and hence it is not clear what the correct url for the webpage should be). So I think one has to copy the logic in https://github.com/fredrikekre/Literate.jl/blob/8d8caac6e651eb02ded5699835384d0f0b139e78/src/Literate.jl#L248-L258 to derive the deploy_folder and use it instead of dev. Maybe one could ask if Literate could add an automatic replacement for @__DEPLOY_FOLDER__, similar to @__REPO_ROOT_URL__.
There was a problem hiding this comment.
Thanks for looking into this. Would you be willing to write it up as an issue in the Literate.jl repo ?
There was a problem hiding this comment.
Awesome, thank you @fredrikekre !
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| using LinearAlgebra | ||
| using Random | ||
| using PDMats: PDMats |
There was a problem hiding this comment.
Are these not needed anymore for the doctests?
| plots = [] | ||
| for i in 1:10 | ||
| grads = Zygote.gradient(() -> loss(k, λ), ps) | ||
| Flux.Optimise.update!(opt, ps, grads) | ||
| p = Plots.scatter(x, y; lab="data", title="Loss = $(loss(k,λ))") | ||
| Plots.plot!(x, f(X, k, λ); lab="Prediction", lw=3.0) | ||
| display(p) | ||
| push!(plots, p) | ||
| end | ||
|
|
||
| # | ||
|
|
||
| l = @layout grid(10, 1) | ||
| plot(plots...; layout=l, size=(300, 1500)) |
There was a problem hiding this comment.
Probably better to generate a gif but IMO it's OK to leave this for a future PR.
| plots = [] | ||
| for i in 1:10 | ||
| grads = Zygote.gradient(() -> loss(k, λ), ps) | ||
| Flux.Optimise.update!(opt, ps, grads) | ||
| p = Plots.scatter(x, y; lab="data", title="Loss = $(loss(k,λ))") | ||
| Plots.plot!(x_test, f(X_test, k, λ); lab="Prediction", lw=3.0) | ||
| display(p) | ||
| push!(plots, p) | ||
| end |
There was a problem hiding this comment.
Again might want to change this to an animation later on.
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: David Widmann <[email protected]>
st--
changed the title
Following #314, this PR adds READMEs for how to build & add to the docs and examples, as well as tidying the workflow up a bit more. It makes all current examples run (with a warning to say they're under construction). This PR is NOT intended to clean up the notebooks themselves - that'll come in later PRs.