You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Bill Barker <bi...@verizon.net> on 2009/04/19 02:55:40 UTC
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
----- Original Message -----
From: <lu...@apache.org>
To: <co...@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?
> /** {@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.
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
[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
Posted by Bill Barker <bi...@verizon.net>.
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" <Lu...@free.fr>
To: "Commons Developers List" <de...@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: <lu...@apache.org>
>> To: <co...@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
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
Posted by Luc Maisonobe <Lu...@free.fr>.
Bill Barker a écrit :
>
> ----- Original Message ----- From: <lu...@apache.org>
> To: <co...@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