[
https://issues.apache.org/jira/browse/MATH-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17911037#comment-17911037
]
Gilles Sadowski commented on MATH-1650:
---------------------------------------
Hello [~michikommader].
I finally got around to look at this again; sorry for the delay.
Rather than make a list of the "cosmetic" changes, I readily implemented them;
please compare the committed version with your original source files so that,
as coding style is concerned, we'll be on the same page for future
contributions.
Thanks for the unit tests!
I was going to merge the new feature into "master"; but then having a quick
look at
[SplineInterpolator|https://commons.apache.org/proper/commons-math/javadocs/api-4.0-beta1/src-html/org/apache/commons/math4/legacy/analysis/interpolation/SplineInterpolator.html]
I notice that there is quite some overlap, up to the same code comments, and
even the reference book (although a newer edition).
If indeed the case, such copy/paste increases maintenance burdens (the more so
that some micro-optimizations seem to have been performed already in
"SplineInterpolator.java").
Eventually re-reading the history of this report, I seem to recall that the
issue was _not_ with the new class inheriting from {{SplineInterpolator}} but
with the {{interpolate}} method not being overridden (allowing incorrect
usage). That would have been fixed in your latest patch since you now define 2
methods (one that indeed overrides the interface's method, and one that
implements the "clamp" feature).
I thus created a branch (named
["feature__MATH-1650"|https://github.com/apache/commons-math/tree/feature__MATH-1650]),
so that we can converge on consolidating the contents of
"SplineInterpolator.java" and "ClampedSplineInterpolator.java" before it goes
to "master".
As noted in my previous comment, the better approach (IMO) would be to create a
new (maven) module (to contain all the interpolators that do not depend on
other "legacy" code. However we can tackle that afterwards; i.e. I'm OK to
introduce "ClampedSplineInterpolator" (but without copy/paste!) in the "legacy"
part of the repository...
> Add clamped spline interpolation
> --------------------------------
>
> Key: MATH-1650
> URL: https://issues.apache.org/jira/browse/MATH-1650
> Project: Commons Math
> Issue Type: New Feature
> Affects Versions: 4.X
> Reporter: Michael Scholz
> Priority: Minor
> Labels: Polynomials, interpolation, spline
> Attachments: 2022-10-05_ClampedSplineInterpolator.patch,
> 2025-01-02_ClampedSplineInterpolator.patch
>
>
> We would like to contribute a new _clamped_ spline interpolation function in
> addition to the already available unclamped spline function. Our new
> {{ClampedSplineInterpolator}} is based on the same textbook as the original
> {{{}SplineInterpolator{}}}. The clamped spline offers additional
> parameterisation of starting and ending slopes (1st derivatives) as boundary
> conditions in order to provide more flexibility in spline creation.
> In this patch we follow the approach of subclassing the original
> {{SplineInterpolator}} and simply overloading it's {{interpolate()}} function
> by these two additional parameters. Is this an acceptable way or does the
> community recommend a different design approach?
> After clarifying the basic implementation approach we could also supply
> necessary tests etc. and finally contribute everything via ordinary GitHub
> pull request.
> Refer to our post on the dev mailing list:
> https://lists.apache.org/thread/34qnx4tgjbyv345lgmd57g0bnlnwdzc8
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
