Fix adding a BigNumber scalar to a number-datatype matrix#3677
Open
apoorva-01 wants to merge 1 commit into
Open
Fix adding a BigNumber scalar to a number-datatype matrix#3677apoorva-01 wants to merge 1 commit into
apoorva-01 wants to merge 1 commit into
Conversation
matAlgo14xDs forced the scalar into the matrix's datatype, so a BigNumber plus a `number` matrix threw instead of promoting. Take that fast path only when the scalar converts, otherwise fall back to the generic callback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add(matrix([[1, 2]], 'dense', 'number'), bignumber(3))threwCannot convert 3 to numberinstead of returning[[4, 5]]as BigNumbers. Thanks for the pointer tomatAlgo14xDs.When a matrix has a declared datatype, the dense scalar path converts the scalar into that datatype first.
BigNumber -> numberis lossy so it throws, instead of promoting like mathjs normally would. Fix takes that fast path only when the scalar actually converts, otherwise it falls back to the generic per-element callback (same path anumbermatrix with no declared datatype already uses). Result is[[bignumber(4), bignumber(5)]], datatype dropped to undefined.One call for you: I kept this to the reported dense case. The sparse path (
matAlgo11xS0s) and the other matrix-scalar algos have the same force-conversion and throw the same way (checked sparse). Happy to fix them all here if you'd rather one sweep, but left it contained given your note that there are probably more spots. Some overlap with #3607 too, which rewrites this file for other reasons.closes #3612