Sam Halliday a écrit :
> 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.
It may help the compiler using directly the optimized versions in
statements like:
m1.add(m2.multiply(m3.subtract(m4))
>
> - 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.
I disagree with and I think its me who started adding this interface
everywhere. The rationale is explained in the following use case:
As a user, I want to implement an interface for some xyz library where
the interface already extends Serializable. In my implementation, I want
to have an instance field that is some RealMatrix. If RealMatrix does
not declare it extends Serializable, I keep getting warnings about
having a non-serializable field in a serializable class. Looking the
other way round having serializable and not using it never caused me any
pain because it was really a generalized pattern. Mixing serializable
and non serializable is difficult, putting it everywhere and sticking to
this policy is simple.
What kind of pain did Serializable cause in MTJ ?
BTW, I'm really +1 to Matrix Market files, could you open a Jira issue
asking for this enhancement for 2.1 ?
>
> - 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
Phil suggested to change RealMatrixImpl to ArrayRealMatrix (and
DenseRealMatrix to BlockRealMatrix). This sounds good to me.
>
> - -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.
+1 to further thoughts and discussion and postponing declaring methods
in the interfaces after 2.0. Lets start with empty marker interfaces.
Luc
>
> - 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?
>
> 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.
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
