commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: svn commit: r1348721 - in /commons/proper/math/trunk/src: main/java/org/apache/commons/math3/linear/OpenMapRealVector.java test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
Date Mon, 11 Jun 2012 11:56:20 GMT
Hello.

> Date: Mon Jun 11 05:52:16 2012
> New Revision: 1348721
> 
> URL: http://svn.apache.org/viewvc?rev=1348721&view=rev
> Log:
> MATH-803:
>   - modified OpenMapRealVector.ebeMultiply() and ebeDivide() to handle
> special cases  0d * NaN, 0d * Infinity, 0d / 0d and 0d / NaN.
>   - added implementation of isNaN() and isInfinite() to
> SparseRealVectorTest.SparseRealVectorTestImpl in order to allow for testing
> of OpenMapRealVector.ebeMultiply() and ebeDivide() with mixed types.
> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
>     commons/proper/math/trunk/src/test/java/org/apache/commons/math3/linear/SparseRealVectorTest.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java?rev=1348721&r1=1348720&r2=1348721&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
(original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/linear/OpenMapRealVector.java
Mon Jun 11 05:52:16 2012
> @@ -341,10 +341,14 @@ public class OpenMapRealVector extends S
>      public OpenMapRealVector ebeDivide(RealVector v) {
>          checkVectorDimensions(v.getDimension());
>          OpenMapRealVector res = new OpenMapRealVector(this);
> -        Iterator iter = entries.iterator();
> -        while (iter.hasNext()) {
> -            iter.advance();
> -            res.setEntry(iter.key(), iter.value() / v.getEntry(iter.key()));
> +        /*
> +         * MATH-803: it is not sufficient to loop through non zero entries of
> +         * this only. Indeed, if this[i] = 0d and v[i] = 0d, then
> +         * this[i] / v[i] = NaN, and not 0d.
> +         */
> +        final int n = getDimension();
> +        for (int i = 0; i < n; i++) {
> +            res.setEntry(i, this.getEntry(i) / v.getEntry(i));
>          }
>          return res;
>      }

I think that this renders the class potentially useless, if we assume that
it is used when "large" vectors are needed.
Indeed, after this operation, all the entries will be stored; thus
cancelling the memory efficiency of the class, and probably making the
returned value slower than an "ArrayRealVector" instance for subsequent
operations.

For every method that a "RealVector" argument, there should be a specialized
implementation that take an "OpenMapRealVector".

Also, couldn't we solve some of these problems if the value of the default
entry was stored and mutable? E.g. if the default for "v" and "w" is zero,
then the default for "v / w" will be "NaN".

> @@ -359,6 +363,25 @@ public class OpenMapRealVector extends S
>              iter.advance();
>              res.setEntry(iter.key(), iter.value() * v.getEntry(iter.key()));
>          }
> +        /*
> +         * MATH-803: the above loop assumes that 0d * x  = 0d for any double x,
> +         * which allows to consider only the non-zero entries of this. However,
> +         * this fails if this[i] == 0d and (v[i] = NaN or v[i] = Infinity).
> +         *
> +         * These special cases are handled below.
> +         */
> +        if (v.isNaN() || v.isInfinite()) {
> +            final int n = getDimension();
> +            for (int i = 0; i < n; i++) {
> +                final double y = v.getEntry(i);
> +                if (Double.isNaN(y)) {
> +                    res.setEntry(i, Double.NaN);
> +                } else if (Double.isInfinite(y)) {
> +                    final double x = this.getEntry(i);
> +                    res.setEntry(i, x * y);
> +                }
> +            }
> +        }

This probably can only be a temporary solution: 3 additional loops over all
the elements...

> [...]

Best,
Gilles

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


Mime
View raw message