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

Preserve Float type for Periodic Kernel #560

Merged
merged 5 commits into from
Jul 22, 2024

Conversation

molet
Copy link
Contributor

@molet molet commented Jul 8, 2024

Summary
Tiny change to preserve float type of inputs and parameters. For most of the kernels, if the inputs and parameters have Float32 type, then the output has also Float32:

julia> v1 = 0.9f0; v2 = 0.7f0;
julia> SqExponentialKernel()(v1, v2)
0.9801987f0
julia> GammaRationalKernel(alpha=2.0f0, gamma=1.0f0)(v1, v2)
0.82644624f0

However, currently, PeriodicKernel doesn't support this:

julia> PeriodicKernel(r=[1.0f0])(v1, v2)
0.8413515018155655

The simple change of exp(-0.5d) to exp(-d / 2) in its kappa function fixes it:

julia> PeriodicKernel(r=[1.0f0])(v1, v2)
0.8413515f0

Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

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

Looks good to me, can you add a test for the fix?

@molet
Copy link
Contributor Author

molet commented Jul 8, 2024

Certainly, it's been added.

Copy link

codecov bot commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 16.29%. Comparing base (c38b366) to head (8686dd1).

Files Patch % Lines
src/basekernels/periodic.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #560       +/-   ##
===========================================
+ Coverage    0.42%   16.29%   +15.87%     
===========================================
  Files          52       52               
  Lines        1423     1430        +7     
===========================================
+ Hits            6      233      +227     
+ Misses       1417     1197      -220     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
Copy link
Member

@willtebbutt willtebbutt left a comment

Choose a reason for hiding this comment

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

LGTM

@willtebbutt
Copy link
Member

The changes that you've made do not appear to have anything to do with the test failures, so I'm going to merge.

@willtebbutt willtebbutt merged commit ab866b9 into JuliaGaussianProcesses:master Jul 22, 2024
12 of 19 checks passed
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.

3 participants