commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rahul Akolkar <rahul.akol...@gmail.com>
Subject Re: svn commit: r739504 - in /commons/proper/math/trunk/src: java/org/apache/commons/math/linear/SparseRealVector.java test/org/apache/commons/math/linear/SparseRealVectorTest.java
Date Sat, 31 Jan 2009 19:24:48 GMT
Minor comment, and arguably subjective -- I find the signature for
checkVectorDimensions(int) a little odd for a public method (I'd make
it private).

-Rahul


On Sat, Jan 31, 2009 at 10:13 AM,  <luc.maisonobe@free.fr> 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
>
> ----- billbarker@apache.org a écrit :
>
>> Author: billbarker
>> Date: Sat Jan 31 04:51:17 2009
>> New Revision: 739504
>>
>> URL: http://svn.apache.org/viewvc?rev=739504&view=rev
>> 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.
>>
<snip/>

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


Mime
View raw message