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

Why does the behavior of n mod 0 differ from the one in JavaScript? #2617

Closed
xxczaki opened this issue Jul 5, 2022 · 14 comments
Closed

Why does the behavior of n mod 0 differ from the one in JavaScript? #2617

xxczaki opened this issue Jul 5, 2022 · 14 comments
Labels

Comments

@xxczaki
Copy link

xxczaki commented Jul 5, 2022

// math.js using `evaluate`
22 % 0 // => 22

// javascript ran in node.js or browser devtools
22 % 0 //=> NaN

As far as I can tell, this wasn't raised in any of the issues yet. Hence, I am thinking that this might be the expected behavior. Most of the people I know would say that the second option should be the correct one, although I did some research and contrary opinions exist; https://math.stackexchange.com/questions/516251/why-is-n-mod-0-undefined/516270#516270.

@josdejong
Copy link
Owner

Thanks for bringing this up. I don't think there is a strong reason for it. It makes sense to have 22 % 0 return NaN indeed, since it is undefined. If people agree we could change this behavior and schedule that change for the first next breaking version 11.

For reference: https://en.wikipedia.org/wiki/Modulo_operation

@josdejong josdejong added this to the v11 milestone Jul 6, 2022
@josdejong josdejong mentioned this issue Jul 6, 2022
11 tasks
@KevBeltrao
Copy link

I'd love to have that assigned to me if possible

@josdejong
Copy link
Owner

Thanks for your offer @KevBeltrao , that would be very nice!

@gwhitney
Copy link
Collaborator

gwhitney commented Jul 16, 2022

I know that I have been away from the project for a few weeks, but I would like to voice a contrary opinion here. Mostly mathjs makes the choices that are familiar and common to mathematicians (after all, math is in the name). And that already leads to one big diference in % in particular: in mathjs -3 % 8 is 5, whereas in JavaScript it's -3. That's because JavaScript is more focused on the "division" point of view of mod, whereas mathjs adopts the mathematical point of view that it's more useful for mod to map everything to a single set of canonical representatives, so every integer mod 8 should end up as one of {0,1,..., 6, 7}, because every integer can be expressed as a+8*b for a having one of these values. From that point of view, it is entirely appropriate and correct for 22 % 0 to be 22, because indeed, 22 can be expressed as 22 + 0*b, and in fact, that's the only way that 22 can be expressed as something plus a multiple of 0. So I would advocate against this change, as it will move the mathjs mod function from being something that has an entirely consistent approach to the mod function -- the "mathematician's mod" for short, but finding the canonical element that differs by a multiple of the modulus, for a fuller explanation that justifies the value -- to an inconsistent hybrid -- the canonical representation approach for positive mods but the division-focused approach for mod zero. Introducing that inconsistency seems like a regression to me, not a move forward.

@gwhitney
Copy link
Collaborator

For reference: https://en.wikipedia.org/wiki/Modulo_operation

Sorry, I did not address this reference. It is silent on the the question of "mod 0". However, it points out a few major variants of of the mod function, and clearly for positive values of the modulus, mathjs is currently consistent with the "Euclidean division" version of mod. The Euclidean division version would prescribe 22 % 0 should be 22, as 22 = 0*0 + 22. Hence we should leave mathjs % alone (except actually mathjs should allow negative modulus values, as 15 % (-8) = 7 is perfectly well-defined, since 15 = (-8)*(-1) + 7. But that would be a separate issue.)

@josdejong
Copy link
Owner

Thanks for your input Glen. I learn something new every day :). Your explanation makes sense, and in mathjs I would indeed prefer the mathematical way rather than the programming-language way. What are your thoughs @xxczaki, @KevBeltrao ?

I did not address this reference. It is silent on the the question of "mod 0"

I was triggered by the text: "(a mod 1 is always 0; a mod 0 is undefined, ...)"

For reference, the current implementation of mod was introduced in v0.9.0, 2013-05-29, by my former self:

export function modNumber (x, y) {
if (y > 0) {
// We don't use JavaScript's % operator here as this doesn't work
// correctly for x < 0 and x === 0
// see https://en.wikipedia.org/wiki/Modulo_operation
return x - y * Math.floor(x / y)
} else if (y === 0) {
return x
} else { // y < 0
// TODO: implement mod for a negative divisor
throw new Error('Cannot calculate mod for a negative divisor')
}
}

@gwhitney
Copy link
Collaborator

I was triggered by the text: "(a mod 1 is always 0; a mod 0 is undefined, ...)"

Ah I missed that in the intro. But in the next sentence it includes a reference to the (mathematical) modular arithmetic article, which notes thar the integers are unchanged mod 0....

@josdejong
Copy link
Owner

We'll release mathjs v11 coming Friday, everything is ready for this except this open discussion about n mod 0.

if there are no further inputs, we'll keep the behavior as it currently is and close this issue coming Friday.

@KevBeltrao
Copy link

I'll agree with gwhitney! Thank you for sharing that

@xxczaki
Copy link
Author

xxczaki commented Jul 19, 2022

Thanks, @gwhitney for the comments, I agree that it does make sense to leave the existing behavior untouched to stay in parallel with the mathematical way of thinking. I do have a question, however; is there an easy way to override the behavior of this? We can override the mod function using a custom scope in evaluate, but I don't think it changes the behavior of %, like in 22 % 0, right?

@gwhitney
Copy link
Collaborator

gwhitney commented Jul 19, 2022

Wow, you're right; from the demo on mathjs.org

f(x,y) = (y==0) ? undefined : x%y; evaluate('mod(22,0)', {mod: f})\n evaluate('22 % 0', {mod: f})

yields [undefined, 22]. Hmmm, not exactly sure why that is, never really focused on this difference between a % b and mod(a,b) -- I thought the former was just syntactic sugar for the latter. @josdejong, is the difference in the ability to override the operation in the scope a bug or a feature?

@josdejong
Copy link
Owner

Ok thanks for your feedback guys. We'll keep the existing behavior of mod, I've removed this topic from the v11 milestone.

is there an easy way to override the behavior of this?

Yes, the % operator of the expression parser just calls the math.mod function, there is no difference between the two. So if you replace this function, you're good:

// default behavior
math.evaluate('22 % 0') // 22

// replace mod
// (the following function is for numbers only, no BigNumber support or anything)
function mod(x, y) {
  return x % y
}
math.import({ mod }, {override: true})

// use the new mod
math.evaluate('22 % 0') // NaN

I noticed though that this mathematical behavior for mod is only implemented for number, and not for BigNumber and Fraction. I opened a separate issue to address that: #2630

@josdejong
Copy link
Owner

Wow, you're right; from the demo on mathjs.org

f(x,y) = (y==0) ? undefined : x%y; evaluate('mod(22,0)', {mod: f})\n evaluate('22 % 0', {mod: f})

yields [undefined, 22]. Hmmm, not exactly sure why that is, never really focused on this difference between a % b and mod(a,b) -- I thought the latter was just syntactic sugar for the former. @josdejong, is the difference in the ability to override the operation in the scope a bug or a feature?

That is an interesting case. What happens right now is that functions and constants can be provided by the scope passed to math.evaluate, but this will not have effect on operators. I.e. passing mod or add or so to scope will not replace the operators % and + etc. It makes sense that they would. We can think that through in a separate issue.

@josdejong
Copy link
Owner

I'll close this issue now. The issue about operators not being overridden will be discussed in #2631.

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

No branches or pull requests

4 participants