[jira] [Commented] (MATH-1650) Add clamped spline interpolation
[ 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
[ 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
[ 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)