Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make documentation clearer that the top-level elements of a 2D rectangular array are considered **rows** of matrices. #2618

Open
JC3 opened this issue Jul 6, 2022 · 8 comments
Labels
documentation Concerns about or enhancements to the mathjs documentation help wanted

Comments

@JC3
Copy link

JC3 commented Jul 6, 2022

I think it would be very helpful if the docs for MathJS mentioned that math.multiply on matrices was pre-multiplication, rather than post-multiplication.

Especially given that all the language at https://mathjs.org/docs/reference/functions/multiply.html strongly implies that math.multiply(a, b) is the product ab for the rest of the types. The only indication that it's pre-multiplication for matrices is somewhat buried down in the example, where it's only implicit in the dimensions of the input matrices.

Thanks! :)

@JC3 JC3 changed the title matrix multiplication documentation matrix multiplication documentation (pre vs post multiply) Jul 6, 2022
@josdejong
Copy link
Owner

Thanks for your input Jason. Sounds good to clarify that in the docs. Can you put together a PR for that? Or anyone else?

So I guess the best place would be in the docs of multiply.js:

/**
* Multiply two or more values, `x * y`.
* For matrices, the matrix product is calculated.
*
* Syntax:
*
* math.multiply(x, y)
* math.multiply(x, y, z, ...)
*
* Examples:
*
* math.multiply(4, 5.2) // returns number 20.8
* math.multiply(2, 3, 4) // returns number 24
*
* const a = math.complex(2, 3)
* const b = math.complex(4, 1)
* math.multiply(a, b) // returns Complex 5 + 14i
*
* const c = [[1, 2], [4, 3]]
* const d = [[1, 2, 3], [3, -4, 7]]
* math.multiply(c, d) // returns Array [[7, -6, 17], [13, -4, 33]]
*
* const e = math.unit('2.1 km')
* math.multiply(3, e) // returns Unit 6.3 km
*
* See also:
*
* divide, prod, cross, dot
*
* @param {number | BigNumber | Fraction | Complex | Unit | Array | Matrix} x First value to multiply
* @param {number | BigNumber | Fraction | Complex | Unit | Array | Matrix} y Second value to multiply
* @return {number | BigNumber | Fraction | Complex | Unit | Array | Matrix} Multiplication of `x` and `y`
*/

@JC3
Copy link
Author

JC3 commented Jul 7, 2022

I can try except I am realizing the documentation issue is a bit larger than that, and statements like "the dimensions are meaningless" need to be clarified and some decisions made on what is a row vs. a column in a 2D matrix, since it affects the description of multiplication. So I'm not really sure what the solution is to bring clarity to the documentation, except maybe to leave a note that specifically says that for column-major matrices, multiply(a,b) is a pre-multiply and represents the product ba.

@josdejong
Copy link
Owner

Makes sense to be pragmatic in this regard. In the almost 10 year that the library exists this is the first time this unclarity is brought up, so it may not be that big of a problem.

So an explanatory note somewhere in the docs is possibly enough. What pointer would have helped you to figure this out?

@JC3
Copy link
Author

JC3 commented Jul 7, 2022

Makes sense to be pragmatic in this regard. In the almost 10 year that the library exists this is the first time this unclarity is brought up, so it may not be that big of a problem.

So an explanatory note somewhere in the docs is possibly enough. What pointer would have helped you to figure this out?

I'm collecting comments on documentation confusion points (re: rows, columns, and multiplication) from coworkers today and will let you know what comes out of it.

I agree that it makes sense to be pragmatic, but I also don't want to be hasty: I.e. it is probably a good idea to glance over the rest of the API to see if anything else is directly affected by a column-major vs. row-major choice.

Also I've edited my original post for tone; sorry about that. I should have waited for the debugging rage to cool down before posting 😂.

I can handle this PR but it will take some time.

@JC3
Copy link
Author

JC3 commented Jul 7, 2022

By the way:

In the almost 10 year that the library exists this is the first time this unclarity is brought up

I keep thinking about that too and I'm very curious about it. I can think of a lot of possible reasons (ranging from 'maybe post multiplication is the more common convention' to 'maybe people don't actually do a lot of matrix math in JS' to 'maybe its covered in a stack overflow post somewhere and so nobody bothers reporting it' to 'maybe I am hugely misinterpreting something'), but I don't have any theories that really stand out. It's a bit strange. In any case, clarity can't hurt.

@josdejong
Copy link
Owner

ha ha, yes I did sense some frustration originally 😉

In any case, clarity can't hurt.

Yes definitely, let's see what we can improve to help future users!

@gwhitney
Copy link
Collaborator

I want to thank everyone in this conversation for their input. However, I am very confused as to what the real issue is here. The example of multiplication from the docs quoted above is

    [ 1 2 ]       [ 1  2 3 ]
c = [     ],  d = [        ]
    [ 4 3 ]       [ 3 -4 7 ]

                     [  7 -6 17 ]
math.multiply(c,d) = [          ]
                     [ 13 -4 33 ]

which is in fact the usual matrix definition of the matrix product cd. So I do not understand what's meant by pre-multiplication vs post-multiplication in the concerns expressed here?

The documentation doesn't say the "dimensions are meaningless". It says they have no pre-assigned meaning. Of course, the multiplication convention tells you in the 2D case which dimension is considered the rows and which is the columns. Namely, in array form [[a, b, c], [d, e, f] the [a, b, c] is a row (which I think is very ordinary in representing matrices in JavaScript). But the point of the comment is particularly directed toward 3-dimensional matrices and higher, and it's particularly apt because mathjs has no definition for the product of two three-dimensional matrices (which is sensible because there isn't a uniquely defined product).

Other than perhaps being more clearly emphatic that in [[a, b, c], [d,e,f]] the [a, b, c] is a row of the matrix, I don't see what's needed to improve the documentation. Thanks for everyone's help in resolving this.

@gwhitney gwhitney added the documentation Concerns about or enhancements to the mathjs documentation label Oct 3, 2023
@gwhitney gwhitney changed the title matrix multiplication documentation (pre vs post multiply) Make documentation clearer that the top-level elements of a 2D rectangular array are considered **rows** of matrices. Oct 3, 2023
@gwhitney
Copy link
Collaborator

gwhitney commented Oct 3, 2023

Since there's been no further input in the past year, I conclude that the core of this issue is simply greater clarity and emphasis in the documentation that rectangular 2-dimensional Arrays are interpreted as row-major matrices. So I have retitled the issue accordingly. A documentation PR remains welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Concerns about or enhancements to the mathjs documentation help wanted
Projects
None yet
Development

No branches or pull requests

3 participants