Skip to content

Reject record UDT pairs whose leaf counts differ - #591

Open
eriknw wants to merge 2 commits into
10-udt-udf-shape-checksfrom
11-udt-record-leaf-count
Open

eriknw wants to merge 2 commits into
10-udt-udf-shape-checksfrom
11-udt-record-leaf-count

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026

Copy link
Copy Markdown
Member

_check_udt_pair rejects two record operands that disagree on shape, but it
compared only top-level field names. Two records can share those and still
nest differently: [("a", f8), ("b", f8)] against
[("a", [("n1", f8), ("n2", f8)]), ("b", f8)] both report fields
["a", "b"] while contributing two leaves and three. The codegen walks one
operand's leaf paths and applies them to both, so applying the scalar path
["a"] to the nested side asks Numba to add a float to a record.

Measured before this change, that pair gets through the checks and comes
back from Numba's typing pass as:

UdfParseError: binary.plus does not work with (_NestFlat, _NestDeep):
No implementation of function Function(<built-in function add>) found
for signature: >>> add(float64, Record(nst_n1[type=float64;offset=0],
nst_n2[type=float64;offset=8];16;False))

That reports a compile failure for what is the same shape disagreement the
three sibling checks in the same function already report as a KeyError.
Comparing leaf counts makes it the fourth of those checks.

The exception class changes, and that is visible in both directions:
UdfParseError derives from GraphblasException, not from KeyError, so code
catching KeyError to mean "no such op" now catches this pair, and code
catching UdfParseError no longer does. OpBase.__contains__ catches both,
so udt in binary.plus is unaffected either way.


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 11-udt-record-leaf-count branch from 7d55403 to d4c4481 Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from d4c4481 to 0bd9607 Compare August 5, 2026 00:06
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch 2 times, most recently from 479c26c to 563b80a Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 563b80a to 123b148 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch 2 times, most recently from 683dcc9 to ec062f1 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from ec062f1 to b8e797f Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from b8e797f to 17bc219 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 17bc219 to da41e30 Compare August 6, 2026 20:36
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from da41e30 to 15b9955 Compare August 6, 2026 20:42
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 15b9955 to 02464a8 Compare August 7, 2026 02:48
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 02464a8 to 2f6fb4d Compare August 7, 2026 05:09
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 2f6fb4d to cdefa0b Compare August 26, 2026 17:30
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from cdefa0b to dc5dbaf Compare September 18, 2026 03:09
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from dc5dbaf to 3b4489d Compare September 18, 2026 03:46
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 3b4489d to 34be774 Compare September 18, 2026 17:34
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 34be774 to ea2bdad Compare September 18, 2026 17:39
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from ea2bdad to 66d8f62 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 11-udt-record-leaf-count branch from 66d8f62 to 632aa2f 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 11-udt-record-leaf-count branch from 632aa2f to bc6aa1f Compare September 24, 2026 20:06
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch 3 times, most recently from 36a4ec5 to e202e59 Compare September 25, 2026 06:10
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from e202e59 to 4280105 Compare September 26, 2026 05:23
`_check_udt_pair` rejects two record operands that disagree on shape, but it
compared only top-level field names. Two records can share those and still
nest differently: `[("a", f8), ("b", f8)]` against
`[("a", [("n1", f8), ("n2", f8)]), ("b", f8)]` both report fields
`["a", "b"]` while contributing two leaves and three. The codegen walks one
operand's leaf paths and applies them to both, so applying the scalar path
`["a"]` to the nested side asks Numba to add a float to a record.

Measured before this change, that pair gets through the checks and comes
back from Numba's typing pass as:

    UdfParseError: binary.plus does not work with (_NestFlat, _NestDeep):
    No implementation of function Function(<built-in function add>) found
    for signature: >>> add(float64, Record(nst_n1[type=float64;offset=0],
    nst_n2[type=float64;offset=8];16;False))

That reports a compile failure for what is the same shape disagreement the
three sibling checks in the same function already report as a KeyError.
Comparing leaf counts makes it the fourth of those checks.

The exception class changes, and that is visible in both directions:
UdfParseError derives from GraphblasException, not from KeyError, so code
catching KeyError to mean "no such op" now catches this pair, and code
catching UdfParseError no longer does. `OpBase.__contains__` catches both,
so `udt in binary.plus` is unaffected either way.
@eriknw
eriknw force-pushed the 11-udt-record-leaf-count branch from 4280105 to 8a37275 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