Repository navigation
Spline fixes - #397
Open
jacobmerson wants to merge 4 commits into
Open
Spline fixes#397jacobmerson wants to merge 4 commits into
jacobmerson wants to merge 4 commits into
Conversation
Several bugs made the spline evaluators wrong or silent on CUDA: - Device builds default Rank2View to layout_left, but the evaluate kernels addressed rows of fval with row-major. - Missing barriers with TeamThreadRange. - Swap std::array to Kokkos::Array and std::min/std::max to kokkos version so code works without --expt-relaxed-constexpr - The 1D coefficient init and the 2D MDRange launches used the default execution space. Interpolator in HostSpace launched device kernels on host pointers.
Check that 1D and 2D selectors return the requested derivatives in PSPLINE column order without touching unused columns, that the explicit and compact representations agree for every boundary condition, and that bicubic splines reproduce a product of cubics for every end condition such a product satisfies. Port the checks of PSPLINE's pspltest that the spline tests lacked: - splines on 3 to 5 points sampled from a two-point spline, with the same derivative ends, are that spline (smalltest) - 2D splines periodic in one direction only are exact along the periodic nodes and wrap only the periodic axis (bpspline, bpsplinb) - 1D value, first and second derivative errors converge at orders 4, 3 and 2, and 2D values at order 4 (pspltest1, pspltest2) - zero samples follow non-zero end and face data, and periodic splines of constant data are constant (smalltest) - queries within the lookup tolerance outside a non-periodic grid are clamped to the end; queries beyond it are left untouched Note: most of the test porting in this commit was done by Claude.
jacobmerson
force-pushed
the
spline-fixes
branch
from
October 25, 2026 05:15
4b9a039 to
d3225e2
Compare
jacobmerson
requested review from
Sichao25
and
a balanced review from Copilot
October 26, 2026 13:42
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The tightened uniformity tolerance rejects valid translated grids because it does not account for coordinate-scale roundoff.
1 open issue
What changed in this PR
This PR improves PCMS cubic and bicubic spline correctness and host/device portability, with extensive regression coverage.
Changes:
- Fix boundary-condition handling, workspace indexing, synchronization, and output layout handling.
- Tighten grid validation and add periodic query wrapping and safer range checks.
- Expand spline tests and include the spline header in the field installation list.
| File | Description |
|---|---|
test/test_spline_interpolator.cpp |
Updates view handling and adds numerical, boundary-condition, and portability tests. |
test/field_test_utils.h |
Adds a helper for creating device rank-one views from vectors. |
src/pcms/field/evaluator/spline_interpolator.hpp |
Fixes spline construction, evaluation, synchronization, and validation. |
src/pcms/field/CMakeLists.txt |
Adds the spline interpolator header to the installation list. |
Review effort: Balanced
Comment on lines
+72
to
+73
| bool is_uniform_ascending = isUniformAscending<T, MemorySpace>( | ||
| x, std::sqrt(std::numeric_limits<T>::epsilon())); |
abhiyanpaudel
pushed a commit
that referenced
this pull request
Oct 29, 2026
This branch was successfully deployed
This file contains hidden or 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
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.
No description provided.