Add more kernels #244
Merged
Add more kernels #244
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Scienfitz
force-pushed
the
feature/more_kernels
branch
from
d3b4423 to
64ac7f9
Compare
AVHopp
reviewed
May 21, 2024
AdrianSosic
requested changes
May 21, 2024
There was a problem hiding this comment.
Hi @Scienfitz, thanks for the PR. Quite a few issues, but all of them small ones that can be fixed in a breeze
AdrianSosic
requested changes
May 21, 2024
Scienfitz
force-pushed
the
feature/more_kernels
branch
from
64ac7f9 to
bf56ba8
Compare
AdrianSosic
force-pushed
the
feature/more_kernels
branch
from
bf56ba8 to
883b352
Compare
Scienfitz
force-pushed
the
feature/more_kernels
branch
from
883b352 to
f028f5c
Compare
AdrianSosic
approved these changes
May 21, 2024
AVHopp
approved these changes
May 21, 2024
AdrianSosic
force-pushed
the
feature/more_kernels
branch
from
f028f5c to
c7435d5
Compare
AdrianSosic
force-pushed
the
feature/more_kernels
branch
from
f028f5c to
bc1bdd5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is adding more kernel choices. There are some notable points to consider below:
Bugfix arithmetic Kernels
These used to apply operators like
mul(*a_list)but since the operators are defined as 2-input only this failed for a 3-composite kernel test I added. It was fixed via rephrasing it toreduce(mul, a_list)Removed Prior Iteration Tests
These have become obsolete as the kernels are tested with many priors. I've also reduced the prior used with the scale kernel in iteration tests to one choice, otherwise the list of kernels just got too large.
RQKernel alpha
Even though this kernel has an attribute called
alpha(also visible in the equation here) it does not seem to accept priors for it. Hence, I have ignoredalpha, both regarding prior and initial value in our corresponding kernelCosineKernel
I was not able to find prior settings that work with this kernel in the iteration tests. The error I get seem purely computational (a la could not fit any reasonable model etc). I have thus not included it in this PR