Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: local policy)
Date: Mon, 11 Jun 2012 18:04:48 +0200
From: Gilles Sadowski <gilles@harfang.homelinux.org>
To: dev@commons.apache.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
Message-ID: <20120611160448.GU10938@dusk.harfang.homelinux.org>
Mail-Followup-To: dev@commons.apache.org
References: <20120611055217.7DC0A2388847@eris.apache.org>
 <20120611115620.GT10938@dusk.harfang.homelinux.org>
 <CAGRH7HoEc1__fZ8F6eZDb-UqsgTKioZ8Tkq_bXNKEDRJ+=SrMQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAGRH7HoEc1__fZ8F6eZDb-UqsgTKioZ8Tkq_bXNKEDRJ+=SrMQ@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-DCC-scarlet.be-Metrics: hel 20001; Body=1 Fuz1=1 Fuz2=1
X-Virus-Scanned: clamav-milter 0.97.1-exp at hel
X-Virus-Status: Clean
X-Virus-Checked: Checked by ClamAV on apache.org

Hello S�bastien.

> >> + � � � �/*
> >> + � � � � * 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.
> >
> I'm not sure that all entries would be stored (except if setEntry does
> not distinguish between zero values and non-zero values).

The problem is when the common values are not the default one, like ...

> 
> > 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".

... in this example.

> >
> >> @@ -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...
> >
> If we cache isNaN and isInfinite, this is "only" one additional loop ;).
> I don't like this solution either, but the bugs I have identified are
> very real, except if we accept that exceptional cases are not handled
> correctly by sparse vectors. I'm personally adverse to this option.
> If you have a better idea, please feel free to change that. By the
> way, would you like me to revert this commit ?

No. But maybe we should already open a ticket signalling that a potential
problem has been identified.

> 
> I just would like to mention that sparse vectors allow a gain in
> memory. Speed is, in my view, a bonus, but you probably think the
> opposite...

No, that's fine; less so if you lose both. ;-)


Best regards,
Gilles

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