On 21/05/2009, Luc Maisonobe <luc.maison...@free.fr> wrote:
> 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.
One can use the "transient" marker for non-serializable fields.
Merely implementing Serializable will prevent the warnings, however in
general it's not trivial to ensure that serialization works correctly.
> 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
