Skip to content

Add KernelInterpolation.jl extension#73

Merged
andrewwinters5000 merged 29 commits intomainfrom
kernelinterpolation
Sep 29, 2025
Merged

Add KernelInterpolation.jl extension#73
andrewwinters5000 merged 29 commits intomainfrom
kernelinterpolation

Conversation

@JoshuaLampert
Copy link
Member

This PR creates a thin wrapper around my package KernelInterpolation.jl as a package extension, such that TrixiBottomTopgraphy.jl can handle arbitrary scattered data, see #45.

Some things to note:

  • I renamed the Makie extension from TrixiBottomTopographyMakieExt to MakieExt following the latest recommendations (https://pkgdocs.julialang.org/v1/creating-packages/#Code-structure), see also Shorten package extension names as per the Pkg docs Trixi.jl#2543.
  • I allowed the same interface as for the B spline interpolations by adding methods, which take either values x, y, (z) or a path to a file containing this data for easy comparison. Additionally, I implemented a general method taking any set of scattered nodes.
  • To verify the implementation, I used the same 1D and 2D datasets of the Rhine river. However, it might make sense to also add an example using truly scattered data to showcase this feature. Notably, at least in 1D, the linear B spline gives the same result up to floating point arithmetic as the RBF interpolation with PolyharmonicSplineKernel{1}(1) and the cubic B spline with free end conditions gives (up to ~1e-12) the same as PolyharmonicSplineKernel{1}(3).
  • I didn't yet add docs apart from docstrings. Should I add a section to "Overview"?

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.48%. Comparing base (4f54b78) to head (9a6fb8f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
- Coverage   96.53%   96.48%   -0.06%     
==========================================
  Files           9        9              
  Lines         491      484       -7     
==========================================
- Hits          474      467       -7     
  Misses         17       17              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coveralls
Copy link

coveralls commented Sep 26, 2025

Pull Request Test Coverage Report for Build 18091922947

Details

  • 28 of 28 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 96.488%

Totals Coverage Status
Change from base Build 18063559082: -0.05%
Covered Lines: 467
Relevant Lines: 484

💛 - Coveralls

@JoshuaLampert
Copy link
Member Author

Failing docs should be fixed by #70.

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this capability for scattered data interpolation! It would be good if we had some scattered data in the example, but for the time being, I left a suggestion for a "manufactured" way to engineer a set of 1D scattered points.

As far as the docs go, it would be good to add a few sentences in the Overview to point to this functionality, and we can add more details or a tutorial later once we have a "real" test case.

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, although we could update some of the function calls in light of #75 . We should merge that PR first.

@andrewwinters5000
Copy link
Member

Can you add a bullet point and update the Functionalities in the README to point out that we can also do scattered interpolation.

@JoshuaLampert
Copy link
Member Author

Can you add a bullet point and update the Functionalities in the README to point out that we can also do scattered interpolation.

Done in 10a17b0.

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@andrewwinters5000 andrewwinters5000 linked an issue Sep 27, 2025 that may be closed by this pull request
@JoshuaLampert
Copy link
Member Author

Ok, finally I fixed the docs such that also the docstrings of the extensions are included in the references. This is ready from my perspective.

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The only thing that is missing is you should add your name to the contributors list in AUTHORS.md

@JoshuaLampert
Copy link
Member Author

LGTM! The only thing that is missing is you should add your name to the contributors list in AUTHORS.md

Done in 9a6fb8f.

@andrewwinters5000 andrewwinters5000 merged commit 65319da into main Sep 29, 2025
8 checks passed
@andrewwinters5000 andrewwinters5000 deleted the kernelinterpolation branch September 29, 2025 18:33
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.

Expand to more general data sets

4 participants