sebb a écrit : > 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.
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. 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-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 > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org