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

More readable kernel names #145

Open
ali-ramadhan opened this issue Apr 2, 2020 · 6 comments
Open

More readable kernel names #145

ali-ramadhan opened this issue Apr 2, 2020 · 6 comments

Comments

@ali-ramadhan
Copy link

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:

  • Lin -> Linear
  • LinArd -> LinearARD ("ARD" should be capitalized as it's an abbreviation)
  • SE -> SquaredExponential
  • Mat52Iso -> Matern52Isotropic

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
Copy link
Author

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Author

ali-ramadhan commented Apr 6, 2020

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants