Skip to content

Add General Decimal Arithmetic ordering, adjacency, and exponent operations - #150

Open
Btocode wants to merge 2 commits into
cockroachdb:masterfrom
Btocode:gda-ordering-ops
Open

Btocode wants to merge 2 commits into
cockroachdb:masterfrom
Btocode:gda-ordering-ops

Conversation

@Btocode

@Btocode Btocode commented Jul 22, 2026 •

Copy link
Copy Markdown

Adds the following General Decimal Arithmetic (GDA) / IEEE 754 decimal operations that apd was missing:

  • Context.Max, Min, MaxAbs, MinAbs: numeric and by-magnitude minimum and maximum, with the specification's NaN and total-order tie-break rules.
  • Context.NextPlus, NextMinus, NextToward: the representable value adjacent to an operand in a given direction.
  • Context.LogB, ScaleB: the adjusted exponent, and scaling by a power of ten.
  • Decimal.CopySign, Decimal.SameQuantum and Decimal.CmpTotalMag: context-free companions to Neg, Abs and CmpTotal.

It also fixes two existing setExponent behaviours these operations depend on, which affect every operation that rounds:

  • An overflowing result is clamped to Nmax, not Infinity, under round-down, round-05up, floor (for positive values) and ceiling (for negative values), as the specification requires. Overflow now also raises Rounded.
  • Subnormal rounding uses the value's sign, so floor and ceiling round negative subnormals in the right direction.

Implementation notes worth a reviewer's attention:

  • Max/Min treat a quiet NaN operand as absent (return the other operand); only a signaling NaN raises InvalidOperation. Ties (numerically equal operands) are broken by total order, so e.g. Max(1.0, 1.00) returns 1.0.
  • The next-* operations step the coefficient directly on the grid of multiples of 10^q near the operand, which is O(precision) and independent of the exponent range. They require a non-zero precision. A result whose exponent is beyond the system limits (e.g. the smallest subnormal of BaseContext.WithPrecision(9)) returns the usual exponent out of range error.
  • ScaleB rounds through setExponent, so a result beyond the context's range overflows or underflows. One beyond the system limits returns an exponent out of range error, as Mul does.

Testing:

  • The official GDA .decTest vectors for all twelve operations run through gda_test.go. That includes the result-aliases-operand checks, and finite results must match the exponent exactly for the operations that select or adjust an operand. The 18 comparetotmag NaN-payload vectors are ignored, as for comparetotal, because apd does not keep NaN payloads.
  • Table-driven tests cover signed zeros, subnormals near Etiny, values near Nmax, infinities, quiet/signaling NaNs, aliasing, wide-exponent and BaseContext.WithPrecision contexts, and directed-rounding overflow. TestOrderingConditions checks every flag.
  • A randomized cross-check against Python's decimal (230k cases, 11 contexts, all 8 rounding modes) gives no result mismatches. The only flag difference is the existing extra Rounded on clamped zeros, which this PR does not change.

…ations

Implement the following GDA / IEEE 754 decimal operations that were
missing from apd:

  - Context.Max, Min, MaxAbs, MinAbs: numeric and by-magnitude minimum
    and maximum, with the specification's NaN and total-order tie-break
    rules.
  - Context.NextPlus, NextMinus, NextToward: the representable value
    adjacent to an operand in a given direction.
  - Context.LogB, ScaleB: the adjusted exponent, and scaling by a power
    of ten.
  - Context.CopySign: the magnitude of one operand with the sign of
    another.
  - Decimal.SameQuantum and Decimal.CmpTotalMag: companions to the
    existing CmpTotal.

The operations follow the General Decimal Arithmetic specification,
including the condition flags they raise. New table-driven tests cover
signed zeros, subnormals near Etiny, values near Nmax, infinities, and
quiet and signaling NaNs; the expected values were cross-checked against
the Python standard library's decimal module.

Signed-off-by: S Afsan Rahmatullah <afsan.scorp@gmail.com>

@mw5h mw5h left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did an adversarial review of this at adaaa00, with the official GDA .decTest vectors for all twelve operations (strict: Form, sign, coefficient, exponent, every flag, plus d==x and d==y aliasing), a 66k-case randomized cross-check against Python's decimal across 11 contexts and all 8 rounding modes, and hand-picked edge cases. Trial merge onto current master is clean and go test ./... passes.

What checks out. Non-aliased official vectors: max 222/222, min 211/211, maxmag 207/207, minmag 197/197, nextplus 103/103, nextminus 101/101, nexttoward 316/316 results, logb 121/121, copysign 57/57, samequantum 333/333; scaleb 141 pass with 8 flag-only misses and comparetotmag 246 pass with 18 NaN-payload misses, both pre-existing apd limitations (see below). Randomized Python cross-check: 0 mismatches for next_plus/next_minus/next_toward/logb/copy_sign/same_quantum over ~33k cases. Quiet-NaN-as-absent, sNaN handling, tie-breaks (±0, 1.0 vs 1.00, negatives), result rounding (prec 3: Max(1.23456, 1) = 1.23 inexact, rounded), nextplus(Nmax) = Inf, nextminus(Inf) = Nmax, nextplus(0) = 1E-Etiny, LogB/ScaleB special cases, the ScaleB bound (= Python's 2*(Emax+prec)), and traps via goError all behave as the spec says. That's a solid base. But two things are blocking.

1. Blocking: NextPlus/NextMinus/NextToward silently return garbage for BaseContext.WithPrecision(n)

nextUpPos/nextDownPos do nc.Add(d, x, &tiny) / nc.Sub(d, x, &tiny) and discard the returned (Condition, error). tiny = 1E(MinExponent-Precision); when that exponent is below the system limit of -100000 (true for every BaseContext.WithPrecision(n) context, which is the idiom in this repo's own example_test.go), or when the exponent gap exceeds 100000, upscale fails before d is written, so d is left untouched with res = 0, err = nil:

BaseContext.WithPrecision(9):  NextPlus(1)      = 0E0           want 1.00000001
                               NextToward(1, 2) = 0E0
                               NextMinus(1)     = 0E0
                               NextMinus(0)     = -1E-100008    (below the system MinExponent)
Emax/Emin ±60000, d pre-filled with junk: NextPlus(1E+59000) returns d unchanged, err = nil

Fix: implement the increment on the coefficient directly (round directed; pad to Precision digits or to Etiny; ±1; carry → exponent+1 / Nmax / Infinity; borrow → 99…9). That is O(precision), cannot fail, and also removes the add-tiny cost (near big exponents the current approach builds ~80k-digit BigInts, ~2ms per call vs ~1µs). At minimum, propagate the Add/Sub errors and reject contexts where etiny()-1 < MinExponent.

2. Blocking (one-line fix): CopySign and NextToward lose the sign when d aliases y

Both do d.Abs(x); d.Negative = y.Negative, which reads y after overwriting it when d == y. That violates the Context doc ("Arguments for any method can safely be used as both result and operand"), and 54 copysign plus 19 nexttoward official vectors fail in the d==y configuration:

d := -1; CopySign(d, 5, d)    →  5            (cpsx004: want -5)
d := -7; NextToward(d, 7, d)  →  6.99999999   (nextt102: want -7; sign lost, then Cmp≠0 so the next-minus path runs)
nextt551: got Infinity, want -Infinity

Fix: neg := y.Negative; d.Abs(x); d.Negative = neg.

3. Should fix

  • Precision 0 is not rejected. setNmax yields coefficient 10^0-1 = 0 with exponent MaxExponent+1, so with &BaseContext: NextMinus(Infinity) = 0E100001, NextPlus(-Infinity) = -0E100001, NextPlus(1) is a 100,039-character number, NextPlus(0) = 1E-100000. Quo/Exp return errZeroPrecisionStr; these should too.
  • ScaleB range handling. if newExp > MaxExponent || newExp < MinExponent → NaN + InvalidOperation. With prec 9, Emax/Emin ±99000: ScaleB(1E+99000, 1500) → NaN invalid operation, although 1500 is well within the spec bound (198018). Spec/Python: Infinity Overflow Inexact Rounded. apd convention (Mul(1E+99000, 1E+1500)): overflow plus an "exponent out of range" error. Routing through d.setExponent(...) + c.round would make SystemOverflow flow through goError like everywhere else.
  • The flag-conformance claim doesn't hold under directed rounding. Max/Min/MaxAbs/MinAbs/ScaleB inherit two pre-existing setExponent bugs: overflow always goes to Infinity (spec: round-down, floor-for-positive, ceiling-for-negative go to Nmax), and subnormal rounding ignores the sign (e.g. prec 3, floor: Min(-1E-102, NaN) → apd -0E-101, Python -1E-101). That's 773 result mismatches vs Python in the sweep, all in these ops. The PR clearly knows about both (the nextPlus comment and the Nmax clamp in nextDownPos work around them) but only inside next-. The durable fix is in setExponent (which benefits Add/Mul/etc. and makes the next- workarounds unnecessary); otherwise the description's "follow the GDA specification, including the condition flags" should be narrowed.
  • Wire the official vectors. .decTest files exist for all twelve operations (3,188 vectors) and the repo convention is to run them via gda_test.go; the harness aliases d to each operand and would have caught item 2. TestOrderingConditions masks to InvalidOperation|DivisionByZero, so Overflow/Underflow/Subnormal/Inexact/Rounded/Clamped are never asserted. Note the stock harness compares finite results with Cmp, which is exponent-insensitive and too weak for max/min/copysign; and the 18 ctmx NaN-payload vectors need GDAignore entries since apd drops payloads.

4. Smaller

  • API shape: SameQuantum and CmpTotalMag are context-free Decimal methods, but the equally context-free CopySign is a Context method that always returns (0, nil). func (d *Decimal) CopySign(x, y *Decimal) *Decimal next to Neg/Abs would be more consistent, and this is hard to change after release.
  • Pre-existing and not caused here, for the record: overflow results lack Rounded and clamped zeros gain a spurious Rounded (the 8 scaleb flag misses); CmpTotalMag cannot order NaN payloads because setString discards them (the 18 comparetotmag misses; worth a doc note). CmpTotalMag also allocates two coefficient copies where a sign-masked cmpOrder would do.

…DA vectors

- NextPlus/NextMinus/NextToward now step the coefficient directly instead of
  adding a value below Etiny. This is O(precision), works for contexts such as
  BaseContext.WithPrecision(n) and large exponent ranges, and reports the
  system exponent limits through goError instead of leaving d untouched.
  Contexts with zero precision are rejected with errZeroPrecisionStr.
- CopySign is now a Decimal method, alongside Neg and Abs. It and NextToward
  read y's sign before writing d, so d may alias y.
- setExponent clamps overflow to Nmax under round-down, round-05up, floor
  (positive) and ceiling (negative), marks overflow as Rounded, and rounds
  subnormals using the value's sign. This fixes Max/Min/MaxAbs/MinAbs/ScaleB
  under directed rounding and removes the Nmax workaround from next-*.
- ScaleB rounds through setExponent: results beyond the context overflow or
  underflow as the specification requires, and results beyond the system
  limits return the usual exponent out of range error.
- CmpTotalMag compares magnitudes without copying its operands, and documents
  that NaN payloads are not ordered.
- Run the official GDA vectors for all twelve operations through gda_test.go,
  comparing exponents as well as values for the operations that select or
  adjust an operand. The NaN payload cases of comparetotmag are ignored, as
  for comparetotal. TestOrderingConditions now checks every flag.
@Btocode

Btocode commented Oct 27, 2026

Copy link
Copy Markdown
Author

Thanks for such a thorough review. The test harness alone caught more than my own testing did. All of it is addressed in 1d9d7ff:

1. next-* with BaseContext.WithPrecision(n). I took your suggestion and the operations now step the coefficient directly. They work out the spacing between representable numbers at the operand (10^q, with q = max(adj − prec + 1, Etiny)), take floor(|x| / 10^q), and add or subtract one. Going past 10^p − 1 bumps the exponent, and going below 10^(p−1) when |x| is an exact power of ten gives p nines one exponent down. This can't fail, takes O(precision) time, and no longer needs the Nmax workaround. BaseContext.WithPrecision(9) now gives NextPlus(1) = 1.00000001. With Emax/Emin ±60000, NextPlus(1E+59000) works and every field of d is set. A result below the system exponent limit (e.g. NextMinus(0) in BaseContext.WithPrecision(9), whose smallest subnormal is 1E-100008) returns the usual exponent out of range error instead of leaving d untouched.

2. Aliasing. CopySign and the equal-operands branch of NextToward read y.Negative before writing d. The GDA harness now covers both d == x and d == y.

3. Should fix

  • Precision 0 returns errZeroPrecisionStr for all three next-* operations.
  • ScaleB sets the exponent and rounds, so ScaleB(1E+99000, 1500) with Emax 99000 behaves like your Mul example: overflow plus an exponent out of range error. Results within the system limits overflow or underflow per the spec.
  • I fixed both problems in setExponent rather than narrowing the description. Overflow now clamps to Nmax under down, 05up, floor (positive) and ceiling (negative), and subnormal rounding passes the value's sign to ShouldAddOne. Overflow also raises Rounded now. The existing GDA suite passes unchanged.
  • The .decTest files for all twelve operations are wired into gda_test.go. Finite results are compared with CmpTotal, so the exponent has to match, for max/min/maxmag/minmag/next*/copysign/scaleb. The 18 ctmx NaN-payload vectors are in GDAignore. TestOrderingConditions no longer masks flags and has overflow, underflow, subnormal and clamped cases.

4. Smaller

  • CopySign is now func (d *Decimal) CopySign(x, y *Decimal) *Decimal.
  • CmpTotalMag compares by form and then by magnitude through a cmpAbs helper factored out of Cmp, so it doesn't copy either operand. It's documented that NaN payloads aren't ordered.
  • I left the extra Rounded on clamped zeros alone, since it's outside this PR.

I also reran a randomized comparison against Python's decimal: 230k cases over 11 contexts and all 8 rounding modes. No results differ. The only flag difference is that clamped-zero Rounded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants