commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <billwbar...@verizon.net>
Subject [math] 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 Tue, 21 Apr 2009 03:29:28 GMT
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.Maisonobe@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: <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.
>

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.

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

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

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


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


Mime
View raw message