You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The current definition of lcm coerces arguments to Int. This generates quite unexpected behavior when passed Rat which can have a defined LCM.
Suggestion
Maintain current behavior if both arguments can be coerced to Int.
If both arguments can be coerced to Rat, do the following:
-> Rat \a, Rat \b {
(a.numerator lcm b.numerator) / (a.denominator gcd b.denominator)
}
Failing all else, fail. Lossy coercion will result in unexpected values. It is better to fail with π lcm e now (and support it later, if symbolic math were added to core) then to produce 6 (when it should be simple πe, or 8.53…). Users can always manually coerce if they want unexpected behavior.
Potential issues
Given 1.5 and 4.5, the above algorithm will generate 4.5 (9/2). Some users may want a whole LCM, but that is more trivially generated from the rational (just the numerator) than the opposite, I think.
Complex numbers can be calculated too, but if we fail now, support can be added later without potentially breaking code.
The text was updated successfully, but these errors were encountered:
The current definition of
lcmcoerces arguments to Int. This generates quite unexpected behavior when passedRatwhich can have a defined LCM.Suggestion
Potential issues
The text was updated successfully, but these errors were encountered: