-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
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.
-
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 thesize()function, because there is noA.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.) -
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.