Add support for tensor-valued spherical functions in `interp_rbf`
Fixes #3236.
This PR adds support for tensor-valued spherical functions in interp_rbf. When the user calls this function, specifying legacy=True internally uses scipy.interpolate.Rbf which is the original behavior; specifying legacy=False internally uses scipy.interpolate.RBFInterpolator, which allows data to have arbitrary dimensions.
It would be great if I could get some feedbacks on the following questions:
- I saw the kwarg
normis already pending deprecation and is not used inscipy.interpolate.RBFInterpolator. As of my initial commit,normis ignored whenlegacy=False. Do we still want to keepnorm? - In the docstring,
functionis said to be one of{'multiquadric', 'inverse', 'gaussian'}, but bothscipy.interpolate.Rbfandscipy.interpolate.RBFInterpolatorsupport a little bit more than that (see here and here). Should I change the docstring? - In
Rbf, whenepsilon=None, it internally estimate the average distance between nodes. However, inRBFInterpolator, this behavior is gone. The user has to specifyepsilonwhenkernelis not one of{'linear', 'thin_plate_spline', 'cubic', or 'quintic'}. Should I change the docstring to reflect this new behavior?
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 83.67%. Comparing base (
19ae6a6) to head (92bed94). Report is 7 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #3237 +/- ##
=======================================
Coverage 83.67% 83.67%
=======================================
Files 153 152 -1
Lines 21381 21373 -8
Branches 3458 3458
=======================================
- Hits 17890 17884 -6
+ Misses 2629 2627 -2
Partials 862 862
I am not sure how to fix the two failing checks.
Hi @kvttt,
Thank you for working on this!
I would recommend to create a new function named rbf_interpolation and deprecate the interp_rbf by adding our decorator (see an example here)
I do not see any benefit to keep all this different behaviors (if legacy or not )and add complexity to the maintenance. 1 function = 1 feature/task if possible.
Do we still want to keep norm?
no
Should I change the docstring?
Yes
Should I change the docstring to reflect this new behavior?
No, since we plan to remove this version and keep only your new version
Can you update this? Thank you !
I am not sure how to fix the two failing checks.
Concerning the checks, no worries concerning this 2 checks
Thank you for the quick update @kvttt!
is there any additional comment ? @arokem ? @Garyfallidis ? someone else ?