mikera / core.matrix

core.matrix : Multi-dimensional array programming API for Clojure
Other
701 stars 113 forks source link

svd behavior depends on current implementation #322

Open mars0i opened 7 years ago

mars0i commented 7 years ago

Applying svd to a persistent-vector or ndarray matrix throws an exception if the current implementation doesn't support svd (i.e. if it's persistent-vector or ndarray), but if the current implementation does support svd (vectorz or clatrix), then svd applied to a persistent-vector or ndarray matrix will return the value of the current implementation's svd. (I don't think that this is an intended behavior; it seems confusing and error-prone to me.) I don't know whether there are other functions that have similar behavior.

For example:

user=> (set-current-implementation :persistent-vector)
:persistent-vector
user=> (lin/svd (matrix :vectorz [[1 2][3 4]]))
{:U #vectorz/matrix [[0.40455358483375675,-0.9145142956773046],
[0.9145142956773046,0.40455358483375686]], :S #vectorz/vector [5.464985704219043,0.36596619062625746], :V* #vectorz/matrix [[0.5760484367663209,0.8174155604703631],
[0.8174155604703631,-0.5760484367663209]]}
user=> (lin/svd (matrix :persistent-vector [[1 2][3 4]]))

ExceptionInfo TODO: Not yet implemented: (mp/svd m options) for class clojure.lang.PersistentVector  clojure.core/ex-info (core.clj:4617)
user=> (lin/svd (matrix :ndarray [[1 2][3 4]]))

ExceptionInfo TODO: Not yet implemented: (mp/svd m options) for class clojure.lang.PersistentVector  clojure.core/ex-info (core.clj:4617)
user=> (set-current-implementation :vectorz)
:vectorz
user=> (lin/svd (matrix :persistent-vector [[1 2][3 4]]))
{:U #vectorz/matrix [[0.40455358483375675,-0.9145142956773046],
[0.9145142956773046,0.40455358483375686]], :S #vectorz/vector [5.464985704219043,0.36596619062625746], :V* #vectorz/matrix [[0.5760484367663209,0.8174155604703631],
[0.8174155604703631,-0.5760484367663209]]}
user=> (lin/svd (matrix :ndarray [[1 2][3 4]]))
{:U #vectorz/matrix [[0.40455358483375675,-0.9145142956773046],
[0.9145142956773046,0.40455358483375686]], :S #vectorz/vector [5.464985704219043,0.36596619062625746], :V* #vectorz/matrix [[0.5760484367663209,0.8174155604703631],
[0.8174155604703631,-0.5760484367663209]]}
mikera commented 7 years ago

The idea was that the default implementation can fall back to the current implementation if the array you call SVD on doesn't otherwise support it.

This seems sensible - if your current implementation supports SVD then I think using it is better than an UnsupportedOperationException in such cases?

Can you think of a better solution?

Agree this default behaviour should be better documented.

mars0i commented 7 years ago

I see. I don't know. I see what you're saying, but on the other hand it seems odd that whether it falls back or throws an exception depends on what the current implementation is set to. I was experimenting and had current implementation set to vectorz, and then restarted without changing the current implementation (i.e. it was persistent-vector), and all of a sudden there were exceptions thrown. That was surprising. I don't have a strong opinion about it, but I think I would prefer to get the exception no matter what, and then I can respond by explicitly convert to an implementation that has svd if I want. But this is not a big deal for me. I had never used svd until I started writing a Moore-Penrose inverse. (Working on that and learning what I needed to know in spare moments.)

For a moment I was thinking that if the current implementation doesn't support svd, it should fall back to ndarray, or maybe vectorz first if that's available, but I think that would probably cause problems with aljabr in Clojurescript. I assume that ndarray is still Clojure-only.

btw I just noticed that svd returns nil on an aljabr matrix. Is that what one should expect?

cljs.user=> (ns foo.bar (:require [clojure.core.matrix :as mx] [thinktopic.aljabr.core :as al] [clojure.core.matrix.linear :as lin]))
nil
foo.bar=> (lin/svd (mx/matrix :aljabr [[1 2][3 4]]))
nil