There are some things I don't like about this, but will try to find the time
to fix them myself . Comments inline (and I can't veto anything in [math],
so these are just suggestions).
----- Original Message -----
From: <l...@apache.org>
To: <comm...@commons.apache.org>
Sent: Wednesday, December 09, 2009 2:56 PM
Subject: svn commit: r889008 - in /commons/proper/math/trunk/src:
main/java/org/apache/commons/math/linear/ site/xdoc/
test/java/org/apache/commons/math/linear/
Author: luc
Date: Wed Dec 9 22:56:11 2009
New Revision: 889008
URL: http://svn.apache.org/viewvc?rev=889008&view=rev
Log:
Added mapping and iteration methods to vectors.
Provided a default implementation for the numerous simple methods in the
RealVectorInterface.
+ protected class SparseEntryIterator implements Iterator<Entry> {
+
+ /** Dimension of the vector. */
+ private final int dim;
+
+ /** Temporary entry (reused on each call to {...@link #next()}. */
+ private EntryImpl tmp = new EntryImpl();
+
+ /** Current entry. */
+ private EntryImpl current;
+
+ /** Next entry. */
+ private EntryImpl next;
+
+ /** Simple constructor. */
+ protected SparseEntryIterator() {
+ dim = getDimension();
+ current = new EntryImpl();
+ if (current.getValue() == 0) {
+ advance(current);
+ }
This totally doesn't work if the vector consists of all zero elements. The
'hasNext' method (below) will return true, and you will get an exeption
trying to get the first element.
+ next = new EntryImpl();
+ next.setIndex(current.getIndex());
+ advance(next);
+ }
+
+ /** Advance an entry up to the next non null one.
+ * @param e entry to advance
+ */
+ protected void advance(EntryImpl e) {
+ if (e == null) {
+ return;
+ }
+ do {
+ e.setIndex(e.getIndex() + 1);
+ } while (e.getIndex() < dim && e.getValue() == 0);
+ if (e.getIndex() >= dim) {
+ e.setIndex(-1);
+ }
+ }
This is fragile, since it relies on e.getIndex() == -1 when invalid (and
then you scan the vector once more).
+
+ /** {...@inheritdoc} */
+ public boolean hasNext() {
+ return current != null;
+ }
+
quick fix is to check that current.getIndex() >= 0
+ /** {...@inheritdoc} */
+ public Entry next() {
+ tmp.setIndex(current.getIndex());
+ if (next != null) {
+ current.setIndex(next.getIndex());
+ advance(next);
+ if (next.getIndex() < 0) {
+ next = null;
+ }
+ } else {
+ current = null;
+ }
+ return tmp;
+ }
+
+ /** {...@inheritdoc} */
+ public void remove() {
+ throw new UnsupportedOperationException("Not supported");
+ }
+ }
+
+}
Modified:
commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/OpenMapRealVector.java
URL:
http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/OpenMapRealVector.java?rev=889008&r1=889007&r2=889008&view=diff
==============================================================================
---
commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/OpenMapRealVector.java
(original)
+++
commons/proper/math/trunk/src/main/java/org/apache/commons/math/linear/OpenMapRealVector.java
Wed Dec 9 22:56:11 2009
@@ -27,7 +27,7 @@
* @version $Revision$ $Date$
* @since 2.0
*/
-public class OpenMapRealVector implements SparseRealVector, Serializable
{
+public class OpenMapRealVector extends AbstractRealVector implements
SparseRealVector, Serializable {
/** Default Tolerance for having a value considered zero. */
public static final double DEFAULT_ZERO_TOLERANCE = 1.0e-12;
@@ -41,8 +41,11 @@
/** Dimension of the vector. */
private final int virtualSize;
- /** Tolerance for having a value considered zero. */
- private double epsilon;
+ /** Negative tolerance for having a value considered zero. */
+ private double minusEpsilon;
+
+ /** Positive tolerance for having a value considered zero. */
+ private double plusEpsilon;
If non-zero defaultValues are allowed, then we need to store the
defaultValue, to avoid floating point precision issues where (plusEpsilon +
minusEpsilon)/2 != defaultValue.
/**
* Build a 0-length vector.
@@ -54,7 +57,7 @@
* into this vector.</p>
*/
public OpenMapRealVector() {
- this(0, DEFAULT_ZERO_TOLERANCE);
+ this(0, DEFAULT_ZERO_TOLERANCE, 0);
}
/**
@@ -62,18 +65,19 @@
* @param dimension size of the vector
*/
public OpenMapRealVector(int dimension) {
- this(dimension, DEFAULT_ZERO_TOLERANCE);
+ this(dimension, DEFAULT_ZERO_TOLERANCE, 0);
}
/**
* Construct a (dimension)-length vector of zeros, specifying zero
tolerance.
* @param dimension Size of the vector
* @param epsilon The tolerance for having a value considered zero
+ * @param defaultValue value for non-specified entries
*/
- public OpenMapRealVector(int dimension, double epsilon) {
+ public OpenMapRealVector(int dimension, double epsilon, double
defaultValue) {
virtualSize = dimension;
- entries = new OpenIntToDoubleHashMap(0.0);
- this.epsilon = epsilon;
+ entries = new OpenIntToDoubleHashMap(defaultValue);
+ setDefault(defaultValue, epsilon);
}
As I stated in the Jira issue, I really dislike supporting non-zero
defaultValues here. It causes too many things to just produce nonsensical
results:
RealVector v = new OpenMapRealVector(1000, 1.0e-12, 1);
assertEquals(v.getEntry(0), 0); // passes
RealVector v = new OpenMapRealVector(1000, 1.0e-12, 1);
RealVector w = new OpenMapRealVector(1000, 1.0e-12, 2);
assertEquals(v.add(w).getEntry(0), 0); // passes
/** {...@inheritdoc} */
- public OpenMapRealVector add(RealVector v) throws
IllegalArgumentException {
+ public RealVector add(RealVector v) throws IllegalArgumentException {
checkVectorDimensions(v.getDimension());
if (v instanceof OpenMapRealVector) {
return add((OpenMapRealVector) v);
+ } else {
+ return super.add(v);
}
- return add(v.getData());
}
/**
- * Optimized method to add two OpenMapRealVectors.
+ * Optimized method to add two OpenMapRealVectors. Copies the larger
vector, iterates over the smaller.
* @param v Vector to add with
* @return The sum of <code>this</code> with <code>v</code>
* @throws IllegalArgumentException If the dimensions don't match
*/
public OpenMapRealVector add(OpenMapRealVector v) throws
IllegalArgumentException{
checkVectorDimensions(v.getDimension());
- OpenMapRealVector res = copy();
- Iterator iter = v.getEntries().iterator();
+ boolean copyThis = entries.size() > v.entries.size();
+ OpenMapRealVector res = copyThis ? this.copy() : v.copy();
+ Iterator iter = copyThis ? v.entries.iterator() :
entries.iterator();
+ OpenIntToDoubleHashMap randomAccess = copyThis ? entries :
v.entries;
while (iter.hasNext()) {
iter.advance();
int key = iter.key();
- if (entries.containsKey(key)) {
- res.setEntry(key, entries.get(key) + iter.value());
+ if (randomAccess.containsKey(key)) {
+ res.setEntry(key, randomAccess.get(key) + iter.value());
} else {
res.setEntry(key, iter.value());
}
This only works (in a wonderland sense of working) when both sides have the
same defaultValue.
@@ -252,16 +259,6 @@
return res;
}
- /** {...@inheritdoc} */
- public double dotProduct(RealVector v) throws
IllegalArgumentException {
- checkVectorDimensions(v.getDimension());
- double res = 0;
- Iterator iter = entries.iterator();
- while (iter.hasNext()) {
- iter.advance();
- res += v.getEntry(iter.key()) * iter.value();
- }
- return res;
- }
-
For this class to be useful, it needs it's own sparseIterator() method to
cover delegating this method to the base class.
@@ -1185,7 +1155,7 @@
/** {...@inheritdoc} */
public void unitize() {
double norm = getNorm();
- if (isZero(norm)) {
+ if (isDefaultValue(norm)) {
throw MathRuntimeException.createArithmeticException("cannot
normalize a zero norm vector");
}
Iterator iter = entries.iterator();
@@ -1196,37 +1166,6 @@
}
This one is just silly. This results in:
RealVector v = new OpenMapRealVector(1000, 1.0e-12, 1);
v.setEntry(0,1);
v.unitize();
throwing an execption.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
