commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Barker" <billwbar...@verizon.net>
Subject Re: [math] Re: commons-math, matrix-toolkits-java and consolidation
Date Fri, 22 May 2009 03:20:41 GMT
Time zones mean that I tend to come in late here :(.  More answers inline.

----- Original Message ----- 
From: "Sam Halliday" <sam.halliday@gmail.com>
To: <dev@commons.apache.org>
Sent: Thursday, May 21, 2009 3:13 AM
Subject: Re: [math] Re: commons-math, matrix-toolkits-java and consolidation


>
> Bill, I've had a look at some of the recent changes... some comments,
> including some new thoughts:-
>
> - changing the return type to be actual classes was only supposed to be 
> for
> copy() for 2.0. Doing this on multiply, add, etc is a big mistake... there
> is no guarantee that is the best thing to do and it is likely this will
> change in the future. This stands for all implementations. Great thought
> needs to go into each method, not just by reading the current
> implementation.
>

I sort of like being able to use the optimized methods for inlining, but 
don't feel strongly on this.  I'll go for whatever the community consensus 
is (and I don't see one at the moment).  It was a search/replace before and 
I can do it again ;).

> - I *strongly* urge you to remove Serializable from everything! Please, we
> did this in MTJ and it turned out to be a major pain. A more appropriate
> approach is to define a class for reading/writing Matrix Market files. 
> This
> can be a new feature in 2.1. If you're going to leave it, at least 
> document
> that the Serializable form is not guaranteed to remain compatible across
> versions.
>

Like Luc, I'm generallly in favor of Serializable.  Since some of the posts 
on this thread have suggested problems with the current implementation, I'll 
try and run some tests over the (what is here, long) weekend.  Again, no 
consensus so not doing anything immediately.

> - I discourage the use of the classes named *Impl. They will get very
> confusing when other implementations are added later! Instead, I recommend
> the names ArrayRealVector, Array2DRowRealMatrix (to indicate a 2D array
> backed implementation using row ordering). This allows a column-based or 
> 1D
> implementation in the future without names getting very confusing. These
> implementations are hidden from users who just use the MatrixUtils help
>

Have no problem doing a deprecate-and-copy.  Will do once there is a 
consensus on the name.

> - -1 for inclusion of methods in the sparse interfaces and shape type 
> enum.
> This needs more thought and discussion before inclusion. I am happy enough
> for marker interfaces to be included (as long as it is documented that 
> these
> interfaces are subject to update in the next release), but strongly 
> against
> including methods in them.
>

In the current SVN, these are empty marker interfaces, since we seem to have 
a consensus on that.

> - could you please describe the hashing algorithm that OpenMap is using? I
> have previously written a GNU Trove based sparse matrix implementation 
> which
> was a map from the primitive "long" to "double". It looks analagous. I 
> broke
> the row/column index into a long for the key using binary operations, but
> the hashcode was tricky as it had to be an integer (not long). A naive
> implementation can lead to collisions for typical or symmetric matrices. 
> It
> looks like the current implementation is using an int -> double backing 
> map!
> Wouldn't it make more sense to use a long?
>

I've just been playing Code-Monkey, so Phil and Luc can do more interesting 
stuff, and we can get the 2.0 version released ;).  OpenIntToDoubleHashMap 
was already there when I got involved with [math], so I just used it.  Luc 
was the one that initially committed OpenIntToDoulbleHashMap, but I think 
that it came from a Jira patch (and, yes, I'm too lazy to look up which 
one).

> The following code might be useful to you (don't forget the L marker and
> casts or the compiler will silently cast to an int), there are two options
> for a hashcode calculator... I've not stress tested the second one but it
> might be more appropriate as it gives more weight to lower bits. Remember
> that FF is 256, or a full byte... so a full 64 bit long is 8 bytes or 16 
> hex
> characters in Java bitwise.
>
>       int hash1(long val) {
>               int first = Integer.reverse(key2row(val));
>               int second = key2col(val);
>               return first + second;
>       }
>
>       int hash2(long val) {
>               int first = key2row(val) & 0x0000FFFF;
>               int second = key2col(val) << 16;
>               return first + second;
>       }
>
>       long key(int row, int column) {
>               return (((long) row) << 32) + (long) column;
>       }
>
>       int key2col(long key) {
>               return (int) (key & 0x00000000FFFFFFFFL);
>       }
>
>       int key2row(long key) {
>               return (int) (key >>> 32);
>       }
>
>
> Bill Barker wrote:
>>
>> I have a slight preference for leaving the markers empty until 3.0, but I
>> can remove them as well.  But I can wait to see what the community
>> consensus
>> is before making changes.
>>
>
> -- 
> View this message in context: 
> http://www.nabble.com/commons-math%2C-matrix-toolkits-java-and-consolidation-tp23537813p23650705.html
> Sent from the Commons - Dev mailing list archive at Nabble.com.
>
>
> ---------------------------------------------------------------------
> 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


Mime
View raw message