Fixed error in matrix multiplication that didn't multiple non-square … - #898
internetdhruv wants to merge 2 commits into
Conversation
…matrices. Also added checks for invalid dimensions
StephenGemin
left a comment
There was a problem hiding this comment.
Maybe I'm just missing the point of this module, but I think we can do the same thing, with less code.
There was a problem hiding this comment.
with numpy arrays, no need to define matrix_c at the beginning of the function. Also, since I can't access the beginning of this module would need to add "import numpy as np".
Below is a suggestion to reduce the number of code lines. Maybe I'm missing the point of this module, but using numpy would be a lot more efficient. I know the rest of the doc has no docstring, but I figured I would add one in the suggestion.
You can the entire function with just 8 lines of code.
There was a problem hiding this comment.
This definitely works. In fact, a lot of stuff in this project can be done using in-built/library functions, but I think the idea of this project is to show how the core algorithms work.
Using numpy.dot would definitely make it work in lesser lines of code, and possibly even be faster, but that level of abstraction would hide what exactly is happening.
I may be wrong though, and if what we're going for is smaller code and speed, your suggestion would work (Although I think numpy.matmul would be a better choice)
There was a problem hiding this comment.
That makes sense. You're right on both points ☺. Best leave your initial commit than.
I never heard of numpy.matmul, now I know.
https://www.numpy.org/devdocs/reference/generated/numpy.dot.html
TLDR: If both a and b are 2-D arrays, it is matrix multiplication, but using matmul or a @ b is preferred.
Appreciate the feedback!

…matrices. Also added checks for invalid dimensions
The original code for matrix multiplication did not support non square matrices (reference to #837 )
The function also did not check if the 2 matrices can even be multiplied.
The error has been fixed, and the check has been added