Skip to content

The numeric % operators disagree with each other, and two of them are wrong #708

Description

@Rafael-SOWNet

The three numeric types answer % three different ways, and two of the answers are wrong. Measured on f81d7aca, .NET 10:

7 % 3 -7 % 3 7 % -3 -7 % -3
Integer % Integer 1 2 throws throws
Real % Real 1 -1 1 -1
Rational % Rational (7/2 % 3 etc.) 1/2 5/2 1/2 -7/2

Three behaviours for one operator:

  • Integer is floored, but EInteger.Mod refuses a negative divisor, so Integer.Create(7) % Integer.Create(-3) throws ArithmeticException: Divisor is negative. A public operator that throws on an ordinary input.
  • Real is truncated — it takes the sign of the dividend.
  • Rational is floored for a positive divisor and simply wrong for a negative one. (-7/2) % (-3) comes back as -7/2, which is not the remainder under any convention: it is not even smaller in magnitude than the divisor. The correct answer is -1/2 floored, -1/2 truncated. The code says as much itself:
// TODO: consider the case for the divisor to be negative
public static Rational operator %(Rational a, Rational b)
    => a.ERational.Remainder(b.ERational)
        .Alias(out var mod)
        .IsNegative switch
        {
            false => mod,
            true => mod + b,
        };

Adding b when the remainder is negative is the truncated-to-floored conversion, but it is only correct when b is positive; for negative b it makes the result further from zero.

Since Integer and Rational are both Real, which one you get depends on the static type at the call site rather than on the values, so the same numbers can give different answers.

What it should be

All three floored — the remainder takes the sign of the divisor, a - b*floor(a/b):

7 % 3 -7 % 3 7 % -3 -7 % -3
all three 1 2 -2 -1

This is the convention SymPy, Mathematica and Maxima use, and the one under which the residues modulo n are the numbers from 0 to n-1. It also matches what the library already does by hand where it needs a remainder it can rely on — ExpressionNumerical.Equality.cs carries a private TrueRemainder that converts C#'s truncation into exactly this.

Integer and Rational are strictly bug fixes: for Integer only the throwing cases change, and for Rational only the case that is currently wrong. Real is a behaviour change — -7 % 3 would go from -1 to 2 — which is why this is an issue rather than a quiet fix.

Found while writing #703, which needs a remainder and computes the floored one itself rather than going through these. Happy to do the fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions