clatrix icon indicating copy to clipboard operation
clatrix copied to clipboard

stable release

Open daydreamt opened this issue 11 years ago • 12 comments

Hey there,

Incanter is depending on clatrix for both QR decomposition and the determinant function, and since there is no recent non-SNAPSHOT version of clatrix yet, the latest Incanter version has no QR decomposition or determinant functions.

https://github.com/liebke/incanter/issues/122

What would it take to make a stable release? More SVD tests?

daydreamt avatar May 11 '13 14:05 daydreamt

Does Incanter build and test with the current snapshot ?

ejackson avatar May 11 '13 14:05 ejackson

The parts of Clatrix upon which Incanter depends haven't changed much in a while. The only movement is in tracking core.matrix, so I'm pretty happy with stability. Others ?

ejackson avatar May 11 '13 15:05 ejackson

I'm afraid it does not. The only change I could see was that the identity matrix function was renamed from id to eye. However, the tests fail. Here is travis running them: https://travis-ci.org/daydreamt/incanter

I will dig deeper later to ensure I haven't done anything stupid.

daydreamt avatar May 11 '13 15:05 daydreamt

So, I went through the failed tests and the error. The tests failed because of API changes, mostly.

(def A [ [1 2 3], [4 5 6], [7,8,9] ,[10,11,12]]

(conj A [13 14 15])
;=> now returns nil, didn't use to

(conj A [[13,14,15]])
;=> A 5x3 matrix
;=>  -------------
;=> 1.00e+00  2.00e+00  3.00e+00 
;=> 4.00e+00  5.00e+00  6.00e+00 
;=> 7.00e+00  8.00e+00  9.00e+00 
;=> 1.00e+01  1.10e+01  1.20e+01 
;=> 1.00e+00  2.00e+00  3.00e+00 
;something like this was returned before, I think

;The next error occurs due to some change in the matrix function. I think.
;;(def M (predict (simple-regression [2 4] [1 3]) 2))
;which returns
(def M (matrix [3]))
;=>  A 1x1 matrix
;=> -------------
;=> 3.00e+00 

(= 3.0 M)
;=> false, but used to be true

;The last one I have to think about. It has to do with vector multiplication.

The serious error is due to the first change, I think. It is not expected behaviour, right?

daydreamt avatar May 12 '13 00:05 daydreamt

If new Clatrix will be release, I'll incorporate it into Incanter immediately...

alexott avatar May 18 '13 12:05 alexott

My hacking to make add core.matrix support to Incanter will probably also depend on a new Clatrix release.

I can push out a release to net.mikera/clatrix if useful as a temporary measure?

mikera avatar May 27 '13 06:05 mikera

Yes, I think that this could work. Although I've discussed this with Edmund & Joseph - they're also ready to push new release when I do changes into Incanter that allow to work with new version

alexott avatar May 27 '13 06:05 alexott

OK, sorry for the absence. Back now.

  1. It seems that cons is incorrectly implemented. Its returning a concrete type where it should be returning a sequence. We need to change that, but its going to cause breakage.
  2. Conj is unimplemented. I'm not sure how to do this, can you give me pointers please ? As I understand it is not in any of the core interfaces or existing protocols. Should we create our own protocol and extend it Matrix and Vector ?
  3. (not (= 3 (matrix [3]))) is the correct behaviour, I think. Agree ?

ejackson avatar May 27 '13 11:05 ejackson

The whole "treat a matrix like a sequence of sub-matrices" concept appears to be a bit problematic from what I've observed in my experiments to get Incanter working with core.matrix. There's a lot of inconsistency lurking around which makes it hard to define correct behaviour.

If we're going to cause breakage anyway, I think it would be worth revisiting the design in this area.

mikera avatar May 27 '13 13:05 mikera

I don't like it myself, how much is it used in Incanter ?

ejackson avatar May 27 '13 20:05 ejackson

Hard to say: you don't need it (there are always other ways of achieving the same thing) but it's quite widely used in the docstrings and tests.

I suggest:

  1. Being more explicit when you actually want a sequence of rows or columns, e.g. telling people to use (first (columns A)) or (map something (rows A))
  2. Mimicking regular Clojure vectors / core.matrix behaviour, e.g. a row is considered to be a 1D Vector rather than a 1xN 2D Matrix. This also matches people's intuitive expectation about arrays in other languages. I believe this is the best way to ensure consistent behaviour

(1) can be done quite easily, just by deprecating the old usage and changing docstrings. (2) is relatively easy to do as well, but will almost certainly cause breakage, so we should probably save it for a major (2.0 ?) release of Incanter and corresponding (3.0? 4.0?) Clatrix release

mikera avatar May 28 '13 00:05 mikera

I've fixed the problem with bind-rows & fresh clatrix. The only remaining issue is: conj of matrix with vector that is described above.

If we can fix this issue, then I can cut the 1.5.0 release of Incanter & after that we can concentrate on 2.0 based on core.matrix.

alexott avatar May 31 '13 16:05 alexott