Skip to content

Define UDT division on both execution paths - #595

Open
eriknw wants to merge 5 commits into
14-udt-minmax-nanfrom
15-udt-division-semantics
Open

eriknw wants to merge 5 commits into
14-udt-minmax-nanfrom
15-udt-division-semantics

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

_C_INFIX_OPS mapped both Python / and // onto C /, so the JIT kernel
for an integer UDT field emitted bare signed division and remainder. Measured
at the parent commit, for a record of int32 and int64 fields:

floordiv:  z->q_a = ((x->q_a) / (y->q_a) - (((x->q_a) % (y->q_a) != 0) && ...))
truediv:   z->q_a = (x->q_a) / (y->q_a)

That is two separate defects.

First, a trap. Bare signed / and % are undefined for a zero divisor and
for INT_MIN / -1. On x86-64 idiv raises #DE, which is SIGFPE and process
death rather than an exception. AArch64's sdiv returns 0 and does not trap,
so this cannot be exhibited on the arm64 machine it was written on. It is a
real defect in the generated C either way.

Second, a wrong answer, and this one is not architecture-specific. C / on
two integers is integer division, but Python's / divides in floating point,
so the two paths disagreed for the same program:

binary.truediv on an int64 UDT, 10**18 / 3
  with a C compiler:    333333333333333333
  without one:          333333333333333312   (numpy's answer)

Float // was wrong on its own account: the kernel computed floor(a / b),
which is not floor division. 1.0 // 0.1 is 9.0 but floor(1.0 / 0.1) is
10.0, and inf // 2.0 is NaN but floor(inf / 2.0) is inf. Both paths now
use the remainder-based algorithm numpy and CPython share.

Three of the values here are choices rather than discoveries, and each
follows numpy rather than the alternative:

  • Integer x / 0 gives 0, which is what np.floor_divide does.
    np.true_divide gives an infinity whose cast back to an integer is
    undefined in numpy too.
  • INT_MIN // -1 wraps to INT_MIN, as numpy does. Numba returns 0 for it,
    deliberately, to dodge the same trap.
  • Complex / by zero gives numpy's infinities. Numba raises
    ZeroDivisionError unconditionally, outside the error model's control, so
    the cfunc previously left the element unwritten.

Two range escapes are left, and both move the paths together rather than
apart. A 64-bit field can leave the range through the (double) conversion
itself, since (2**63 - 1) / 1 rounds up to 2**63; both paths then land on
the same hardware conversion rather than on defined behaviour, so they agree
with each other but need not agree across machines (measured saturating to
INT64_MAX on arm64). Operands of mixed signedness escape through a negative
divisor the INT_MIN guard does not see; the _expr_binary docstring details
why that also cannot split the paths.


Stack created with GitHub Stacks CLI • Give Feedback 💬

@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch 2 times, most recently from 0c83611 to 76b468c Compare August 5, 2026 00:06
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 76b468c to 71acc5a Compare August 5, 2026 03:18
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 71acc5a to 8c89392 Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 8c89392 to 7b2c580 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 7b2c580 to d2791ee Compare August 5, 2026 18:05
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from d2791ee to c2c0836 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from c2c0836 to 358571c Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 358571c to bdfb7e6 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch 2 times, most recently from 075c702 to 99c9e2b Compare August 6, 2026 20:41
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 99c9e2b to c2748c1 Compare August 7, 2026 02:48
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from c2748c1 to e5fae30 Compare August 7, 2026 05:09
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from e5fae30 to 4083935 Compare August 26, 2026 17:30
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 4083935 to 128e43e Compare September 18, 2026 03:09
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 128e43e to 769e327 Compare September 18, 2026 03:46
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 769e327 to 0545183 Compare September 18, 2026 17:34
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 0545183 to 51a2173 Compare September 18, 2026 17:39
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 51a2173 to 3f19306 Compare September 23, 2026 16:22
@eriknw
eriknw removed this pull request from stack #627 September 23, 2026 18:18
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 3f19306 to 61f63ce Compare September 23, 2026 18:19
@eriknw
eriknw added this pull request to stack #634 September 23, 2026 18:19
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 61f63ce to 2e1311b Compare September 24, 2026 20:06
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 2e1311b to 9073ae6 Compare September 24, 2026 20:07
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 9073ae6 to ae9e1a9 Compare September 25, 2026 03:33
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch 2 times, most recently from 47cd893 to 43e9b47 Compare September 26, 2026 05:23
`_C_INFIX_OPS` mapped both Python `/` and `//` onto C `/`, so the JIT kernel
for an integer UDT field emitted bare signed division and remainder. Measured
at the parent commit, for a record of int32 and int64 fields:

    floordiv:  z->q_a = ((x->q_a) / (y->q_a) - (((x->q_a) % (y->q_a) != 0) && ...))
    truediv:   z->q_a = (x->q_a) / (y->q_a)

That is two separate defects.

First, a trap. Bare signed `/` and `%` are undefined for a zero divisor and
for `INT_MIN / -1`. On x86-64 `idiv` raises #DE, which is SIGFPE and process
death rather than an exception. AArch64's `sdiv` returns 0 and does not trap,
so this cannot be exhibited on the arm64 machine it was written on. It is a
real defect in the generated C either way.

Second, a wrong answer, and this one is not architecture-specific. C `/` on
two integers is integer division, but Python's `/` divides in floating point,
so the two paths disagreed for the same program:

    binary.truediv on an int64 UDT, 10**18 / 3
      with a C compiler:    333333333333333333
      without one:          333333333333333312   (numpy's answer)

Float `//` was wrong on its own account: the kernel computed `floor(a / b)`,
which is not floor division. `1.0 // 0.1` is 9.0 but `floor(1.0 / 0.1)` is
10.0, and `inf // 2.0` is NaN but `floor(inf / 2.0)` is inf. Both paths now
use the remainder-based algorithm numpy and CPython share.

Three of the values here are choices rather than discoveries, and each
follows numpy rather than the alternative:

- Integer `x / 0` gives 0, which is what `np.floor_divide` does.
  `np.true_divide` gives an infinity whose cast back to an integer is
  undefined in numpy too.
- `INT_MIN // -1` wraps to `INT_MIN`, as numpy does. Numba returns 0 for it,
  deliberately, to dodge the same trap.
- Complex `/` by zero gives numpy's infinities. Numba raises
  `ZeroDivisionError` unconditionally, outside the error model's control, so
  the cfunc previously left the element unwritten.

Two range escapes are left, and both move the paths together rather than
apart. A 64-bit field can leave the range through the `(double)` conversion
itself, since `(2**63 - 1) / 1` rounds up to `2**63`; both paths then land on
the same hardware conversion rather than on defined behaviour, so they agree
with each other but need not agree across machines (measured saturating to
`INT64_MAX` on arm64). Operands of mixed signedness escape through a negative
divisor the `INT_MIN` guard does not see; the `_expr_binary` docstring details
why that also cannot split the paths.
The JIT C for ``truediv`` on a complex field spelled the zero-divisor case
with ``CMPLX``/``CMPLXF``. macOS does not define those C11 macros, so
whether the kernel compiled came down to which compiler SuiteSparse's JIT
happened to be configured with: clang accepted it, gcc rejected it outright
with "implicit declaration of function 'CMPLX'" and the whole op fell back.

That is not a hypothetical. conda-forge's graphblas switched the compiler it
bakes into the JIT config from clang to gcc on osx-arm64 between 10.4.0 and
10.5.0, which took ``test_udt_complex_truediv_by_zero[jit]`` down with it.

``GB_complex.h`` is already on the JIT's include path and already handles
this, in a comment naming the same case: "gcc on the Mac does not define the
CMPLX and CMPLXF macros". Its ``GB_CMPLX32``/``GB_CMPLX64`` fall back to
building the value through a two-element array rather than by arithmetic, so
they stay Inf/NaN-safe and the reasoning the docstring gives for not using
``I`` is unchanged.
@eriknw
eriknw force-pushed the 15-udt-division-semantics branch from 43e9b47 to 58738b5 Compare September 26, 2026 05:25
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.

1 participant