add invmod (modular multiplicative inverse)

[thetazero]

Added invmod (modular multiplicative inverse)

Based off #1744
Test Cases based of the ones for xgcd, plus the addition of dealing with the non existence of a multiplicative inverse, and a zero modulus.
In this implementation it returns NaN if there is no multiplicative inverse, rather than throwing an error.
I think this is reasonable, but if you have another idea for how to handle that case let me know.

[josdejong]

Thanks @thetazero, looks good and well tested. Two feedbacks:

1. I made one inline comment
2. Can you also add an entry with documentation in https://github.com/josdejong/mathjs/blob/develop/src/expression/embeddedDocs/embeddedDocs.js, so math.help(math.invmod) will work?

[josdejong]

The check config.matrix === 'Matrix' is implicit, can you change this to either use isMatrix(x) from /src/utils/is.js and use .toArray() or .valueOf() on the Matrix instead of using it's private ._data property? Or simply call res.valueOf() which will return Array for every case (when res is Array, DenseMatrix, or SparseMatrix).

[josdejong]

One more thought: I think it should be possible to merge the two implementations into one, using the generic functions like mod, isZero, isInteger, isEqual etc that mathjs already has. Then the implementation would automatically work for other numeric types too, like Fraction. What do you think?

[thetazero]

Really appreciate all of your feedback I think it all makes a lot of sense.
However unfortunately as it stand moding fractions with bignumbers is not supported (ie: math.mod(math.fraction(3), math.bignumber(1)) throws an error).
I believe the rest of your suggestions are implemented.
I think it makes the most sense to fix the issue with mod in a separate pull, thoughts?

[josdejong]

Thanks for your updates. Ok so making a generic solution requires more work on fraction/bignumber. Makes sense to address that separately.

I think all is ready, I only see that browser testing is failing on the NaN unit tests. Can you change these tests from assert.strictEqual(..., NaN) to something like assert(isNaN(...))? NaN is one of these JavaScript pitfalls (i.e. NaN === NaN returns false in JavaScript), so maybe best to just avoid it.

[thetazero]

I was confused about that browser test failing, thanks so much for figuring out what the issue was. I've updated the unit test to work now.
On a side note I believe NaN not being equal to NaN makes sense in our case because then invmod(3, 6)==invmod(4,8) evaluates to false rather than true, which I believe is proper behavior.

[josdejong]

Thanks, looks good. Yeah, NaN is a weird case but it sort of makes sense 😄 .

[josdejong]

Published now in v10.1.0, Thanks again!