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