aherbert commented on PR #350: URL: https://github.com/apache/commons-math/pull/350#issuecomment-6694395325
Multiple calls to getPercentile will have significant performance overhead using a copy. StatUtils is not documented to be non-destructive to the array order. So an option is to document the current behaviour. If we keep this change to revert to Math3 behaviour the method should be documented to have performance overhead. However the DescriptiveStatistics is documented to return `getValues()` in order added. So this is breaking behaviour. The patch is valid but users should be aware of the implications of calling repeatedly (e.g. with different percentile) when the underlying data has not changed. This should be documented in the `getPercentile(double)` method. The previous Percentile implementation used a working array copy and maintained pivots to improve performance of the single-pivot quick select method. The current class could be modified to cache a working copy and a reference to input values and the [begin, length) to identify repeat calls. Feel free to examine this idea. This moves the copy operation from Quantile to be managed more efficiently by Percentile class. The simpler alternative is to document exactly what is happening on each call and provide the user with advice to get the values array once and call Quantile themselves with all desired percentiles in a single call. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
