Update implementations of `show`

[devmotion]

There are mainly two things that can be improved in the current implementations of show: one can remove string interpolation and implement show(::IO, ::MIME"text/plain", x) for more verbose multi-line output and show(::IO, x) for compact one-line output (see https://docs.julialang.org/en/v1/manual/types/#man-custom-pretty-printing-1). The second point is particularly important for printing containers of Kernels and printing of kernels in other packages. For instance, currently one gets:

julia> [SqExponentialKernel(), LinearKernel() ⊗ GammaRationalQuadraticKernel(), LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))]
3-element Array{Kernel,1}:
 Squared Exponential Kernel
 Tensor product of 2 kernels:
        Linear Kernel (c = 0.0)
        Gamma Rational Quadratic Kernel (α = 2.0, γ = 2.0)
 Sum of 2 kernels:
        Linear Kernel (c = 0.0)
        Product of 2 kernels:
                        Squared Exponential Kernel
                                        - σ² = 4.0
                        Linear Kernel (c = 2.0)
                                        - Scale Transform (s = 3.0)

(Arguably, also \t messes up the output.)

This PR is a proposal that fixes both issues. I added missing compact output formats, such that the example is printed as

julia> [SqExponentialKernel(), LinearKernel() ⊗ GammaRationalQuadraticKernel(), LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))]
3-element Array{Kernel,1}:
 Squared Exponential Kernel
 Linear Kernel (c = 0.0) ⊗ Gamma Rational Quadratic Kernel (α = 2.0, γ = 2.0)
 Linear Kernel (c = 0.0) + ((4.0 * Squared Exponential Kernel) * (Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0)))

instead. I.e., the main idea is to more explicitly display the mathematical definition of each kernel and adding parentheses where needed. I adopted the same format for the more verbose output, similar to the example in the Julia documentation, but maybe some other format that is more similar to the previous output would be preferable. E.g., maybe one could write down the compact form of the factors of a KernelProduct in different lines (but not expand lower levels). In this case, I think I would also prefer if \t is replaced with explicit whitespace characters. Some more verbose examples:

julia> LinearKernel() ⊗ GammaRationalQuadraticKernel()
Tensor product of 2 kernels:
   Linear Kernel (c = 0.0) ⊗ Gamma Rational Quadratic Kernel (α = 2.0, γ = 2.0)

julia> LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))
Sum of 2 kernels:
   Linear Kernel (c = 0.0) + ((4.0 * Squared Exponential Kernel) * (Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0)))

Additionally, one could also limit the amount of nested levels that are shown and add "..." as a place holder at some point.

[theogf]

I really like the compact representation, this is definitely a huge plus! For the extended representation, I agree \t is probably too much, however having one kernel per line make it much more readable. Regarding the display of the transform/variance of the kernel I agree that it could be used in a compact way if it's already in a tree, however for a composite kernel it would nicer to keep seeing a tree structure. Maybe an example would be clearer:

julia> LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))
Sum of 2 kernels:
  Linear Kernel (c = 0.0)
  Product of 2 kernels
    4.0 * Squared Exponential Kernel
    Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0)

To avoid recoding everything we could also use the printing from AbstractTrees.jl. We just have to define children(k::KernelSum) etc... and printnode(io::IO, k::Kernel...) and then print_tree(k) does the trick :)

[devmotion]

My alternative suggestion above was

julia> LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))
Sum of 2 kernels:
  Linear Kernel (c = 0.0)
  (4.0 * Squared Exponential Kernel) * (Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0))

Personally, I think it is difficult to distinguish summaries and kernels and tell apart the different levels of kernels in your suggestion. Also the type is really a KernelSum, so to me it seems natural to focus on the sum in the presentation.

I'm a bit sceptical regarding AbstractTrees. Since I don't view kernels as trees (or leaves of a tree), I think it's a bit weird to implement its interface only to achieve a tree-like display. I also assume that it is simple enough to achieve a tree-like output with only an internal function, similar to print_nested for the compact output.

[theogf]

My arguments is indeed more about simplicity, here is how it would look like :

julia> using AbstractTrees, KernelFunctions

julia> k = LinearKernel() + (4.0 * SqExponentialKernel() * transform(LinearKernel(; c = 2.0), 3.0))
 Sum of 2 kernels:
   Linear Kernel (c = 0.0) + ((4.0 * Squared Exponential Kernel) * (Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0)))

julia> AbstractTrees.children(k::Union{KernelSum, KernelProduct}) = k.kernels

julia> AbstractTrees.children(k::Kernel) = ()

julia> AbstractTrees.printnode(io::IO, k::KernelSum) = print(io, "Sum of $(length(k.kernels)) kernels")

julia> AbstractTrees.printnode(io::IO, k::KernelProduct) = print(io, "Product of $(length(k.kernels)) kernels")

julia> AbstractTrees.printnode(io::IO, k::Kernel) = show(io, k)

julia> print_tree(k)
Sum of 2 kernels
├─ Linear Kernel (c = 0.0)
└─ Product of 2 kernels
   ├─ 4.0 * Squared Exponential Kernel
   └─ Linear Kernel (c = 2.0) ∘ Scale Transform (s = 3.0)

I think it's a bit weird to implement its interface only to achieve a tree-like display

I am actually wondering if it could be useful for more things...

[st--]

I like the compact format. It already looks almost like the syntax for constructing the given kernel (composite or not), how about losing the spaces and actually making it more or less copy&pastable? might be helpful for debugging!

[devmotion]

Maybe we should revive the PR in its initial (?) state @theogf? It seems now that we retired transform the initial proposal would actually correspond to the preferred way to construct these kernels.

[theogf]

I keep thinking that having sum/products of kernels on one line is not a good idea because since we don't limit sum/products to 2 kernels this can quickly become unreadable...

Otherwise the kernel \circ transform notation is great, I am definitely on board!

@st-- I don't think having the output copy/pastable is really an argument for me. Print outputs are not really made of this, they should make things as human readable as possible. Losing spaces would not go in this direction. But that's just my opinion I guess

[devmotion]

IMO at least we should respect the convention that print(io, x) only prints a compact single-line representation and print(io, ::MIME"text/plain", x) displays a more detailed multiline representation, as e.g. when returned in the REPL. So in any case we need some compact representation that is not nested.

[willtebbutt]

@nickrobinson251 would you mind weighing in on this? My impression is that you have opinions on the various conventions for show with different numbers of arguments (I never quite grasped all the various permutations or arguments and their associated conventions).

[codecov[bot]]

Codecov Report

Merging #244 (17641ea) into master (33d64d1) will decrease coverage by 30.75%. The diff coverage is 35.55%.

@@             Coverage Diff             @@
##           master     #244       +/-   ##
===========================================
- Coverage   89.23%   58.47%   -30.76%     
===========================================
  Files          52       52               
  Lines        1198     1168       -30     
===========================================
- Hits         1069      683      -386     
- Misses        129      485      +356     

Impacted Files	Coverage Δ
src/kernels/kernelproduct.jl	36.84% <0.00%> (-50.66%)	:arrow_down:
src/kernels/kernelsum.jl	36.84% <0.00%> (-54.83%)	:arrow_down:
src/kernels/kerneltensorproduct.jl	80.51% <0.00%> (-13.31%)	:arrow_down:
src/kernels/scaledkernel.jl	38.23% <0.00%> (-50.01%)	:arrow_down:
src/kernels/transformedkernel.jl	54.54% <0.00%> (-18.19%)	:arrow_down:
src/mokernels/independent.jl	0.00% <0.00%> (-93.75%)	:arrow_down:
src/mokernels/slfm.jl	0.00% <0.00%> (-95.24%)	:arrow_down:
src/generic.jl	46.15% <55.55%> (-53.85%)	:arrow_down:
src/transform/chaintransform.jl	55.55% <60.00%> (-18.36%)	:arrow_down:
src/basekernels/periodic.jl	100.00% <100.00%> (ø)
... and 35 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 33d64d1...17641ea. Read the comment docs.

[willtebbutt]

This is stale, so I'm closing. Please re-open if you fancy it.