Repository navigation
1d/2d cubic spline interpolator - #187
Conversation
|
@Sichao25 I meged the |
| } | ||
|
|
||
| template <typename T, typename MemorySpace> | ||
| void isUniformAscending(Rank1View<T, MemorySpace> x, const T &ztol) { |
There was a problem hiding this comment.
Can we use is_sorted instead? https://kokkos.org/kokkos-core-wiki/API/algorithms/std-algorithms/all/StdIsSorted.html
There was a problem hiding this comment.
Just following up on this issue. mdspan doesn’t provide iterators, right? If so, would it be better to stick with a hand-coded loop here?.
|
@Sichao25 We should be ready to make this into a regular pull request (currently it is a draft) |
|
Thanks for the notification. Just did that. |
jacobmerson
left a comment
jacobmerson
left a comment
There was a problem hiding this comment.
This is looking good. One last thing I have for @Sichao25.
The test case is is comprehensive which is great. I think it would be helpful to have a simple example with some documentation of performing a single 2D interpolation.
| const T half = 0.5; | ||
| const T sixth = 0.166666666666666667; |
There was a problem hiding this comment.
I think these should be constexpr T half = 1./2 and constexpr T sixth = 1./6 like you have elsewhere.
| Rank1View<T, MemorySpace> x, | ||
| LO nx, LO &ier) { | ||
| if (xget < x[0] || xget > x[nx - 1]) { | ||
| T zxtol = 4.0e-7 * std::max(std::abs(x[0]), std::abs(x[nx - 1])); |
There was a problem hiding this comment.
Should the tolerance be an argument to the function? It's not clear if this should be hardcoded.
There was a problem hiding this comment.
If this is meant to be an argument, should it be user-configurable? That would mean adding it to the evaluate API with a default value.
There was a problem hiding this comment.
Generally it's better to provide the user the ability to mess with tolerances. Comparing floating point numbers is surprisingly frought, so there isn't really a universal tolerance that works for everything.
c6cc582 to
780ced4
Compare
|
@Sichao25 Do you know how to re-run the pipelines so that the clang-tidy checks are run? Once we run that I am ready to merge this. Thanks for your work on this! |
|
I might need to merge the develop branch here. |
|
There aren't be any conflicts, so hopefully that will be relatively painless. |
Implemented 1D and 2D spline interpolators with corresponding test cases. The interpolators use
mdspanas input to give users full control over memory allocation strategies, allowing compatibility with various data layouts.While there is significant room for improvement in areas such as performance optimizations and implementation enhancements (e.g., more flexible constructors, integration with
UniformGridclass, increased parallelism, C++ details, documentation ... ), this represents an initial version that's ready for preliminary feedback to identify any fundamental concerns.