Skip to content

Fix typo #42

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

Merged
merged 2 commits into from
Mar 12, 2020
Merged

Fix typo #42

merged 2 commits into from
Mar 12, 2020

Conversation

willtebbutt
Copy link
Member

No description provided.

@willtebbutt willtebbutt requested a review from theogf March 11, 2020 15:07
@willtebbutt
Copy link
Member Author

This actually might require renaming the internal parameter of ScaledKernel to avoid weird naming inconsistencies, but this is definitely the right idea. The parameter of the scaled kernel is its variance, as opposed to std. dev.

Copy link
Member

@theogf theogf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a reasonable change. I will then do all the renaming of ScaledKernel

@willtebbutt
Copy link
Member Author

That sounds like a reasonable change. I will then do all the renaming of ScaledKernel

@theogf no need, I've just done it!

@theogf
Copy link
Member

theogf commented Mar 11, 2020

Just saw that. Maybe that would be good to also add a variance function that returns sigma for a ScaledKernel and 1 for other kernels? It just might end up complicated for nested kernels...

@willtebbutt
Copy link
Member Author

Just saw that. Maybe that would be good to also add a variance function that returns sigma for a ScaledKernel and 1 for other kernels? It just might end up complicated for nested kernels...

This sounds reasonable. Could we do it in a different PR though please?

@theogf theogf merged commit 716285c into master Mar 12, 2020
@willtebbutt willtebbutt deleted the wct/fix-typo branch May 3, 2020 19:15
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

Successfully merging this pull request may close these issues.

2 participants