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

Fix SqExponential and GammaExponential + style #158

Merged
merged 8 commits into from
Sep 21, 2020
Merged

Conversation

willtebbutt
Copy link
Member

@willtebbutt willtebbutt commented Aug 30, 2020

Our implementations of the mentioned kernels were quite non-standard, so I've brought them inline with what is usually used.

I've also fixed some style issues. Apologies for conflating the two in a single PR.

The only changes of importance are those in src/exponential.jl -- everything else is either style or modifying tests to respect the new convention.

@willtebbutt
Copy link
Member Author

Drop in coverage appears to be to do with style-related changes inflating the line count slightly.

@willtebbutt willtebbutt mentioned this pull request Aug 30, 2020
2 tasks
src/basekernels/exponential.jl Show resolved Hide resolved
src/basekernels/exponential.jl Show resolved Hide resolved
src/matrix/kernelmatrix.jl Show resolved Hide resolved
src/matrix/kernelpdmat.jl Outdated Show resolved Hide resolved
src/matrix/kernelpdmat.jl Show resolved Hide resolved
@willtebbutt
Copy link
Member Author

@theogf @sharanry the test failures appear to be unrelated to this PR. Any idea what's going on?

@theogf
Copy link
Member

theogf commented Sep 21, 2020

Looks like Gabor fails randomly...

@willtebbutt
Copy link
Member Author

That's a bit sad. I'm reluctant to break the build, because that's really no fun for development. Have we got work ongoing to fix Gabor + AD?

@devmotion
Copy link
Member

It seems to be fine apart from the tests with Julia nightlies, doesn't it? I assume the test errors there are caused by some Zygote and compiler-related issues that we do not have to worry about (at least I know that these Zygote/compiler problems cause test errors of Turing with Julia nightly quite consistently). Maybe best to just allow test failures on Julia nightly?

@willtebbutt
Copy link
Member Author

willtebbutt commented Sep 21, 2020

Oh, weird. It was failing on version 1 before. I guess someone re-ran the builds? Assuming that docs pass then this is good to go.

@theogf
Copy link
Member

theogf commented Sep 21, 2020

Yeah I reran the failing build

@willtebbutt willtebbutt merged commit 744419e into master Sep 21, 2020
@willtebbutt willtebbutt deleted the wct/fix-bug branch September 21, 2020 14:42
johannesgiersdorf added a commit to johannesgiersdorf/KernelFunctions.jl that referenced this pull request Oct 5, 2020
theogf pushed a commit that referenced this pull request Oct 5, 2020
* Update docstring corresponding to #158

* LGTM style fix

Co-authored-by: willtebbutt <wt0881@my.bristol.ac.uk>

* bump the patch version

* Update src/basekernels/exponential.jl

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

Co-authored-by: willtebbutt <wt0881@my.bristol.ac.uk>
Co-authored-by: David Widmann <devmotion@users.noreply.github.com>
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