On 21/05/2009, Luc Maisonobe <Luc.Maisonobe@free.fr> wrote:
> sebb a écrit :
>
> > On 21/05/2009, Luc Maisonobe <Luc.Maisonobe@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.
>
>
> Transient is OK when the field state can be reconstructed on
> deserialization from other fields information. This is quite rare and is
> surely not the case with matrices which may be huge and therefore not
> duplicated in several different fields.
>
>
> >
> > Merely implementing Serializable will prevent the warnings, however in
> > general it's not trivial to ensure that serialization works correctly.
>
>
> This is probably simpler in math objects (except graph theory perhaps)
> than in other business domains. Our objects typically hold a primitive
> and arrays and some other lower level objects with similar simple
> structures.
>
Nevertheless, adding serialization needs to be considered carefully,
as pointed out here:
http://www.javapractices.com/topic/TopicAction.do?Id=45
Furthermore, readObject() cannot be used with final fields. Bloch
(Effective Java) suggests using readResolve() instead, but even this
has limitations.
As the book points out, merely making a class Serializable is
equivalent to adding a public constructor which sets all the
non-transient fields without perfoming any validation.
If there are any constraints on the fields, these have to be addressed
in readObject() and/or readResolve() methods.
> Luc
>
>
> >
> >> 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-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
> >
> >
>
>
> ---------------------------------------------------------------------
> 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
|