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

Gilles Sadowski commented on STATISTICS-77:
-------------------------------------------

{quote}
{code}
final Sum a = Sum.create(Type.SIMPLE); // Create an instance that uses 
SimpleSum implementation.
a.accept(1);
final Sum b = Sum.create(Type.KAHAN); // Create an instance that uses 
CompensatedSum implementation.
b.accept(2);
{code}
{quote}

If "extensions" are only going to be provided by [Statistics] (and the 
constructor being package-private prevents otherwise), then why not use the 
same pattern as in [RNG] (i.e. an "enum" of all supported algorithms)? Then, 
it's purely internal business to provide the best possible result for the two 
instances being combined.

This might entail creating a new instance, incurring unwanted side-effects (or 
unwanted lack thereof).

To avoid future complications, I'd favour the same approach as for {{Sum}} in 
[Numbers]: Computation is a black-box, trying to provide the best possible 
result without accuracy guarantee.
The second best option is to let the user choose the algorithm (cf. "enum") but 
any combination will be applied on the first instance, using its algorithm, 
even if the algorithm of the second is more accurate.
If we go this route, then the place is [Numbers], not [Statisitics];  I mean 
that [Statistics] would provide the choice, *only* if the summation algorithms 
are defined in [Numbers] (no copy, please, of these extremely sensitive 
manipulations of floating-point numbers!).  If we stand by the decision made in 
[Numbers] (to not provide the flexible choice of algorithm), there is no reason 
to do it here.
 
{quote}
{code}
// Disallowed. Throws IllegalArgumentException / UnsupportedOperationException. 
final double disallow = b.combine(a).getAsDouble(); 

// Allowed. Updates a.sum = a.sum + b.getAsDouble()
final double allow = a.combine(b).getAsDouble();
{code}
{quote}

The public API defines a single entity: "Sum".  Getting an exception from 
attempting to perform a sum of two "Sum" instances does not make sense at the 
user level.

> Implement Sum
> -------------
>
>                 Key: STATISTICS-77
>                 URL: https://issues.apache.org/jira/browse/STATISTICS-77
>             Project: Commons Statistics
>          Issue Type: Sub-task
>          Components: descriptive
>            Reporter: Anirudh Joshi
>            Priority: Minor
>              Labels: gsoc, gsoc2023
>
> This implementation uses {{commons-numbers}} 
> [Sum|https://github.com/apache/commons-numbers/blob/master/commons-numbers-core/src/main/java/org/apache/commons/numbers/core/Sum.java]
>  as the underlying implementation.
> This may be further expanded to include various other Sum implementations 
> like [Kahan Sum|https://en.wikipedia.org/wiki/Kahan_summation_algorithm], 
> {{Neumaier Sum}} (Two sum method) etc. with the option for the users to 
> choose an implementation. However, before beginning the implementation of 
> various other algorithms it may be worthwhile to discuss and investigate the 
> relative speed of the current Sum implementation compared to 
> {{DoubleStream.sum}} through JMH benchmarks.
> When we know the different speeds, it would be more clear if the user should 
> have a choice for this statistic, e.g.
>  * When using {{DoubleStream.sum}} is too slow and it would be nice to have a 
> simple sum variation.
>  * When the sum has a lot of cancellation and {{DoubleStream.sum}} suffers 
> from random walk around zero.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to