From eb91edecc7bb1eab4d0b96a1adebc8f50dce2047 Mon Sep 17 00:00:00 2001 From: Rafael Vuijk Date: Tue, 4 Aug 2026 22:33:05 +0000 Subject: [PATCH] Give the three numeric types one remainder between them (#708) They answered three different ways, and two of the answers were wrong: Integer threw ArithmeticException on a negative divisor, because EInteger.Mod refuses one. A public operator, on ordinary input. Real truncated, so it took the sign of the dividend where the other two took the sign of the divisor. Rational added the divisor whenever the truncated remainder came out negative, which is the right conversion only for a positive divisor. For a negative one it moved the answer away from zero, so (-7/2) % (-3) came back as -7/2 -- larger in magnitude than the divisor, and a remainder under no convention at all. The code carried a TODO saying as much. Integer and Rational are both Real, so which of the three applied depended on the static type at the call site rather than on the values. All three are now floored: the remainder takes the sign of the divisor, and a - b*floor(a/b) is what every one of them computes. Checked against SymPy 1.14 on all four sign pairs, integer and rational alike. It is also what this library already did by hand where it needed a remainder it could rely on -- ExpressionNumerical.Equality carries a private TrueRemainder that converts C#'s truncation into exactly this. For Integer and Rational only the broken cases move. Real is a behaviour change: -7 % 3 was -1 and is 2. 12 of the 27 new tests fail without it, two of them property tests -- that the remainder is smaller than the divisor, and that the difference is a whole multiple of it, which between them are the definition. Suite 4572 passed, 0 failed, with no internal call site disturbed. --- .../Entity.Continuous.Integer.Definition.cs | 15 ++- .../Entity.Continuous.Rational.Definition.cs | 22 ++-- .../Entity.Continuous.Real.Definition.cs | 17 ++- .../UnitTests/Common/NumericModulusTest.cs | 108 ++++++++++++++++++ 4 files changed, 153 insertions(+), 9 deletions(-) create mode 100644 Sources/Tests/UnitTests/Common/NumericModulusTest.cs diff --git a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Integer.Definition.cs b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Integer.Definition.cs index af1c22271..bbd1795cd 100644 --- a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Integer.Definition.cs +++ b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Integer.Definition.cs @@ -131,7 +131,20 @@ internal static bool TryParse(string s, public static Integer operator -(Integer a, Integer b) => OpSub(a, b); public static Integer operator *(Integer a, Integer b) => OpMul(a, b); public static Real operator /(Integer a, Integer b) => (Real)OpDiv(a, b); - public static Integer operator %(Integer a, Integer b) => a.EInteger.Mod(b.EInteger); + /// + /// The floored remainder, which takes the sign of the divisor: -7 % 3 is 2 and + /// 7 % (-3) is -2. See https://github.com/asc-community/AngouriMath/issues/708. + /// + /// + /// Not EInteger.Mod, which refuses a negative divisor outright and so + /// made this operator throw on ordinary input. + /// + public static Integer operator %(Integer a, Integer b) + => a.EInteger.Remainder(b.EInteger) + .Alias(out var truncated) + .IsZero || truncated.Sign == b.EInteger.Sign + ? truncated + : truncated.Add(b.EInteger); public static Integer operator +(Integer a) => a; public static Integer operator -(Integer a) => OpMul(MinusOne, a); public static implicit operator Integer(sbyte value) => Create(value); diff --git a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Rational.Definition.cs b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Rational.Definition.cs index 37d3bfc70..92cd286db 100644 --- a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Rational.Definition.cs +++ b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Rational.Definition.cs @@ -174,15 +174,23 @@ internal static bool TryParse(string s, public static Rational operator +(Rational a) => a; public static Rational operator -(Rational a) => OpMul(Integer.MinusOne, a); - // TODO: consider the case for the divisor to be negative + /// + /// The floored remainder, which takes the sign of the divisor: -7/2 % 3 is 5/2 + /// and -7/2 % (-3) is -1/2. + /// See https://github.com/asc-community/AngouriMath/issues/708. + /// + /// + /// Adding the divisor whenever the truncated remainder came out negative is the + /// right conversion only where the divisor is positive; for a negative one it + /// moved the answer further from zero, so (-7/2) % (-3) came back as -7/2 -- + /// larger in magnitude than the divisor, and a remainder under no convention. + /// public static Rational operator %(Rational a, Rational b) => a.ERational.Remainder(b.ERational) - .Alias(out var mod) - .IsNegative switch - { - false => mod, - true => mod + b, - }; + .Alias(out var truncated) + .IsZero || truncated.IsNegative == b.ERational.IsNegative + ? truncated + : truncated + b; public static implicit operator Rational(sbyte value) => (long)value; public static implicit operator Rational(byte value) => (ulong)value; public static implicit operator Rational(short value) => (long)value; diff --git a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Real.Definition.cs b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Real.Definition.cs index afed4451d..d2b4c33ab 100644 --- a/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Real.Definition.cs +++ b/Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Real.Definition.cs @@ -122,7 +122,22 @@ internal static bool TryParse(string s, public static Real operator /(Real a, Real b) => OpDiv(a, b).Downcast(); public static Real operator +(Real a) => a; public static Real operator -(Real a) => OpMul(Integer.MinusOne, a); - public static Real operator %(Real a, Real b) => a.EDecimal.Remainder(b.EDecimal, MathS.Settings.DecimalPrecisionContext); + /// + /// The floored remainder, which takes the sign of the divisor: -7 % 3 is 2 and + /// 7 % (-3) is -2. See https://github.com/asc-community/AngouriMath/issues/708. + /// + /// + /// This one used to truncate and so took the sign of the dividend, disagreeing + /// with the same operator on and on + /// -- which one applied depended on the static type at + /// the call site rather than on the values. + /// + public static Real operator %(Real a, Real b) + => a.EDecimal.Remainder(b.EDecimal, MathS.Settings.DecimalPrecisionContext) + .Alias(out var truncated) + .IsZero || truncated.IsNegative == b.EDecimal.IsNegative + ? truncated + : truncated.Add(b.EDecimal, MathS.Settings.DecimalPrecisionContext); public static implicit operator Real(sbyte value) => (long)value; public static implicit operator Real(byte value) => (ulong)value; public static implicit operator Real(short value) => (long)value; diff --git a/Sources/Tests/UnitTests/Common/NumericModulusTest.cs b/Sources/Tests/UnitTests/Common/NumericModulusTest.cs new file mode 100644 index 000000000..6beaa0515 --- /dev/null +++ b/Sources/Tests/UnitTests/Common/NumericModulusTest.cs @@ -0,0 +1,108 @@ +// +// Copyright (c) 2019-2022 Angouri. +// AngouriMath is licensed under MIT. +// Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. +// Website: https://am.angouri.org. +// + +using AngouriMath; +using Xunit; +using static AngouriMath.Entity.Number; + +namespace AngouriMath.Tests.Common +{ + /// + /// The % operator over the numeric types. The three of them used to answer three + /// different ways -- threw outright on a negative divisor, + /// truncated, and was wrong under every + /// convention for a negative divisor -- so which answer you got depended on the static type + /// at the call site rather than on the values. + /// See https://github.com/asc-community/AngouriMath/issues/708. + /// + public sealed class NumericModulusTest + { + /// + /// Floored: the remainder takes the sign of the divisor. This is what SymPy, + /// Mathematica and Maxima answer, and the convention under which the residues modulo n + /// are the numbers from 0 to n - 1. + /// + [Theory] + [InlineData(7, 3, 1)] + [InlineData(-7, 3, 2)] + [InlineData(7, -3, -2)] + [InlineData(-7, -3, -1)] + [InlineData(6, 3, 0)] + [InlineData(-6, 3, 0)] + [InlineData(6, -3, 0)] + [InlineData(2, 5, 2)] + [InlineData(-2, 5, 3)] + public void TheThreeTypesAgree(int dividend, int divisor, int expected) + { + var a = Integer.Create(dividend); + var b = Integer.Create(divisor); + Assert.Equal(Integer.Create(expected), a % b); + Assert.Equal((Real)Integer.Create(expected), (Real)a % (Real)b); + } + + /// + /// A negative divisor used to raise ArithmeticException: Divisor is negative from + /// the arbitrary-precision layer, on an operator that is public and on ordinary input. + /// + [Fact] + public void ANegativeDivisorDoesNotThrow() => + Assert.Equal(Integer.Create(-2), Integer.Create(7) % Integer.Create(-3)); + + /// + /// The rational cases, against SymPy's answers for the same four sign pairs. The last + /// used to come back as -7/2 -- larger in magnitude than the divisor, so a remainder + /// under no convention at all. + /// + [Theory] + [InlineData(7, 2, 3, 1, 1, 2)] + [InlineData(-7, 2, 3, 1, 5, 2)] + [InlineData(7, 2, -3, 1, -5, 2)] + [InlineData(-7, 2, -3, 1, -1, 2)] + public void RationalsAgreeWithTheSameConvention( + int aNum, int aDen, int bNum, int bDen, int expectedNum, int expectedDen) => + Assert.Equal( + Rational.Create(expectedNum, expectedDen), + (Rational)Rational.Create(aNum, aDen) % (Rational)Rational.Create(bNum, bDen)); + + /// + /// The remainder is always strictly smaller in magnitude than the divisor. That is what + /// the Rational case was failing, and it holds whatever the signs. + /// + [Theory] + [InlineData(7, 3)] + [InlineData(-7, 3)] + [InlineData(7, -3)] + [InlineData(-7, -3)] + [InlineData(100, 7)] + [InlineData(-100, 7)] + [InlineData(1, 1000)] + [InlineData(-1, 1000)] + public void TheRemainderIsSmallerThanTheDivisor(int dividend, int divisor) + { + var remainder = Integer.Create(dividend) % Integer.Create(divisor); + Assert.True(remainder.Abs() < Integer.Create(divisor).Abs(), + $"{dividend} % {divisor} came out as {remainder}"); + } + + /// + /// And a % b is congruent to a modulo b, that is, a - (a % b) is a whole multiple of b. + /// Between them these two properties are the definition. + /// + [Theory] + [InlineData(7, 3)] + [InlineData(-7, 3)] + [InlineData(7, -3)] + [InlineData(-7, -3)] + [InlineData(-100, 7)] + public void TheDifferenceIsAWholeMultipleOfTheDivisor(int dividend, int divisor) + { + var a = Integer.Create(dividend); + var b = Integer.Create(divisor); + Assert.Equal(Integer.Create(0), (a - a % b) % b); + } + } +}