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

Matrix sum along a dimension is defined to return a MathScalarType, but might return higher dimension #3202

Open
itaiperi opened this issue May 12, 2024 · 5 comments

Comments

@itaiperi
Copy link

itaiperi commented May 12, 2024

Describe the bug
When summing over an axis of a matrix, we always lose just one dimension. If the matrix is 2D, we'll get a 1D array. If the matrix is 3D, we'll get a 2D matrix.
What happens now, is that math.sum declares that it returns a MathScalarType, instead of optionally returning a matrix.

To Reproduce

const mat = math.matrix([[1,2,3],[4,5,6]])
console.log(math.sum(mat, 0))

> DenseMatrix { _data: [ 5, 7, 9 ], _size: [ 3 ], _datatype: undefined }
@josdejong
Copy link
Owner

Thanks for reporting Itai.

We have to fix the TypeScript type definitions for sum (and check functions like prod to see if there are similar issues there).

Anyone able to help with a PR fixing the types?

@deflucaseng
Copy link
Contributor

Could I get assigned to this?

@josdejong
Copy link
Owner

Thanks @deflucaseng, I've assigned you for this task.

@deflucaseng
Copy link
Contributor

@josdejong I also saw that deepForEach.test.js was left as TODO. Is that something I can also work on?

@josdejong
Copy link
Owner

Definitely! Thanks Lucas. Can you please create two separate PR's, so each addresses 1 thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants