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); + } + } +}