----- "Bill Barker" <billwbar...@verizon.net> a écrit : > Apologies for forgetting that I sent this from my personal email, that > I > hadn't subscribed from yet. I'm slowly moving my Apache subscriptions > here > from my corporate email, but since I was using gmane for this list, > hadn't > gotten there yet :(. > > Relevant answers inline. > > ----- Original Message ----- > From: "Luc Maisonobe" <luc.maison...@free.fr> > To: "Commons Developers List" <dev@commons.apache.org> > Sent: Monday, April 20, 2009 11:37 AM > Subject: Re: svn commit: r766337 - in /commons/proper/math/trunk/src: > > java/org/apache/commons/math/linear/SparseRealVector.java > test/org/apache/commons/math/linear/SparseRealVectorTest.java > > > > Bill Barker a écrit : > >> > >> ----- Original Message ----- From: <l...@apache.org> > >> To: <comm...@commons.apache.org> > >> Sent: Saturday, April 18, 2009 8:17 AM > >> Subject: svn commit: r766337 - in /commons/proper/math/trunk/src: > >> java/org/apache/commons/math/linear/SparseRealVector.java > >> test/org/apache/commons/math/linear/SparseRealVectorTest.java > >> > >> > >>> Author: luc > >>> Date: Sat Apr 18 15:17:12 2009 > >>> New Revision: 766337 > >>> > >>> URL: http://svn.apache.org/viewvc?rev=766337&view=rev > >>> Log: > >>> fixed an error in SparseRealVector.isInfinite, NaN was not > checked > >>> beforehand > >>> fixed an error in SparseRealVector.hashcode, code did not depend > on > >>> vector entries > >>> fixed tests accordingly > >>> > >>> Modified: > >>> > >>> > commons/proper/math/trunk/src/java/org/apache/commons/math/linear/SparseRealVector.java > >>> > >>> > >>> > commons/proper/math/trunk/src/test/org/apache/commons/math/linear/SparseRealVectorTest.java > >>> > >>> > >>> Modified: > >>> > commons/proper/math/trunk/src/java/org/apache/commons/math/linear/SparseRealVector.java > >>> > >>> URL: > >>> > http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/linear/SparseRealVector.java?rev=766337&r1=766336&r2=766337&view=diff > >>> > >>> > ============================================================================== > >>> > >>> --- > >>> > commons/proper/math/trunk/src/java/org/apache/commons/math/linear/SparseRealVector.java > >>> (original) > >>> +++ > >>> > commons/proper/math/trunk/src/java/org/apache/commons/math/linear/SparseRealVector.java > >>> Sat Apr 18 15:17:12 2009 > >>> @@ -593,14 +593,20 @@ > >>> > >>> /** {...@inheritdoc} */ > >>> public boolean isInfinite() { > >>> + boolean infiniteFound = false; > >>> + boolean nanFound = false; > >>> Iterator iter = entries.iterator(); > >>> while (iter.hasNext()) { > >>> iter.advance(); > >>> - if (Double.isInfinite(iter.value())) { > >>> - return true; > >>> + final double value = iter.value(); > >>> + if (Double.isNaN(value)) { > >>> + nanFound = true; > >>> + } > >>> + if (Double.isInfinite(value)) { > >>> + infiniteFound = true; > >>> } > >>> } > >>> - return false; > >>> + return infiniteFound && (!nanFound); > >>> } > >>> > >> > >> Why not just return 'true' when either is found instead of going > through > >> the rest of the map? > > > > We could return false as soon as NaN is found, but not infinite. > The > > contract from the interface is that as soon as one component is NaN, > the > > vector is NaN and not infinite (same behavior as Complex). This is > what > > the test did in the first place and why it failed when I commented > it out. > > > > Ok, I mis-read the contract. > > >> > >>> /** {...@inheritdoc} */ > >>> @@ -1228,6 +1234,12 @@ > >>> temp = Double.doubleToLongBits(epsilon); > >>> result = prime * result + (int) (temp ^ (temp >>> 32)); > >>> result = prime * result + virtualSize; > >>> + Iterator iter = entries.iterator(); > >>> + while (iter.hasNext()) { > >>> + iter.advance(); > >>> + temp = Double.doubleToLongBits(iter.value()); > >>> + result = prime * result + (int) (temp ^ (temp >>> > 32)); > >>> + } > >>> return result; > >>> } > >>> > >> > >> Mostly out of interest, do you have a use-case for having this as > a > >> key? In any case I have to -1 it since equals only considers > values > >> within epsilon (e.g. a.subtract(b) is a zero vector). So this > part > >> breaks the contract that a.hashcode() == b.hashcode() if > a.equals(b) == > >> true. > > > > Any object can be a key, mainly when one wants to store it in a > HashSet. > > This allows to quickly check when the object is already known or > when it > > is something new. > > > > I didn't notice the discrepancy with equals, it is a major problem. > I > > have to admit I don't really like equals using epsilon at all. As > far as > > I remember we don't use it in other classes. The previous version > of > > hashcode was consistent with equals but has the drawback of having > many > > vector sharing the same hash value. > > > > Yes, I always saw the drawback, but had assumed that there wasn't a > use-case > for hashcode, so thought it didn't matter much as long as it was > consistent > with equals. I'm OK with using an exact comparison for equals, as > long as > the JavaDocs have a big bold warning that a.equals(b) is not the same > as > a.subtract(b) is the zero vector (and can volunteer to do it). > Programs > that actually call equals are probably doing something like your use > case > anyway.
It would be great if you could provide an implementation of both equals and hascode that are consistent with each other, do use the vector components and don't use epsilon in the comparison (they probably should not use it at all). > > From my experience with SparseVectors in other languages, they tend to > get > un-sparse very fast if you don't include an epsilon check. The worst > that > I've seen is a simplex linear optimization implementation using > SparseVectors, where they go dense extremely quickly. With this explanation, I better understand the rationale for the epsilon. It makes sense. So I suggest epsilon is only used to prevent loss of sparcity but not comparison/hascode. > > > Since I am busy on other part of [math] (finishing the Field stuff > in > > linear algebra and using it for accurate coefficients > initializations in > > the ODE part for stiff systems), I'll change the hashcode > implementation > > back to its previous value and update the test. > > > > Most of the Jira issues targeting 2.0 are in stats, which isn't my > strongest > area. But if you need help in Fields (e.g. finite or p-adic > implementations), just let me know. The Field implementation by itself is really basic: one class for elements providing essentially add/subtract/multiply/divide and one class for the field itself providing access to the zero and one elements. The tedious part is almost duplicating the linear algebra classes and replacing constructors, declarations and operations (a + b -> a.add(b) ...). I've done it for numerous classes and interfaces (XxxxRealYyyy becoming XxxxFieldYyyy). The remaining ones are OpenIntToDoubleHashMap, SparseRealMatrix and SparseRealVector. If you are ready to do them, I'll happily let you do and go back to my pending ODE solvers tasks. The only tricks are to add a Field<T> attribute in the classes, to add a corresponding parameter in some (but not all) constructors and to use a static function to allocate arrays with the appropriate type. Take a look at AbstractFieldMatrix and compare it to AbstractRealMatrix to get the idea. If you don't want to do it, I can continue myself, this is not a problem for me. Luc > > > Luc > > > >> > >> This is why I put in a weak hashcode for SparseRealVector in the > first > >> place. > >> > >>> > >>> Modified: > >>> > commons/proper/math/trunk/src/test/org/apache/commons/math/linear/SparseRealVectorTest.java > >>> > >>> URL: > >>> > http://svn.apache.org/viewvc/commons/proper/math/trunk/src/test/org/apache/commons/math/linear/SparseRealVectorTest.java?rev=766337&r1=766336&r2=766337&view=diff > >>> > >>> > ============================================================================== > >>> > >>> --- > >>> > commons/proper/math/trunk/src/test/org/apache/commons/math/linear/SparseRealVectorTest.java > >>> (original) > >>> +++ > >>> > commons/proper/math/trunk/src/test/org/apache/commons/math/linear/SparseRealVectorTest.java > >>> Sat Apr 18 15:17:12 2009 > >>> @@ -1082,7 +1082,7 @@ > >>> > >>> assertFalse(v.isInfinite()); > >>> v.setEntry(0, Double.POSITIVE_INFINITY); > >>> - assertFalse(v.isInfinite()); // NaN is checked before > infinity > >>> + assertFalse(v.isInfinite()); // NaN has higher priority > than > >>> infinity > >>> v.setEntry(1, 1); > >>> assertTrue(v.isInfinite()); > >>> > >>> @@ -1091,7 +1091,7 @@ > >>> assertNotSame(v, new SparseRealVector(new double[] { 0, 1, > 2 + > >>> Math.ulp(2)})); > >>> assertNotSame(v, new SparseRealVector(new double[] { 0, 1, > 2, > >>> 3 })); > >>> > >>> - assertEquals(new SparseRealVector(new double[] { > Double.NaN, > >>> 1, 2 }).hashCode(), > >>> + assertTrue(new SparseRealVector(new double[] { > Double.NaN, 1, > >>> 2 }).hashCode() != > >>> new SparseRealVector(new double[] { 0, > >>> Double.NaN, 2 }).hashCode()); > >>> > >>> assertTrue(new SparseRealVector(new double[] { Double.NaN, > 1, > >>> 2 }).hashCode() != > >>> > >>> > >>> > >> > >> > >> > --------------------------------------------------------------------- > >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >> For additional commands, e-mail: dev-h...@commons.apache.org > >> > >> > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
