commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Luc Maisonobe <Luc.Maison...@free.fr>
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
Date Mon, 20 Apr 2009 18:37:50 GMT
Bill Barker a écrit :
> 
> ----- Original Message ----- From: <luc@apache.org>
> To: <commits@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.

> 
>>     /** {@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.

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.

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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 
> 


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


Mime
View raw message