math.size and Matrix.size are inconsistent

[cshaa]

The .size() method on matrices always returns number[]. The math.size(x) method, which should be more robust, can take not only Matrix instances, but also array matrices and scalars. For arrays it returns number[], but for matrices it returns an instance of Matrix.

let a = [ [1], [2] ]
let m = math.matrix(a)
math.size(a) // [1,2]
m.size() // [1,2]
math.size(m) // Matrix([1,2])

This is unexpected and useless. Size is usefull when one can read the dimensions rightaway, with matrix, you have to convert it to array first.

[cshaa]

In the documentation for math.size, they say that:

math.size(2.3)                  // returns []
math.size('hello world')        // returns [11]

const A = [[1, 2, 3], [4, 5, 6]]
math.size(A)                    // returns [2, 3]
math.size(math.range(1,6))      // returns [5]

This is not true in the current version. Size actually returns Matrix instances for most of them, not arrays, as it's specified in the documentation.

[cshaa]

I was surprised how many tests dependend on size returning a matrix. But most of the time the dependence was only because the size was checked to be deep-equal to a matrix.

The only real-life consequence of size always returning arrays that I could think of is this:

let m = math.matrix([ [0, 1], [1, 0] ])
let e = math.ones( size(m) )
// used to return: matrix([ [1, 1], [1, 1] ])
// now returns: [ [1, 1], [1, 1] ]

This inconvenience can be easily fixed by using spread:

let e = math.ones( ...size(m) ) // matrix(...)

However, it is a breaking change.

[josdejong]

Thanks for your suggestions Michal. It's interesting to think about how we can make this "most" consistent.

So far, all functions of mathjs try to follow this behavior:

Array in -> Array out
Matrix in -> Matrix out

So functions like diag, ones and also size return a Matrix when the input is a Matrix, and return an Array when the input is an Array.

So I'm not sure if it's a good idea to change size such that it always returns an Array, since this will be inconsistent with this behaviour.

Just thinking aloud: would it make sense to have DenseMatrix.size return a DenseMatrix instead of an Array to make the behavior more consistent? If we would do that, I think we should also add a new method DenseMatrix.sizeAsArray or something like that else we would lose performance in all functions which utilize the DenseMatrix.size method on a low level.

Or we could simply leave this as is, and accept it is just a coincidence that the function math.size has the same name as the method DenseMatrix.size whilst they are not fully the same.

At least we should clarify the docs and examples so you can see when output is an Array or a Matrix.

[cshaa]

I understand that the current status quo is array in – array out; matrix in – matrix out. But is there actually a good reason why anybody would want size as a matrix? I struggle to imagine such a scenario. Matrices are useful if you multiply, transpose or decompose them. I don't think anybody will ever need to do something like this with size.

But then again, I might be wrong. If there are actual benfits to having size as a matrix, adding sizeAsArray sounds like a good idea to me. But either way, I think that math.size and DenseMatrix.size should be consistent.

[josdejong]

But is there actually a good reason why anybody would want size as a matrix?

That's a true point, I can't come up with a clear case where you want want the returned size to be a Matrix necessarily. The idea behind keeping types consistent input -> output is that you can easily predict what type your result will be: Matrix in, Matrix out. If you can rely on that, you automatically know what properties and methods it has (like matrix.format for example).

What is the reason why you want to have math.size and DenseMatrix.size the same? To me they are something different, just as different as DenseMatrix.size and Array.size (which are obviously not the same since Array.size simply doesn't exist 😉 ).

[cshaa]

What is the reason why you want to have math.size and DenseMatrix.size the same?

And what is the reason they shouldn't be the same? math.mul(Type, Type) is the same as Type.mul(Type), math.add(Type, Type) is the same as Type.add(Type),… This holds for almost any function. Making math.size and DenseMatrix.size behave differently is so wildly (and unnecessarily) inconsistent with the rest of the library that I can't even believe you ask about it 😀️

The idea behind keeping types consistent input -> output is that you can easily predict what type your result will be: Matrix in, Matrix out.

While I don't agree that this is a good principle, I respect it. If it's really that important for you, let's make a sizeAsArray function then. But again, I don't think anyone will ever need a size as a matrix.

[josdejong]

I've been giving this a bit more thought. I think having size always return an Array is the more practical, pragmatic approach, and that this outweighs having consistent input -> output.

So let's go for it and change size to always return an Array, schedule this for v7 ok?

[josdejong]

I've created a PR: #1805

I have to say, I'm not entirely happy. Some of the changes required in the unit tests show the effect of the changed behavior:

const a = math.matrix([[1, 2, 3], [4, 5, 6]])
const b = math.zeros(math.size(a))
assert.deepStrictEqual(b.size(), a.size())  // Error

now, if you are working with matrices, like a = matrix(...), and then create a new matrix based on properties of a, like b = zeros(size(a)), you end up with b being an Array instead of a Matrix, which feels really inconsistent and unexpected to me.

This really makes me doubt about it again if it's a good idea after all 😕

[cshaa]

I already mentioned this inconvenience in my previous comment. If one wants to create a DenseMatrix, the code you posted is easy to correct:

const a = math.matrix([[1, 2, 3], [4, 5, 6]])
const b = math.zeros(...math.size(a)) // spread the size
assert.deepStrictEqual(b.size(), a.size()) // works

But I agree that this isn't very straightforward and would probably be a bad design.

The problem here is that zeros, ones and identity were designed with the old API in mind. They count on the fact that the size of a matrix also contains information about the type of the matrix. Which, to me, sounds like something that shouldn't be true.

Instead of trying to work around the legacy API, I propose a new call signature for zeros, ones and identity in v7:

// When given a matrix, it will return a new matrix with the same type and dimensions
function zeros(matrix: DenseMatrix): DenseMatrix
function zeros(matrix: SparseMatrix): SparseMatrix
function zeros(matrix: number[][]): number[][]

// When given a size, it will create a DenseMatrix (the default one)
function zeros(size: number[]): DenseMatrix
function zeros(m: number, n: number): DenseMatrix

// When given a size and a type, it will create the correct type of matrix
function zeros(size: number[], type: 'dense'): DenseMatrix
function zeros(size: number[], type: 'sparse'): SparseMatrix
function zeros(size: number[], type: 'array'): number[][]

I'm willing to make a PR that implements this.

With this new API, the code you've posted would have been:

const a = math.matrix([[1, 2, 3], [4, 5, 6]])
const b = math.zeros(a)
assert.deepStrictEqual(b.size(), a.size()) // works

[josdejong]

I already mentioned this inconvenience in my previous comment.

Ah, your're right. Yes I had seen it coming too, but using it for real helps me weight the impact (for me and all current users with certain expectations). I'm sorry for the back-and-forth in this regard, but it still doesn't feel "right" so I'm reluctant to make this change. I have the feeling that we're going to mix the current API with a different type of API.

Instead of trying to work around the legacy API, I propose a new call signature for zeros, ones and identity in v7:
[...]

It's an interesting idea to have zeros and ones become more "smart" and have them take the size of the input in case the input is a SparseMatrix or DenseMatrix. It doesn't really appeal to me though, because of a couple of reasons:

• I like to have a simple, flat API. Having size do an extra interpretation step in certain cases make the function less transparent to me. I like that it is simply dimensions in -> Matrix out.
• In mathjs, a DenseMatrix can have any number of dimensions, 1-d, 2-d, n-d, etc. I find it important that the function behaves consistent for any type of matrix (Array, DenseMatrix, SparseMatrix). However, for 1-d Arrays the behavior would be different than 2-d or n-d Arrays in your proposal. So, zeros(denseMatrix) could behave differently than zeros(denseMatrix.toArray()).

The problem here is that zeros, ones and identity were designed with the old API in mind. They count on the fact that the size of a matrix also contains information about the type of the matrix. Which, to me, sounds like something that shouldn't be true.

I think this touches the core of our different way of looking at the API. The API of mathjs is designed around the simple rule "data type in -> same data type out". Such a rule gives predictability and makes it easy to use the library. The intention also is to use functions and no class methods (functional API). If you're doing calculations with BigNumbers, the results are in BigNumbers and should not suddenly switch to numbers. When calculating with plain Array, you get plain Array out, and not a DenseMatrix. And when working with DenseMatrices, you get DenseMatrices out and not suddenly switch to low level Arrays.

It may be good to take a step back and first get the bigger picture clear. You have some interesting ideas and a different view on things than me which is always good. Here an open question to you: if you could start from scratch, what API would you come up with for mathjs? Would you like to replace the "data type in -> same data type out" API with something totally different, and if so what?

[gwhitney]

There's a lot of back and forth here with no clear actionable outcome, so moving to Discussions.