[jira] [Comment Edited] (MATH-1134) unsafe initialization in BicubicSplineInterpolatingFunction

["Gilles (JIRA)" <ji...@apache.org>]

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

Gilles edited comment on MATH-1134 at 7/2/14 9:05 PM:
------------------------------------------------------

bq. I don't like the NPE part.

This is similar to how other parts of the CM code would behave if preconditions are not satisfied. Here, the user requests an interpolating function whose base interface is a "BivariateFunction"; the derivative part is a "bonus" for those who comply with the precondition (which is that the flag must be set to true).

In fact, I chose "false" as the default because you advocated that a user who just wants to interpolate should not pay the price needed to use the derivative functionality.
But the converse is safer: let then the default be "true"; a user that explicitly requests no initialization can only blame himself if he calls one of the derivative methods afterwards.

Using on-demand caching complicates the code and prevents making the field final.
Also, the derivatives were initially intended for internal purposes (to be used in "TricubicInterpolator").

The initial code (initialization at access time) was really based on (untested) efficiency considerations, and I would consider it premature optimization. As it is now the code is both safe (if the user abides by the simple precondition) and efficient.



was (Author: erans):
bq. I don't like the NPE part.

This is similar to how other parts of the CM code would behave if preconditions are not satisfied. Here, the user requests an interpolating function whose base interface is a "BivariateFunction"; the derivative part is a "bonus" for those who comply with the precondition (which is that the flag must be set to true).

In fact, I chose "false" as the default because you advocated that a user who just wants to interpolate should not pay the price needed to use the derivative functionality.
But the converse is safer: let then the default be "true"; a user that explicitly requests no initialization can only blame himself if he calls one of the derivative methods afterwards.

Using on-demand caching complicates the code and prevents making the field final.
Also, the derivatives were initially intended for internal purposes (to be used in "TricubicInterpolator").

The initial code (initialization at access time) was really based on (untested) efficiency considerations, and I would consider it premature optimization. As it is now the code is both safe (if the user abodes by the simple precondition) and efficient.


> unsafe initialization in BicubicSplineInterpolatingFunction
> -----------------------------------------------------------
>
>                 Key: MATH-1134
>                 URL: https://issues.apache.org/jira/browse/MATH-1134
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 3.3
>            Reporter: Derek Scherger
>            Priority: Minor
>
> The lazy initialization of the internal array of partialDerivatives in BicubicSplineInterpolatingFunction is not thread safe. If multiple threads call any of the partialDerivative functions concurrently one thread may start the initialization and others will see the array is non-null and assume it is fully initialized. If the internal array of partial derivatives was initialized in the constructor this would not be a problem.
> i.e. the following check in partialDerivative(which, x, y)
>         if (partialDerivatives == null) {
>             computePartialDerivatives();
>         }
> will start the initialization. However in computePartialDerivatives()
>         partialDerivatives = new BivariateFunction[5][lastI][lastJ];
> makes it appear to other threads as the the initialization has completed when it may not have.



--
This message was sent by Atlassian JIRA
(v6.2#6252)