Skip to content

Change return type of size() of Dense and SparseMatrix to avoid inconsistencies #3529

@gwhitney

Description

@gwhitney

Describe the bug
Consider

const SM = math.sparse([[1, 0]])
const DM = math.matrix([[1, 0]])

Both SM and DM represent the 2×1 matrix [[1, 0]], just with different internal representations. But as we shall see, they do not have equal or even equivalent results of the size() function which purports to give the length of each dimension of a matrix. Since those lengths match for these two entities, size() should produce at least equivalent results on them.

To Reproduce
Now execute

console.log(size(SM).toString()) // [[1], [2]]
console.log(size(DM).toString()) // [1, 2]

The difficulty is that the current convention is that size() returns a collection of the same type as its argument, when its argument is a collection. But critically, SparseMatrix cannot currently encode anything but 2-dimensional collections, so size(SM) returns a column vector as a 1×2 SparseMatrix, whereas size(DM) returns a 1-dimensional DenseMatrix (of length 2). These two return values are not === or == or even math.deepEqual(), which is inappropriate given that SM and DM are surely of identical size.

Discussion

The mathjs library should have a size() function with the property that the return values are at least deepEqual on any two entities that are the same size. I think the size() function provides an example in which it is counterproductive to return different types depending on the type of the input. The size() function is not like, say, the diff() function that returns a new collection based on the entries of an input collection, where it is helpful to preserve the type of the input in generating the output. Rather, it is providing a property of the input, and it is a property that all representations of collections have in common, and so that property should have a fixed type regardless of the specific type of the collection argument. It is just a coincidence that the property happens to be collection-valued, and so it is a red herring to make the type of that property depend on the type of the input, since it's the same property regardless.

Therefore, I have two proposals to resolve the difficulty, either of which could be adopted (they are mutually exclusive), or I would be open to other ideas that would preserve the condition that matrices of the same size have deepEqual size() values.

  1. Just adopt the convention that the size of (any instance of any type of) matrix is a plain JavaScript array. This type for the size is completely adequate and appropriate in that the size is just a list of numbers. This proposal is also completely consistent with the fact that the M.size() method always returns a plain JavaScript array. (This latter fact is not sufficient, however, to resolve the problem with the size() function, because there is no A.size() method on plain arrays, and there needs to be some consistent way to get the size of any collection that might be an Array or either type of Matrix.)

  2. Instead, always have size(M) return a collection of the type config.matrix (defaulting to DenseMatrix if config.matrix is unspecified), regardless of the specific type of the collection M.

Note that both of these proposals have the property that the type of the size of a collection does not depend on the specific type of the collection, which I would recommend as the proper way to go. Personally, I have a slight preference for (1), but could really go either way.

Unfortunately either of these proposals is a breaking change, so I would hope this issue could be considered, and a fix be chosen, in time to include a PR in mathjs 15 or at worst 16.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions