-
Notifications
You must be signed in to change notification settings - Fork 79
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
[Draft] add single-argument kernel evaluator #541
[Draft] add single-argument kernel evaluator #541
Conversation
Excellent! - I look forward to trying this out. Glad to know the switch
works.
I would like to set it up so kerevalmeth=0,1, switches *both* spreadinterp
and finufft:onedim_*_kernel
together, so they always use the same kernel, be it ES or Horner.
Opinions how to do that in cleanest way welcome..
Thanks, Alex
…On Wed, Aug 28, 2024 at 7:21 AM Libin Lu ***@***.***> wrote:
To address issue #528
<#528>, added a
single-argument kernel evaluator using current polynomial coefficients.
@ahbarnett <https://github.com/ahbarnett> In case you want to test the
coefficients sooner.
I tested by replacing evaluate_kernel((FLT)z[n], opts); with evaluate_kernel_horner((FLT)z[n],
opts); in functions onedim_fseries_kernel and onedim_nuft_kernel, make
test still passes.
This may be merged after 2.3 release.
------------------------------
You can view, comment on, or merge this pull request online at:
#541
Commit Summary
- aa2be60
<aa2be60>
add single-argument kernel evaluator
File Changes
(2 files <https://github.com/flatironinstitute/finufft/pull/541/files>)
- *M* include/finufft/spreadinterp.h
<https://github.com/flatironinstitute/finufft/pull/541/files#diff-4a0efeca16f9348de45eace7da9efbdddfa20170009474275fb861d4780bb0cb>
(2)
- *M* src/spreadinterp.cpp
<https://github.com/flatironinstitute/finufft/pull/541/files#diff-183554bb1a791ce2cfe2a1149dffebabd6bddb30b97ec42e4da0fe823fa44802>
(76)
Patch Links:
- https://github.com/flatironinstitute/finufft/pull/541.patch
- https://github.com/flatironinstitute/finufft/pull/541.diff
—
Reply to this email directly, view it on GitHub
<#541>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACNZRSXS4PW4QT6TPIA2QK3ZTWXD7AVCNFSM6AAAAABNIB4UP2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ4TCOBQGU4DKNA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
*-------------------------------------------------------------------~^`^~._.~'
|\ Alex Barnett Center for Computational Mathematics, Flatiron Institute
| \ http://users.flatironinstitute.org/~ahb 646-876-5942
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe throwing an exception is not the most evil thing as the user cannot reach throw since this is internal API.
Is this ready to merge? Can you write a test that checks that both code (even odd and this) give same result? |
Got it :) Libin can also help. Sorry, returning to your review after a big
interruption yesterday...
…On Tue, Oct 15, 2024 at 6:28 AM mreineck ***@***.***> wrote:
This PR will almist certainly conflict with #567
<#567>. If #567
<#567> is merged first
(which I think is the less complicated ordering), I'll help with fixing
those!
—
Reply to this email directly, view it on GitHub
<#541 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACNZRST3BMQPNF4AJ2EIWX3Z3TU43AVCNFSM6AAAAABNIB4UP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJTGUYDKNZYHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
*-------------------------------------------------------------------~^`^~._.~'
|\ Alex Barnett Center for Computational Mathematics, Flatiron Institute
| \ http://users.flatironinstitute.org/~ahb 646-876-5942
|
I think now is a good time to merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, very nice.
To address issue #528, added a single-argument kernel evaluator using current polynomial coefficients.
@ahbarnett In case you want to test the coefficients sooner.
I tested by replacing
evaluate_kernel((FLT)z[n], opts);
withevaluate_kernel_horner((FLT)z[n], opts);
in functionsonedim_fseries_kernel
andonedim_nuft_kernel
,make test
still passes.This may be merged after 2.3 release.