Use only docstrings in documentation + fix them by devmotion · Pull Request #236 · JuliaGaussianProcesses/KernelFunctions.jl

devmotion · 2021-01-19T02:16:06Z

Addresses #218 and #216 and uses the docstrings in the documentation of kernel functions and transformation.

The docstrings are updated and synced with the documentation, a bunch of errors are fixed (e.g., incorrect Matérn kernel functions and rational quadratic kernels), formatting of most docstrings of kernels and transformations is fixed (basically everything apart from the Wiener kernel and spectral mixture kernel, I was too tired to fix the mess there), doctests are added to most transformations, and incorrect explanations and outdated code of the transformations are fixed.

devmotion · 2021-01-19T10:24:36Z

-        @check_args(GammaExponentialKernel, γ, γ >= zero(T), "γ > 0")
-        return new{T}([γ])
+    function GammaExponentialKernel(; gamma::Real=2.0, γ::Real=gamma)
+        @check_args(GammaExponentialKernel, γ, zero(γ) < γ ≤ 2, "γ ∈ (0, 2]")


Otherwise the kernel is not positive definite (the case 0 is not interesting and was excluded in the docstring but not in the check itself).

devmotion · 2021-01-19T10:25:37Z

-        @assert 0.0 <= h <= 1.0 "FBMKernel: Given Hurst index h is invalid."
-        return new{T}([h])
+    function FBMKernel(; h::Real=0.5)
+        @check_args(FBMKernel, h, zero(h) ≤ h ≤ one(h), "h ∈ [0, 1]")


Also here h=0 is uninteresting.

devmotion · 2021-01-19T10:27:36Z

+
+For inputs ``x, x' \\in \\mathbb{R}^d``, the Matérn kernel of order ``3/2`` is given by
+```math
+k(x, x') = \\big(1 + \\sqrt{3} \\|x - x'\\|_2 \\big) \\exp\\big(- \\sqrt{3}\\|x - x'\\|_2\\big).


The current documentation is incorrect, it is missing the minus sign inside the exponential.

devmotion · 2021-01-19T10:28:03Z

+For inputs ``x, x' \\in \\mathbb{R}^d``, the Matérn kernel of order ``5/2`` is given by
+```math
+k(x, x') = \\bigg(1 + \\sqrt{5} \\|x - x'\\|_2 + \\frac{5}{3}\\|x - x'\\|_2^2\\bigg)
+           \\exp\\big(- \\sqrt{5}\\|x - x'\\|_2\\big).


Same here, the current docs are missing the minus sign.

devmotion · 2021-01-19T10:33:20Z

+For inputs ``x, x' \\in \\mathbb{R}^d``, the rational-quadratic kernel with shape parameter
+``\\alpha > 0`` is defined as
+```math
+k(x, x'; \\alpha) = \\bigg(1 + \\frac{\\|x - x'\\|_2^2}{2\\alpha}\\bigg)^{-\\alpha}.


Currently, docstring and documentation are not consistent - the documentation is missing a factor of 2 in the denominator. Since the docstring and code were consistent, I only fixed the format of the docstring. IIRC you changed this a while back @willtebbutt?

devmotion · 2021-01-19T10:47:16Z

-function kappa(κ::GammaRationalQuadraticKernel, d²::Real)
-    return (one(d²) + d²^(first(κ.γ) / 2) / first(κ.α))^(-first(κ.α))
+function kappa(κ::GammaRationalQuadraticKernel, d::Real)
+    return (one(d) + d^first(κ.γ) / (2 * first(κ.α)))^(-first(κ.α))


Is there a reason for using the SqEuclidean distance and dividing gamma by 2 instead of this approach, @willtebbutt?

devmotion · 2021-01-19T11:12:59Z

        @test KernelFunctions.iskroncompatible(k) == true

-        test_ADs(γ -> GammaExponentialKernel(; gamma=first(γ)), [γ])
+        test_ADs(γ -> GammaExponentialKernel(; gamma=first(γ)), [1 + 0.5 * rand()])


Ideally, one should use a reparameterization R -> (0,2] here, e.g. f(x) = 2 * logistic(x). With the current boundary point gamma = 2 finite differencing errors since it steps outside the domain.

willtebbutt

This is really fantastic work. Just a few things to fix.

Co-authored-by: willtebbutt <wct23@cam.ac.uk>

willtebbutt

Thanks for addressing my points. Now LGTM.

st-- · 2021-01-20T11:22:47Z

The downside of this is that while the maths may look nice on the rendered docs online, the REPL help is now rather less helpful compared to the previous use of unicode maths in the docstrings, e.g.:

  MaternKernel(; ν::Real=1.5)

  Matérn kernel of order ν.

  Definition
  ≡≡≡≡≡≡≡≡≡≡≡≡

  For inputs x, x' \in \mathbb{R}^d, the Matérn kernel of order \nu > 0 is defined as

  k(x,x';\nu) = \frac{2^{1-\nu}}{\Gamma(\nu)}\big(\sqrt{2\nu}\|x-x'\|_2\big) K_\nu\big(\sqrt{2\nu}\|x-x'\|_2\big),

  where \Gamma is the Gamma function and K_{\nu} is the modified Bessel function of the second kind of order \nu.

  A Gaussian process with a Matérn kernel is \lceil \nu \rceil - 1-times differentiable in the mean-square sense.

How about keeping the unification (:+1:) but using unicode for maths as in the docstrings before?

devmotion · 2021-01-20T11:33:56Z

I tried this recently in some other package but unfortunately unicode math does not work reliably with Documenter (or the javascript libraries). Greek letters are displayed in a different non-math font, brackets can not be adjusted properly, unicode supports only a limited number of sub- and superscripts etc. Other things such as \frac are just not possible to write with unicode characters only. So in any case one would end up with a mix of both LaTeX and unicode characters, which makes e.g. the differences of the fonts even more prominent and does not solve the problem of LaTeX syntax in docstrings.

devmotion · 2021-01-20T11:38:25Z

Possibly also due to these reasons, LaTeX syntax in docstrings is common practice in other packages such as Distributions (see e.g. https://github.com/JuliaStats/Distributions.jl/blob/master/src/univariate/discrete/betabinomial.jl) and standard libraries (see e.g. https://github.com/JuliaLang/julia/blob/d6f99481b1d5cf399165be2eb3893c5bdbe90dcf/stdlib/LinearAlgebra/src/qr.jl#L19)

theogf · 2021-01-20T13:04:31Z

I chime in definitely too late, but that was the reason for two versions of the docs (docstring vs docs.md), unicode readable for docstring and latex for documenter.

devmotion · 2021-01-20T13:59:23Z

I definitely agree that it would be nice to have some more readable output in the REPL and I tried to use unicode but it does not work. Only the simplest formulas can be written with unicode only, so one basically always has to write them at least partly with LaTeX syntax. And even the simplest unicode formulas look terrible on the webpage (e.g. even subscripts and operators such as exp).

devmotion · 2021-01-20T14:03:15Z

On a different note, it might also be problematic if the docstring contains characters that are only supported by very few fonts.

Use only docstrings in documentation + fix them

4ebdecf

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread docs/src/kernels.md Outdated

devmotion and others added 6 commits January 19, 2021 03:17

Update docs/src/kernels.md

d59f5ab

Fix typo

662e252

Fix LaTeX

f18c8b1

Add missing @ref

f7d3621

Fix tests

8db1c3d

Fix format

d17e753

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread src/basekernels/rationalquad.jl Outdated

devmotion commented Jan 19, 2021

View reviewed changes

Revert reparameterization and extend docstring

120c957

devmotion commented Jan 19, 2021

View reviewed changes

devmotion requested a review from willtebbutt January 19, 2021 11:22

Update docstring of Wiener kernel

fd8fdbc

willtebbutt requested changes Jan 19, 2021

View reviewed changes

Comment thread docs/src/transform.md Outdated

Comment thread docs/src/transform.md

Comment thread docs/src/kernels.md

Comment thread src/kernels/scaledkernel.jl

Comment thread src/kernels/transformedkernel.jl

Comment thread src/kernels/transformedkernel.jl

devmotion mentioned this pull request Jan 19, 2021

Inconsistency in rational kernels #238

Closed

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread src/kernels/transformedkernel.jl Outdated

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread src/kernels/transformedkernel.jl Outdated

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread src/kernels/transformedkernel.jl

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread src/kernels/scaledkernel.jl Outdated

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread docs/src/kernels.md

devmotion commented Jan 19, 2021

View reviewed changes

Comment thread docs/src/transform.md Outdated

devmotion and others added 3 commits January 19, 2021 15:48

Apply suggestions from code review

4a2a844

Co-authored-by: willtebbutt <wct23@cam.ac.uk>

Try to reformat kernel functions

dd2223e

Fix links

befebd9

devmotion added 3 commits January 19, 2021 17:08

Update transform.md

93ac89e

Fix typo

4bbac93

Update according to @willtebbut's suggestion

44e5e27

willtebbutt approved these changes Jan 19, 2021

View reviewed changes

devmotion added 2 commits January 19, 2021 17:45

Fix some remaining inconsistencies

d2317b7

Fix two links

f0f7b70

devmotion merged commit 45226b0 into master Jan 19, 2021

devmotion deleted the dw/docs branch January 19, 2021 18:27

This was referenced Jan 19, 2021

Consolidate mathematical description of kernels between docs and docstrings #218

Closed

Split API docs into separate pages, remove redundancy with kernels/transforms #216

Closed

Conversation

devmotion commented Jan 19, 2021

Uh oh!

Uh oh!

devmotion Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

devmotion Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

devmotion Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

devmotion Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

devmotion Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devmotion Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devmotion Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willtebbutt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

willtebbutt left a comment

Choose a reason for hiding this comment

Uh oh!

st-- commented Jan 20, 2021

Uh oh!

devmotion commented Jan 20, 2021

Uh oh!

devmotion commented Jan 20, 2021

Uh oh!

theogf commented Jan 20, 2021

Uh oh!

devmotion commented Jan 20, 2021

Uh oh!

devmotion commented Jan 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

devmotion Jan 19, 2021 •

edited

Loading

devmotion Jan 19, 2021 •

edited

Loading