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

math.subset converts matrix to scalar #1484

Closed
DbCrWk opened this issue Apr 23, 2019 · 8 comments
Closed

math.subset converts matrix to scalar #1484

DbCrWk opened this issue Apr 23, 2019 · 8 comments

Comments

@DbCrWk
Copy link

DbCrWk commented Apr 23, 2019

First, this is a great package!

One small quirk that I've noticed is that if I do something like:

const matrix = [[1, 0], [0, 1]];
const expectedSubMatrix = [[ 1 ]]; // this is a 1-by-1 matrix

const upperElement = math.range(0, 0);
const actualSubMatrix = math.subset(matrix, math.index(upperElement, upperElement));
// actualSubMatrix is the scalar/number 1, rather than a 1-by-1-matrix

Is this an intentional casting of a 1-by-1 matrix into a scalar? If so, is there a way to enforce that pulling the subset of a matrix always returns a matrix?

(For my particular use case, I have a function that accepts an arbitrary index k and returns the k-by-k upper left submatrix. Therefore, to get around this quirk, I have to special case the situation when the passed k value is 1.)

@pdeaville

@josdejong
Copy link
Owner

Thanks @DbCrWk, great to hear you like the library :)

Changing 1x1 output of a matrix subset is indeed intentional. When retrieving a single value from a matrix, often you want to continue working with it as being a scalar.

There is currently no way to enforce output being kept a matrix. We could consider making this possible for example by configuring predictable: true globally, or passing this as an extra option to subset.

What you could do for now is create a small wrapper around math.subset which checks if the output returned by math.subset is a scalar and if so, turn it into a 1x1 Matrix again and return that.

@DbCrWk
Copy link
Author

DbCrWk commented Apr 24, 2019

Good to know. I have a wrapper written around it for now. If you're good with some parameter being passed (or equivalent), I can open a PR on this!

@josdejong
Copy link
Owner

Thanks for your offer Dev. I'm still a bit in doubt about what would be the best approach here: passing an option along with subset, or using the global option predictable for this. I think I have a slight preference for using a global option for this, since inside the editor you use bracket notations like c = A[1, 2], which under the hood uses subset, but there is no way to pass an option there.

@DbCrWk
Copy link
Author

DbCrWk commented Apr 29, 2019

Sounds good! I'll take a crack at it.

@josdejong
Copy link
Owner

Cool, thanks!

This will be a breaking change so we have to schedule when to release.

@DbCrWk can you please work from the modular_architecture branch instead of develop? That will save us some merge conflicts. We're about to release a large, breaking refactor of mathjs, which for example has a different dependency injection mechanism. See #71.

@DbCrWk
Copy link
Author

DbCrWk commented Apr 30, 2019

Cool, will do

@josdejong
Copy link
Owner

Dev, I've just merged modular_architecture into develop, so please continue any work there again.

@gwhitney
Copy link
Collaborator

gwhitney commented Oct 6, 2023

Closing as duplicate of #2344.

@gwhitney gwhitney closed this as completed Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants