mikera / core.matrix

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

incompatible shapes when multiplying by transpose #279

Closed crinklywrappr closed 8 years ago

crinklywrappr commented 8 years ago

thoughts?

ml-solver1.core> (let [A (m/matrix [[1 2 3] [4 5 6]])]
                   (m/mul A (m/transpose A)))
ExceptionInfo Incompatible shapes, cannot broadcast shape [3 2] to [2 3]  clojure.core/ex-info (core.clj:4617)
ml-solver1.core> (let [A (m/matrix [[1 2 3] [4 5 6]])]
                   (m/mul (m/transpose A) A))
ExceptionInfo Incompatible shapes, cannot broadcast shape [2 3] to [3 2]  clojure.core/ex-info (core.clj:4617)
ml-solver1.core> *e
#error {
 :cause "Incompatible shapes, cannot broadcast shape [2 3] to [3 2]"
 :data {}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message "Incompatible shapes, cannot broadcast shape [2 3] to [3 2]"
   :data {}
   :at [clojure.core$ex_info invokeStatic "core.clj" 4617]}]
 :trace
 [[clojure.core$ex_info invokeStatic "core.clj" 4617]
  [clojure.core$ex_info invoke "core.clj" 4617]
  [clojure.core.matrix.impl.persistent_vector$eval25776$fn__25777 invoke "persistent_vector.cljc" 151]
  [clojure.core.matrix.protocols$eval20718$fn__20719$G__20709__20726 invoke "protocols.cljc" 281]
  [clojure.core.matrix.impl.persistent_vector$eval25786$fn__25787 invoke "persistent_vector.cljc" 166]
  [clojure.core.matrix.protocols$eval20774$fn__20775$G__20765__20782 invoke "protocols.cljc" 299]
  [clojure.core.matrix.protocols$broadcast_compatible invokeStatic "protocols.cljc" 1232]
  [clojure.core.matrix.protocols$broadcast_compatible invoke "protocols.cljc" 1222]
  [clojure.core.matrix.impl.persistent_vector$eval25937$fn__25938 invoke "persistent_vector.cljc" 409]
  [clojure.core.matrix.protocols$eval21644$fn__21645$G__21635__21652 invoke "protocols.cljc" 512]
  [clojure.core.matrix$mul invokeStatic "matrix.cljc" 1413]
  [clojure.core.matrix$mul invoke "matrix.cljc" 1403]
  [ml_solver1.core$eval38933 invokeStatic "form-init8747809854634968286.clj" 284]
  [ml_solver1.core$eval38933 invoke "form-init8747809854634968286.clj" 283]
  [clojure.lang.Compiler eval "Compiler.java" 6927]
  [clojure.lang.Compiler eval "Compiler.java" 6890]
  [clojure.core$eval invokeStatic "core.clj" 3105]
  [clojure.core$eval invoke "core.clj" 3101]
  [clojure.main$repl$read_eval_print__7408$fn__7411 invoke "main.clj" 240]
  [clojure.main$repl$read_eval_print__7408 invoke "main.clj" 240]
  [clojure.main$repl$fn__7417 invoke "main.clj" 258]
  [clojure.main$repl invokeStatic "main.clj" 258]
  [clojure.main$repl doInvoke "main.clj" 174]
  [clojure.lang.RestFn invoke "RestFn.java" 1523]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__648 invoke "interruptible_eval.clj" 87]
  [clojure.lang.AFn applyToHelper "AFn.java" 152]
  [clojure.lang.AFn applyTo "AFn.java" 144]
  [clojure.core$apply invokeStatic "core.clj" 646]
  [clojure.core$with_bindings_STAR_ invokeStatic "core.clj" 1881]
  [clojure.core$with_bindings_STAR_ doInvoke "core.clj" 1881]
  [clojure.lang.RestFn invoke "RestFn.java" 425]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate invokeStatic "interruptible_eval.clj" 85]
  [clojure.tools.nrepl.middleware.interruptible_eval$evaluate invoke "interruptible_eval.clj" 55]
  [clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__693$fn__696 invoke "interruptible_eval.clj" 222]
  [clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__688 invoke "interruptible_eval.clj" 190]
  [clojure.lang.AFn run "AFn.java" 22]
  [java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1145]
  [java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 615]
  [java.lang.Thread run "Thread.java" 744]]}
crinklywrappr commented 8 years ago

Maybe I'm using it wrong. operators/* & operators/\ both give this error, but matrix/mmul works as expected.

mikera commented 8 years ago

* is elementwise multiplication, so the matrices need to be the same shape (not transposes).

Likewise ** is elementwise exponent.

So this is all as expected :-)

crinklywrappr commented 8 years ago

Did this change? I'm reading through a book on machine learning in Clojure and at every step in the first chapter they assume operators/* performs matrix multiplication.

blueberry commented 8 years ago

The confusion between * and mmul arises often - the documentation should be much better IMO. BUT, I suspect the problem in this case lies in the book. Packt publishing books are notorious for their low quality, but if it is true (I didn't check) that the author missed even the most important operation, it is something that I didn't expect even from them.

mikera commented 8 years ago

I've tried to be as consistent as possible with other matrix libraries when defining this, e.g. NumPy also assumes multiply is elementwise: http://docs.scipy.org/doc/numpy-1.10.1/reference/generated/numpy.multiply.html

*, -, /, and + are all elementwise in c.c.m.operators.

I sometimes like to use inner-product instead of mmul to avoid ambiguity (they do the same thing).

@blueberry how would you improve the documentation for this?

blueberry commented 8 years ago

@mikera By stressing in early examples and in many other places that mmul is for multiplication and * is not. This issue arises regularly, it even have it's own stackoverflow topic. I also think that reusing Clojure's * + etc operators for matrices is confusing; (emap * x y) is here and is much clearer). But this is another topic, that I won't get into. Users are very creative when it comes to misuse :) I do not know whether that would help, cause many people tend to not read the docs at all. As I said, in this case, the problem is in the low quality of the book publisher.

blueberry commented 8 years ago

Just another comment regarding numpy as a reference: Isn't that in numpy matrix multiplication is done by x.dot(y)? Not very intuitive, if you ask me...

mikera commented 8 years ago

Well.... dot is conventionally used in mathematics to indicate inner product, which of course is a generalisation of matrix multiply. So I guess it makes sense.

An observation is that there seems to be a lot of prejudice about notation depending on whether you come from engineering, pure maths, stats, computer science etc. :-)

blueberry commented 8 years ago

Sure, it makes some sense when explained in detail; but by that logic, the right name for elementwise multiply would be "asterisk"? What people first think of when they read dot in the context of (computational) linear algebra is "dot product". Now, having two methods with the same name doing two different fundamental operations for two different fundamental data structures is a bit confusing. That's why there is vdot in numpy. So, matrix multiplication is called "dot" while dot product is called "vdot". I wouldn't call that intuitive, but ok.

cloojure commented 8 years ago

Matlab has a similar naming problem trying to differentiate between matrix operators and elementwise operators. They use * for matrix multiply (also works for scalars), and the .* operator for element-wise multiplication. They use + for either matrix or scalar addition, but beware of silent autoconversion since (A + 5) => (A + 5*I).

I have sometimes wondered if it might be worth it to make everything unambiguous, like mmul vs emul (matrix vs element multiplication), etc. Besides avoiding incorrect or conflicting assumptions, such names are much easier to search for using either grep or google. Alan

On Mon, Mar 28, 2016 at 3:17 AM, Dragan Djuric notifications@github.com wrote:

Sure, it makes some sense when explained in detail; but by that logic, the right name for elementwise multiply would be "asterisk"? What people first think of when they read dot in the context of (computational) linear algebra is "dot product". Now, having two method with the same name doing two different fundamental operations for two different fundamental data structures is a bit confusing. That's why there is vdot in numpy. So, matrix multiplication is called "dot" while dot product is called "vdot". I wouldn't call that intuitive, but ok.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/mikera/core.matrix/issues/279#issuecomment-202333062