Misleading Ord implementation for Vector.Storable

[DanielMe]

(The following is actually a consequence of the implementation in the vector package. However, I think it is not really a bug there. At the same time HMatrix tries to provide a convenient interface for matrix/vector operations so I guess it should be solved here at the API level.)

Consider the following minimal example:

*Numeric.LinearAlgebra.Data> let a = fromList [0,1,1]
*Numeric.LinearAlgebra.Data> let b = fromList [0,2,-2]
*Numeric.LinearAlgebra.Data> a <= b
True

It is clear where this behaviour comes from: The Ord class always requires a total ordering (due to the definition of the compare method) which vectors are not. I suppose Vector.Storable only has an Ord instance to make it compatible with Data.Set et al.

However, I think this is a big API problem. It is very tempting to write a <= b as shown above in your code which will compile but produce totally unexpected results.

I'm not sure how this can be fixed - we cannot simply hide this instance due to the open world assumption. But at the very least, there should be very obvious lt and gt implementations that do what you would expect. Maybe even a PartialOrd typeclass.

Another option would be to create a newtype wrapper for the Vector type without an Ord instance. This way it wouldn't be possible to make the above mentioned mistake. On the other hand this would be more cumbersome to implement and not backwards compatible.

[albertoruiz]

The vectors provided by the safer statically checked interface do not have an Ord instance (here are a few usage examples.)

Early hmatrix versions provided a custom vector type but later it seemed that using directly the Storable vector from the vector package would facilitate interoperation with other libraries. I am not sure that this wrong Ord instance (for mathematical vectors) is a problem big enough to give up the practical advantages of working with a standard Haskell type in the traditional, dynamic dimension hmatrix interface.

[DanielMe]

Thanks for the note about the static interface - I was planning to switch to that eventually anyway.

I agree that getting rid of the Ord instance is probably not worth it. But there should at least be a very obvious set of functions that implements the vector ordering one would expect with a big fat warning in the doc to not use <= and friends.

[albertoruiz]

Ok, I will add a warning in the docs. But it is not clear to me what comparison function should be included: compare :: vector -> vector -> Maybe Ordering or lt,gt :: vector->vector->Bool ? Comparing vectors of different sizes should produce an error? Perhaps it is not a big problem to define on the fly a suitable comparison function for each specific application. For instance: vlessEqThan u v = (size u ==) $ fromIntegral$ sumElements $ cond u v 1 1 (0::Vector I) (Int (32 and 64) elements are supported in the repo version, not yet on Hackage)