commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <>
Subject Re: svn commit: r739504 - in /commons/proper/math/trunk/src: java/org/apache/commons/math/linear/ test/org/apache/commons/math/linear/
Date Sun, 01 Feb 2009 00:46:15 GMT
It's a straight copy/paste from RealVectorImpl.  I agree that public isn't 
best (but would probably go for protected instead of private so still usable 
by subclasses).  However, making it non-public breaks the junit tests (which 
are a copy of the RealVectorImpl junit test with a search/replace, and some 
non-important one currently commented out).  Would be +1 for changing 
checkVectorDimension(int) to protectected in both RealVectorImpl and 

"Rahul Akolkar" <> wrote in message
Minor comment, and arguably subjective -- I find the signature for
checkVectorDimensions(int) a little odd for a public method (I'd make
it private).


On Sat, Jan 31, 2009 at 10:13 AM,  <> wrote:
> Here are my comments.
> The two isZero methods seem inconsistent with each other. If I explicitely 
> set a component to epsilon/2 at index i, then isZero(i) would be false 
> (the element is set) but isZero(getEntry(i)) would be true.
> It would be nice to be able to set epsilon at construction. If for example 
> I want to build a vector from an array filtering out the small values, 
> currently I need to first build an empty vector, then set epsilon, then 
> fill the vector with my array.
> In the optimized version of add(SparseRealVector), I wonder if the call to 
> res.set which either calls entries.put or entries.remove could not 
> invalidate the iterator. After a quick look, it seems OK 
> (findInsertionIndex in OpenIntToDoubleHashMap should always return a 
> negative value and newMapping should be always reset to false), but I'm 
> not sure and this seems to really be implementation dependent.
> In the add, subtract and ebeXxx methods, the result vector is first built 
> as a copy of the instance and later all its elements are overwritten. The 
> initial copy could probably be avoided by simply iterating on the instance 
> by itself and setting the elements of an initially empty vector, built 
> with the advanced use constructor with both expected size and dimension.
> This is a nice work, thanks
> Luc
> ----- a écrit :
>> Author: billbarker
>> Date: Sat Jan 31 04:51:17 2009
>> New Revision: 739504
>> URL:
>> Log:
>> Initial checkin for the SparseRealVectorClass.
>> I know that it doesn't work 100% with the map*** methods that
>> shouldn't be used with a sparse vector.  I'll clean those up shortly
>> (including uncommenting unit tests).  Just want to get more eyes on
>> this for the methods that matter.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message