STOR-i / GaussianProcesses.jl

A Julia package for Gaussian Processes
https://stor-i.github.io/GaussianProcesses.jl/latest/
Other
308 stars 53 forks source link

More readable kernel names #145

Open ali-ramadhan opened 4 years ago

ali-ramadhan commented 4 years ago

Hi I'm new to Gaussian processes and just started integrating Gaussian process regression with an existing machine learning + probabilistic programming pipeline to develop better surrogate models for climate models. Thanks for the developing this package, I really like the interface!

One thing I am struggling with is the kernel names are all abbreviated (sometimes inconsistently) which makes my scripts quite unreadable as you have to be familiar with GaussianProcesses.jl to understand lines like RQ(0.0,0.0,-1.0) + SE(-2.0,-2.0).

I wonder if the developers think it would be a good idea to introduce more complete and readable names for kernels. Some examples of what I'm suggesting:

I believe introducing more readable names will make scripts that depend on GaussianProcesses.jl more readable and easier to understand, especially for people not familiar with Gaussian processes and common kernels.

I understand this would be a pretty big breaking change. Perhaps introduces aliases, e.g. const SquaredExponential = SE would address this issue without breaking anything.

ali-ramadhan commented 4 years ago

Could also introduce keyword arguments to make kernels easier to use/debug and understand, but maybe this is a separate issue.

For example, instead of

kernel = SE(-2.0,-2.0)  # Quite cryptic

this would be more readable and easier to debug

kernel = SquaredExponential(ll=-2.0, lσ=-2.0)  # More readable

or maybe even better

# Should be quite understandable by someone not familiar with Gaussian processes.
kernel = SquaredExponential(length_scale=-2.0, signal_std=-2.0)
thomaspinder commented 4 years ago

Hi @ali-ramadhan, thanks for your suggestion. I'm inclined to agree with you on the renaming of the Kernel structs i.e. SE to SquaredExponential. Given the @deprecate macro in Julia, I don't think such a renaming would be too much of a breaking change.

Unfortunately, due to time constraints, it's unlikely to be something I'd consider doing before May. If @chris-nemeth agrees that it's a worthwhile alteration, then I'd be happy to get this done in May though.

maximerischard commented 4 years ago

I would be in favour of this. @ali-ramadhan, would you be willing to contribute a pull request? I agree with @thomaspinder that proper deprecation is important.

chris-nemeth commented 4 years ago

I also agree that this would be worth doing. A pull request from @ali-ramadhan would be great. If not, then we could probably do this ourselves in a month or so. I think it should be straightforward to do this with a string search and replace and then add in the deprecation warning that @thomaspinder mentioned.

ali-ramadhan commented 4 years ago

Glad to hear it's a welcome change! I'm happy to open a PR to start things off. Pretty familiar with Julia but not GaussianProcesses.jl so should be a useful exercise.

I'll probably just change the names to start and add @deprecate macros. Adding keyword arguments might be a little messy in the same PR.

thomaspinder commented 4 years ago

Thanks, @ali-ramadhan . Any problems, then just post something here and one of us will give you a hand.