[math] UnivariateImpl suggestions

[Brent Worden <br...@worden.org>]

I was looking at the source code for UnivariateImpl and noticed a couple of
comments in the insertValue method that I would like to address.

First,
// Include the influence of the new
// TODO: The next two lines seems rather expensive, but
// I don't see many alternatives.
min = doubleArray.getMin();
max = doubleArray.getMax();

One alternative is to only recompute the min(max) when the discarded values
is equal to the min(max).  The reasoning is, if the extremes are not equal
to the discarded value, the extremes must still be in the valid values.
Hence, they do not change.  So, a couple if checks could save some
computation time.  The new code could look something like:

if(discarded == min){
    min = doubleArray.getMin();
}
if(discarded == max){
    max = doubleArray.getMax();
}

Next,
// Note that the product CANNOT be discarded
// properly because one cannot discount the effect
// of a zero value.  For this reason, the product
// of the altered array must be calculated from the
// current array elements.  Product must be recalculated
// everytime the array is "rolled"

I think this reasoning can be refined to produce similar computational
gains.  Please verify the following logic:
1) If the current product is non-zero then, the discarded value is non-zero
and can be safely be used as a divisor.
2) If the current product is zero then, there exists at least one zero value
in the list.
3) If the discarded value is non-zero then, a zero product is unaffected by
the removal because "there exists at least one zero value in the list."
4) If the discarded value is zero then, the last zero value might be removed
resulting in the product needing to be recalculated.

The above reasoning corresponds to the following code:

if(product != 0.0){
    // can safely remove discarded value
    product /= discarded;
} else if(discard == 0.0){
    // need to recompute product
    product = 1.0;
    double[] elements = doubleArray.getElements();
    for( int i = 0; i < elements.length; i++ ) {
        product *= elements[i];
    }
}

Just a couple of thoughts.  Feel free to comment and criticize at your
leisure.

Brent Worden
http://www.brent.worden.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org

Re: [math] UnivariateImpl suggestions

[Phil Steitz <ph...@steitz.com>]

Brent Worden wrote:
> I was looking at the source code for UnivariateImpl and noticed a couple of
> comments in the insertValue method that I would like to address.
> 
> First,
> // Include the influence of the new
> // TODO: The next two lines seems rather expensive, but
> // I don't see many alternatives.
> min = doubleArray.getMin();
> max = doubleArray.getMax();
> 
> One alternative is to only recompute the min(max) when the discarded values
> is equal to the min(max).  The reasoning is, if the extremes are not equal
> to the discarded value, the extremes must still be in the valid values.
> Hence, they do not change.  So, a couple if checks could save some
> computation time.  The new code could look something like:
> 
> if(discarded == min){
>     min = doubleArray.getMin();
> }
> if(discarded == max){
>     max = doubleArray.getMax();
> }

I agree.  This is better. Thanks.

> 
> Next,
> // Note that the product CANNOT be discarded
> // properly because one cannot discount the effect
> // of a zero value.  For this reason, the product
> // of the altered array must be calculated from the
> // current array elements.  Product must be recalculated
> // everytime the array is "rolled"
> 
> I think this reasoning can be refined to produce similar computational
> gains.  Please verify the following logic:
> 1) If the current product is non-zero then, the discarded value is non-zero
> and can be safely be used as a divisor.
> 2) If the current product is zero then, there exists at least one zero value
> in the list.
> 3) If the discarded value is non-zero then, a zero product is unaffected by
> the removal because "there exists at least one zero value in the list."
> 4) If the discarded value is zero then, the last zero value might be removed
> resulting in the product needing to be recalculated.
> 
> The above reasoning corresponds to the following code:
> 
> if(product != 0.0){
>     // can safely remove discarded value
>     product /= discarded;
> } else if(discard == 0.0){
>     // need to recompute product
>     product = 1.0;
>     double[] elements = doubleArray.getElements();
>     for( int i = 0; i < elements.length; i++ ) {
>         product *= elements[i];
>     }
> }
> 

I agree with you here too.

Thanks for the suggestions.

Phil

> Just a couple of thoughts.  Feel free to comment and criticize at your
> leisure.
> 
> Brent Worden
> http://www.brent.worden.org
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 




---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org