[jira] [Commented] (MATH-1650) Add clamped spline interpolation

2022-11-03 Thread Gilles Sadowski (Jira)


[ 
https://issues.apache.org/jira/browse/MATH-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17628356#comment-17628356
 ] 

Gilles Sadowski commented on MATH-1650:
---

Sorry, in my previous comment, I hadn't recalled the interface issue which we 
had mentioned on the ML (IIRC).

A programming error in the following code
{code}
double[] xval = ...
double[] yval = ...
UnivariateFunction func = new ClampedSplineInterpolator().interpolate(xval, 
yval);
{code}
goes undetected because the new {{interpolate}} method in 
{{ClampedSplineInterpolator}} _overloads_ the method defined in the 
{{UnivariateInterpolator}} interface.

My first impression is that the functionality (i.e. whether derivatives are 
required) should be exposed at the interface level.


> 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
>
>
> 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)


[jira] [Commented] (MATH-1650) Add clamped spline interpolation

2022-11-03 Thread Michael Scholz (Jira)


[ 
https://issues.apache.org/jira/browse/MATH-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17628306#comment-17628306
 ] 

Michael Scholz commented on MATH-1650:
--

Thanks Gilles for your feedback! My initial thought was to sketch this new 
class in order to get a general first feedback if this approach is viable from 
the design perspective (subclass with overloaded {{interpolate()}} function). 
If it complies with your expectations, I can start working on tests :)

> 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
>
>
> 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)


[jira] [Commented] (MATH-1650) Add clamped spline interpolation

2022-11-01 Thread Gilles Sadowski (Jira)


[ 
https://issues.apache.org/jira/browse/MATH-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17627196#comment-17627196
 ] 

Gilles Sadowski commented on MATH-1650:
---

Thanks for the patch, and sorry for the delay in starting the review.

It's great that it compiles fine, but... there are no unit tests for this new 
class!

Trivial (e.g. precondition) tests are necessary to ensure early failure with an 
appropriate diagnostics.
Functionality tests are required to ensure expected behaviour and prevent 
inadvertent code changes.
At least, the new {{interpolate}} method should have 100% coverage and several 
use-cases (functions).

I guess that you could use {{SplineInterpolatorTest}} as a starting point.


> 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
>
>
> 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)