Conversation
eriknw
marked this pull request as ready for review
August 4, 2026 16:07
eriknw
force-pushed
the
15-udt-division-semantics
branch
2 times, most recently
from
August 5, 2026 00:06
0c83611 to
76b468c
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 5, 2026 03:18
76b468c to
71acc5a
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 5, 2026 17:44
71acc5a to
8c89392
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 5, 2026 18:03
8c89392 to
7b2c580
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 5, 2026 18:05
7b2c580 to
d2791ee
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 6, 2026 07:59
d2791ee to
c2c0836
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 6, 2026 15:39
c2c0836 to
358571c
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 6, 2026 15:41
358571c to
bdfb7e6
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
2 times, most recently
from
August 6, 2026 20:41
075c702 to
99c9e2b
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 7, 2026 02:48
99c9e2b to
c2748c1
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 7, 2026 05:09
c2748c1 to
e5fae30
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
August 26, 2026 17:30
e5fae30 to
4083935
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 18, 2026 03:09
4083935 to
128e43e
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 18, 2026 03:46
128e43e to
769e327
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 18, 2026 17:34
769e327 to
0545183
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 18, 2026 17:39
0545183 to
51a2173
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 23, 2026 16:22
51a2173 to
3f19306
Compare
eriknw
removed this pull request from stack #627
September 23, 2026 18:18
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 23, 2026 18:19
3f19306 to
61f63ce
Compare
eriknw
added this pull request to stack #634
September 23, 2026 18:19
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 24, 2026 20:06
61f63ce to
2e1311b
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 24, 2026 20:07
2e1311b to
9073ae6
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
from
September 25, 2026 03:33
9073ae6 to
ae9e1a9
Compare
eriknw
force-pushed
the
15-udt-division-semantics
branch
2 times, most recently
from
September 26, 2026 05:23
47cd893 to
43e9b47
Compare
`_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
force-pushed
the
15-udt-division-semantics
branch
from
September 26, 2026 05:25
43e9b47 to
58738b5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_C_INFIX_OPSmapped both Python/and//onto C/, so the JIT kernelfor an integer UDT field emitted bare signed division and remainder. Measured
at the parent commit, for a record of int32 and int64 fields:
That is two separate defects.
First, a trap. Bare signed
/and%are undefined for a zero divisor andfor
INT_MIN / -1. On x86-64idivraises #DE, which is SIGFPE and processdeath rather than an exception. AArch64's
sdivreturns 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
/ontwo integers is integer division, but Python's
/divides in floating point,so the two paths disagreed for the same program:
Float
//was wrong on its own account: the kernel computedfloor(a / b),which is not floor division.
1.0 // 0.1is 9.0 butfloor(1.0 / 0.1)is10.0, and
inf // 2.0is NaN butfloor(inf / 2.0)is inf. Both paths nowuse 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:
x / 0gives 0, which is whatnp.floor_dividedoes.np.true_dividegives an infinity whose cast back to an integer isundefined in numpy too.
INT_MIN // -1wraps toINT_MIN, as numpy does. Numba returns 0 for it,deliberately, to dodge the same trap.
/by zero gives numpy's infinities. Numba raisesZeroDivisionErrorunconditionally, outside the error model's control, sothe 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)conversionitself, since
(2**63 - 1) / 1rounds up to2**63; both paths then land onthe same hardware conversion rather than on defined behaviour, so they agree
with each other but need not agree across machines (measured saturating to
INT64_MAXon arm64). Operands of mixed signedness escape through a negativedivisor the
INT_MINguard does not see; the_expr_binarydocstring detailswhy that also cannot split the paths.
Stack created with GitHub Stacks CLI • Give Feedback 💬