GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Let UDF registration name its return dtype with ret_dtype by eriknw · Pull Request #613 · python-graphblas/python-graphblas

Conversation

@eriknw

@eriknw eriknw commented Aug 4, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

A UDF's output type is inferred from what the function returns, matched
back to one of the input dtypes by base element type and rank. Inference
can only name a type the operator already has in hand, so an output UDT
that is not an operand is unreachable. A rank-reducing unary op such as
FP64[9] -> FP64[3] cannot be expressed at all: the inferred type is the
input's, and the shape check then rejects the shorter array the UDF
returns. The workaround of passing the desired type in as an extra
operand does not rescue that case either, since two float64 rank-1 UDTs
are indistinguishable to the matcher ("matches more than one input array
UDT").

Nothing structural was in the way. GrB_UnaryOp_new and friends take
ztype separately, and _finalize_udt_op already passes ret_type._carg
independently of the operand types. Only the inference step was
short-circuiting the choice.

Add a ret_dtype= keyword to register_anonymous and register_new on
UnaryOp, BinaryOp, IndexUnaryOp, and IndexBinaryOp. It is validated
through lookup_dtype, stored on the parent op, and consulted by
_udt_ret_type in place of _resolve_udt_return_type at the four call
sites.

The registration-time shape probe still runs. It lives inside
_get_udt_wrapper, which receives the resolved return type, so declaring
ret_dtype redirects the fit check rather than skipping it: a UDF whose
result cannot fill the declared element is rejected at typing time, the
same as in the inferred case. This is worth more here than it is for
inference. An inferred type is derived from what the UDF returned and so
can hardly contradict it, whereas a declared type is an independent
claim the UDF can get wrong. test_udt_ret_dtype_still_shape_checked
pins all three behaviors: a bad fit rejected, a broadcast-compatible
return accepted, and the record-leaf half of the check.

Every choice below is a judgement call, not a forced move. None is
load-bearing for the feature, and each is listed with what reversing it
would cost, because this is public API and the commitment is the
maintainer's to make, not mine.

  1. SelectOp gets no ret_dtype at all.
    For: GraphBLAS fixes a select operator's return type at BOOL, and
    SelectOp._compile_udt already hardcodes it rather than calling the
    resolver, so the parameter's only legal value would be its default.
    Against: the signature is then inconsistent with the other four
    classes, and a user who does not know the BOOL rule gets a bare
    TypeError from Python's argument binding rather than an explanation.
    To decline: add ret_dtype=None to the two SelectOp register methods
    and raise unless it resolves to BOOL. Purely additive, no caller
    changes; the only cost is that the error test changes shape.

  2. ret_dtype requires is_udt=True.
    For: the builtin path derives a return type per input dtype by
    compiling the function against each sample value and then downcasting
    it toward the input type. One fixed dtype cannot describe that, and
    forcing one would silently recast results across every builtin type.
    Against: the downcast heuristic already carries the comment "There
    should be a way for users to be explicit", so the builtin path is
    arguably where an explicit return type is most wanted.
    To decline: honoring it there means overriding ret_type inside the
    per-sample-value loop in each _build. That is a few lines, but the
    wrapper signature is built from the return type, so a declared type
    that Numba will not store into changes a registration-time error into
    a wrong number. Widening later is compatible; narrowing later is not,
    which is the argument for starting narrow.

  3. The dtype is fixed for the operator, not per input dtype.
    For: it matches how ret_dtype reads, and it keeps the stored state to
    a single attribute consulted at each compile.
    Against: an operator whose output type genuinely varies with its
    inputs now needs one registration per output type.
    To decline: accept a callable and call it with the operand dtypes at
    compile time. Backward compatible, since a DataType and a callable
    are distinguishable. Deliberately not built now.

  4. ret_dtype is rejected with parameterized=True.
    For: a parameterized operator builds its function when called, and
    the inner registration graphblas performs accepts no return dtype
    today, so accepting the keyword at the outer call would promise a
    path that does not exist until the Parameterized* wrappers forward
    it. The error says the combination is unsupported rather than
    pointing at a register call the user cannot reach.
    Against: a user reasonably expects a keyword to work wherever
    is_udt does.
    To decline: thread it through the four Parameterized* classes (slot,
    init kwarg, and the register_anonymous call in _call). Mechanical
    and additive; skipped here to keep the diff narrow while binary.py's
    register internals are being reworked in the preceding commit.

  5. No output UDT is invented from the probe.
    For: the probe is best-effort and runs the UDF once on stand-in
    values, so a UDF whose output shape depends on its input values would
    mint the wrong type silently.
    Against: for the common value-independent UDF the type could have
    been inferred with no user input at all.
    To decline: this one should stay declined. A silently wrong dtype is
    the worst failure mode available here, and the explicit keyword costs
    the user one argument.

Pickling preserves the declared type. The shared reduce predated
ret_dtype, so a cross-process round trip re-registered the op without it
and the reconstruction failed its own shape probe; in-process pickling
hid this, because _deserialize_udf finds the already-registered object
and returns it. The reduce tuple now carries ret_dtype, passed on only
when set, so SelectOp (which shares the path and has no such slot) and
pickles written before this keyword keep working. The test crosses a
real process boundary for exactly that reason.

ret_dtype is also validated eagerly under lazy=True, so an invalid
combination fails at the registration site instead of at first
attribute touch of the delayed op.

A later commit fixes test_udt_ret_dtype_rejects_a_return_it_cannot_hold on SuiteSparse 8. _RetDMisNested's numpy repr is 141 characters, over the 128 a type name holds below SuiteSparse 9, so its registration warns there, and with warnings as errors the test failed on psg 8 CI draws that run slow tests on the suitesparse backend. It now ignores that warning, like the other tests whose UDTs are that large; the test is about the ret_dtype check, not serialization.

Fixes from review.

  • Refuse a UDT UDF that raises for its operand types when the op is typed, then Type a
    UDF that raises on stand-ins as before; only the literal rule reads it
    . A narrowing op
    with a literal thunk or operand, such as lambda x, i, j, t: x[:2] * t with ret_dtype
    a 2-element UDT, gave all zeros. The literal was a 6-element element of the input UDT,
    and the cfunc raised for every element, where Numba prints and swallows the error. The
    shape probe now records a UDF that raises on every stand-in. Typing does not refuse it,
    since stand-ins are degenerate (np.linalg.inv raises on a matrix of ones); the record
    is read only by the next fix. Keep a UDF's stand-in failure as text, so the op it records
    can be freed
    keeps the record from holding the op alive through the exception's
    traceback.
  • Type a number as itself beside a user's op that cannot take it as an element. A
    literal beside a user's is_udt op still becomes an element of the UDT when the op can
    take one. When it cannot (the op does not compile with one, or its UDF raised on every
    stand-in), a Python or numpy number is typed as itself, as a Scalar of it is, so
    v.apply(iu, 3.0) gives what v.apply(iu, Scalar.from_value(3.0)) gives. Probe a
    select UDF that returns an array, so a literal thunk can type as itself
    does the same
    for a select such as x[0] > t.
  • Write 0 (False) into a UDF's built-in output before calling it. When a user SelectOp's
    UDF raised, nothing wrote SuiteSparse's bool keep, an uninitialized local, and the
    stray byte went into the bitmap and the entry count (nvals from 6 to 256 in a Vector of
    size 2). This predates Let UDF registration name its return dtype with ret_dtype #613. Every wrapper with a built-in output now reads its
    operands, writes 0, and calls the UDF. Test that each built-in-type UDF wrapper writes 0
    before calling the UDF
    pins the 16 wrappers for built-in types.
  • Let ret_dtype name the operand's own date, duration or text UDT. Declaring the type
    inference picks for an operand returned as is raised "Return a scalar".
  • Say that a complex return cannot be written as a real ret_dtype (instead of Numba's
    "Failed in nopython mode pipeline"), and Let a complex return be declared as
    ret_dtype=BOOL again
    .
  • Say when a wrong ret_dtype raises, and what it and inference cover and Say in udt.rst
    what a UDF that raises gets, at typing and when it runs
    : doc sentences that were wrong
    or stale.
  • Test ret_dtype with lazy and parameterized registration for every op class. Only
    UnaryOp was tested.

Open question. When a UDF raises at run time for an element, a UDT output keeps
leftover bytes there; Numba prints the traceback, and the bytes are wrong values, not
broken structure. An output of an operand's own type cannot be zeroed without copying the
operand, since GraphBLAS can pass it as that operand (z = f(z, y)). The options are:
(a) zero every UDT output that no operand type has: a prototype exists, and it made an op
widening FP64 to FP64[256] 20% slower (2.8 to 3.4 ms for 20000 elements, JIT off), with
little cost for a narrowing op; (b) zero only an output that narrows the element, as
ret_dtype makes, at little cost; (c) leave it. Narrowing-only or leaving it is
recommended.


Stack created with GitHub Stacks CLI • Give Feedback 💬

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@eriknw
eriknw marked this pull request as ready for review August 4, 2026 16:07
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 2a84393 to 7dc89b9 Compare August 4, 2026 16:12
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 7dc89b9 to 4784268 Compare August 5, 2026 00:06
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 4784268 to 9f5a7a7 Compare August 5, 2026 03:18
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 9f5a7a7 to 80e9dfa Compare August 5, 2026 17:44
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 80e9dfa to 38d1c10 Compare August 5, 2026 18:03
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch 2 times, most recently from 51bb3a8 to c9c6a51 Compare August 6, 2026 07:59
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from c9c6a51 to 48bbd23 Compare August 6, 2026 15:39
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 48bbd23 to c9791e6 Compare August 6, 2026 15:41
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from c9791e6 to 896bc49 Compare August 6, 2026 20:36
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch 2 times, most recently from 067de67 to dd7690a Compare August 7, 2026 02:49
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from dd7690a to a9b0b47 Compare August 7, 2026 05:09
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from a9b0b47 to 4187846 Compare August 26, 2026 17:30
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 4187846 to df7aef5 Compare September 18, 2026 03:09
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from de0c9de to 363a200 Compare October 4, 2026 06:21
@eriknw
eriknw removed this pull request from stack #634 October 8, 2026 21:02
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 363a200 to 6b183d6 Compare October 8, 2026 21:40
@eriknw
eriknw added this pull request to stack #635 October 8, 2026 21:46
@eriknw
eriknw force-pushed the 33-udf-ret-dtype branch from 6b183d6 to 3e33d20 Compare October 9, 2026 03:06
@codecov

codecov Bot commented Oct 19, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.63636% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.94%. Comparing base (eb7c59b) to head (84836e8).

Additional details and impacted files
@@                       Coverage Diff                       @@
##           21-autogenerate-drift-guard     #613      +/-   ##
===============================================================
+ Coverage                        98.91%   98.94%   +0.02%     
===============================================================
  Files                              108      108              
  Lines                            33265    33764     +499     
  Branches                          3868     3915      +47     
===============================================================
+ Hits                             32904    33407     +503     
+ Misses                             233      219      -14     
- Partials                           128      138      +10     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eriknw added 20 commits November 11, 2026 23:16
@eriknw
Let UDF registration name its return dtype with ret_dtype
afd97f8
A UDF's output type is inferred from what the function returns, matched
back to one of the input dtypes by base element type and rank. Inference
can only name a type the operator already has in hand, so an output UDT
that is not an operand is unreachable. A rank-reducing unary op such as
FP64[9] -> FP64[3] cannot be expressed at all: the inferred type is the
input's, and the shape check then rejects the shorter array the UDF
returns. The workaround of passing the desired type in as an extra
operand does not rescue that case either, since two float64 rank-1 UDTs
are indistinguishable to the matcher ("matches more than one input array
UDT").

Nothing structural was in the way. GrB_UnaryOp_new and friends take
ztype separately, and _finalize_udt_op already passes ret_type._carg
independently of the operand types. Only the inference step was
short-circuiting the choice.

Add a ret_dtype= keyword to register_anonymous and register_new on
UnaryOp, BinaryOp, IndexUnaryOp, and IndexBinaryOp. It is validated
through lookup_dtype, stored on the parent op, and consulted by
_udt_ret_type in place of _resolve_udt_return_type at the four call
sites.

The registration-time shape probe still runs. It lives inside
_get_udt_wrapper, which receives the resolved return type, so declaring
ret_dtype redirects the fit check rather than skipping it: a UDF whose
result cannot fill the declared element is rejected at typing time, the
same as in the inferred case. This is worth more here than it is for
inference. An inferred type is derived from what the UDF returned and so
can hardly contradict it, whereas a declared type is an independent
claim the UDF can get wrong. test_udt_ret_dtype_still_shape_checked
pins all three behaviors: a bad fit rejected, a broadcast-compatible
return accepted, and the record-leaf half of the check.

Every choice below is a judgement call, not a forced move. None is
load-bearing for the feature, and each is listed with what reversing it
would cost, because this is public API and the commitment is the
maintainer's to make, not mine.

1. SelectOp gets no ret_dtype at all.
   For: GraphBLAS fixes a select operator's return type at BOOL, and
   SelectOp._compile_udt already hardcodes it rather than calling the
   resolver, so the parameter's only legal value would be its default.
   Against: the signature is then inconsistent with the other four
   classes, and a user who does not know the BOOL rule gets a bare
   TypeError from Python's argument binding rather than an explanation.
   To decline: add ret_dtype=None to the two SelectOp register methods
   and raise unless it resolves to BOOL. Purely additive, no caller
   changes; the only cost is that the error test changes shape.

2. ret_dtype requires is_udt=True.
   For: the builtin path derives a return type per input dtype by
   compiling the function against each sample value and then downcasting
   it toward the input type. One fixed dtype cannot describe that, and
   forcing one would silently recast results across every builtin type.
   Against: the downcast heuristic already carries the comment "There
   should be a way for users to be explicit", so the builtin path is
   arguably where an explicit return type is most wanted.
   To decline: honoring it there means overriding ret_type inside the
   per-sample-value loop in each _build. That is a few lines, but the
   wrapper signature is built from the return type, so a declared type
   that Numba will not store into changes a registration-time error into
   a wrong number. Widening later is compatible; narrowing later is not,
   which is the argument for starting narrow.

3. The dtype is fixed for the operator, not per input dtype.
   For: it matches how ret_dtype reads, and it keeps the stored state to
   a single attribute consulted at each compile.
   Against: an operator whose output type genuinely varies with its
   inputs now needs one registration per output type.
   To decline: accept a callable and call it with the operand dtypes at
   compile time. Backward compatible, since a DataType and a callable
   are distinguishable. Deliberately not built now.

4. ret_dtype is rejected with parameterized=True.
   For: a parameterized operator builds its function when called, and
   the inner registration graphblas performs accepts no return dtype
   today, so accepting the keyword at the outer call would promise a
   path that does not exist until the Parameterized* wrappers forward
   it. The error says the combination is unsupported rather than
   pointing at a register call the user cannot reach.
   Against: a user reasonably expects a keyword to work wherever
   is_udt does.
   To decline: thread it through the four Parameterized* classes (slot,
   __init__ kwarg, and the register_anonymous call in _call). Mechanical
   and additive; skipped here to keep the diff narrow while binary.py's
   register internals are being reworked in the preceding commit.

5. No output UDT is invented from the probe.
   For: the probe is best-effort and runs the UDF once on stand-in
   values, so a UDF whose output shape depends on its input values would
   mint the wrong type silently.
   Against: for the common value-independent UDF the type could have
   been inferred with no user input at all.
   To decline: this one should stay declined. A silently wrong dtype is
   the worst failure mode available here, and the explicit keyword costs
   the user one argument.

Pickling preserves the declared type. The shared __reduce__ predated
ret_dtype, so a cross-process round trip re-registered the op without it
and the reconstruction failed its own shape probe; in-process pickling
hid this, because _deserialize_udf finds the already-registered object
and returns it. The reduce tuple now carries ret_dtype, passed on only
when set, so SelectOp (which shares the path and has no such slot) and
pickles written before this keyword keep working. The test crosses a
real process boundary for exactly that reason.

ret_dtype is also validated eagerly under lazy=True, so an invalid
combination fails at the registration site instead of at first
attribute touch of the delayed op.
@eriknw
Use unique array shapes for the ret_dtype test UDTs
1a5f786
@eriknw
Drop the stale separate-DataTypes note from _array_udt_operands
462f242
The docstring said a flat FP64[2, 3] and a layered FP64[3][2] are separate
DataTypes with separate GrB_Type handles that Numba collapses to one type.
Since "Flatten nested subarray dtypes so each UDT layout has one DataType"
(#589), registration flattens the layered form, so the two are one
DataType. The dedup is unchanged; only its explanation was out of date.
@eriknw
Check a declared ret_dtype against what the UDF returns
ecfac53
A declared ret_dtype skipped inference, and nothing checked it in its place.
Some mismatches were silent wrong answers: a record given a longer tuple
dropped the extra values, a 2-element array given a 3-tuple was left
uninitialized, and FP64 given an array kept its first element. The rest
failed with a multi-line Numba error that never mentioned ret_dtype.
_check_declared_ret_type accepts what inference would: one value per field
(or a value of the record type) for a record, an array or a scalar for an
array UDT, and a scalar otherwise. The field count it reports for a nested
record is the flattened one, and the message says so.

Also, register_new no longer adds an IndexUnaryOp declared to return
something other than BOOL to the select namespace, and an op without
ret_dtype pickles as the 3-tuple it always did, so older versions can still
read it. Remove the unused _array_udt_operands and a repeated validation
call, give the output-UDT test an input shape no other test uses, cover
lazy=True with ret_dtype and an operand returned as-is into a declared
shape, and have the user guide use gb.unary.register_anonymous and
describe the new check.
@eriknw
Ignore the UDT repr size warning in the ret_dtype mismatch test
e8e1870
_RetDMisNested's numpy repr is 141 characters, over the 128 a type name
holds below SuiteSparse 9, so its registration warns there, and the test
failed on psg 8 draws that ran slow tests on the suitesparse backend. It is
about the ret_dtype check, not serialization.
@eriknw
Cover a declared record ret_dtype returned as is, and a return that c…
5d16c7e
…an be None

_check_declared_ret_type accepts a value of the declared record type,
such as the operand itself, and names a return that can be None in its
error, but no test reached either. Return the record operand from a
unary op with that record as ret_dtype and check the result, and check
the error for x if x > y else None under ret_dtype=FP64.
@eriknw
Test that unpickling a named UDF re-registers it with its ret_dtype
6cf04d6
_deserialize_udf registers a named op again when the name is missing,
which only happened in the cross-process pickle test's subprocess, so
coverage never saw it. Register a named op with a ret_dtype, check that
unpickling returns the registered op, remove the name, and check that
unpickling registers a new op that still returns the declared type. The
name is removed again afterwards, as the cross-process tests in
test_pickle.py do.
@eriknw
Refuse a UDT UDF that raises for its operand types when the op is typed
d6b0cb0
The shape probe ran the UDF on stand-in operands and gave up on any
exception, so an op whose UDF cannot run on its operand types typed, and
its cfunc raised for every element, where Numba prints and swallows the
error: each element kept what SuiteSparse had in the buffer.

    iu = IndexUnaryOp.register_anonymous(lambda x, i, j, t: x[:2] * t,
                                         is_udt=True, ret_dtype=R2)
    v.apply(iu, 3.0)    (v of a 6-element array UDT R6; the literal is an R6)
    before: [[0, 0], [0, 0], [0, 0]], and "Exception ignored ... ValueError:
            unable to broadcast argument 1 to output array" on stderr
    after:  UdfParseError: UDT UDF raised ValueError when run on sample
            values of (R6, UINT64, UINT64, R6): unable to broadcast ...

The same for a BinaryOp with right=3.0. A UDF that raises on the probe's
ones is now run again on twos, since what it raises may depend on the
values, and only an exception on both is refused; an ArithmeticError still
leaves the op unchecked. test_udt_udf_shape_check_runs_udf_only_for_built_arrays
keeps its UDF that raises for ones only, which now types from the twos.
On #612 the same UDF without ret_dtype gave the same zeros.
@eriknw
Type a number as itself beside a user's op that cannot take it as an …
8bfacf4
…element

A literal beside a user's is_udt op (an untyped BinaryOp, IndexUnaryOp or
SelectOp) becomes an element of the operand's UDT. A narrowing op with
ret_dtype that uses the number, as x[:2] * t does, cannot take a 6-element
t, so a literal failed where a Scalar of it works:

    iu = IndexUnaryOp.register_anonymous(lambda x, i, j, t: x[:2] * t,
                                         is_udt=True, ret_dtype=R2)
    v.apply(iu, 3.0), v.apply(iu, np.float64(3.0)), and a BinaryOp's
    right=3.0 or left=3.0     (v of a 6-element array UDT R6)
    on #613:   [[0, 0], [0, 0], [0, 0]], silently
    on the previous commit: UdfParseError (the UDF raises on R6, R6)
    after:     [[0, 3], [30, 33], [60, 63]], as with Scalar.from_value(3.0)

_own_scalar_type_for_user_op types a Python or numpy number as itself
(FP64 for 3.0, INT64 for 3, FP32 for np.float32) when the op cannot be
typed with an element and can be typed with the number; otherwise it stays
an element, so an op that takes an element (x + t on INT8[3], typed INT8[3]
as before), and the fit check (0.5 for INT8[3] raises), are unchanged. A
record op that adds a number to each field, which raised before, now works.
Built-in ops, typed ops and Monoids are untouched, as is an int beyond
INT64. udt.rst's literal row says so.
@eriknw
Let ret_dtype name the operand's own date, duration or text UDT
1cc276d
_check_declared_ret_type's scalar branch took only numbers and bools, while
inference names a date, duration or bytes UDT for an operand returned as
is, so declaring the type inference picks broke an op that works:

    dt = register_anonymous(np.dtype("M8[s]"))      (also m8[s], S5)
    UnaryOp.register_anonymous(lambda x: x, is_udt=True, ret_dtype=dt)[dt]
    before: UdfParseError: UDT UDF returned datetime64[s], which cannot be
            written as ret_dtype=... Return a scalar.
    after:  typed, with return type dt

A value of the declared UDT's own Numba type is accepted, as the record
branch accepts a record of the declared type.
@eriknw
Say that a complex return cannot be written as a real ret_dtype
7e7fe2a
Numba has no cast from complex to real, so the wrapper failed to compile
and _summarize_numba_typing_error kept only Numba's first line:

    BinaryOp.register_anonymous(lambda x, y: x * 1j + y, is_udt=True,
                                ret_dtype=FP64)[FP64, FP64]
    before: UdfParseError: binary.<lambda> does not work with (FP64, FP64):
            Failed in nopython mode pipeline (step: native lowering)
    after:  UdfParseError: UDT UDF returned complex128, which cannot be
            written as ret_dtype=FP64. Return a real value, or declare a
            complex ret_dtype.

The same for a complex scalar or array into a real array UDT, which said
"No implementation of function ... setitem".
@eriknw
Say when a wrong ret_dtype raises, and what it and inference cover
c0fbe10
udt.rst called a wrong ret_dtype "a registration error"; registering
succeeds, and the error comes when the op is typed (op[dtype], or its
first use), as the sentence before says. It also said ret_dtype does not
apply to "builtin (non-UDT) dtypes", but it does apply to built-in input
dtypes and accepts a built-in return type; what it excludes is an op
registered without is_udt=True. The four register_anonymous docstrings
(and _udt_ret_type's) said inference "can only name a type that is
already an input"; it also names a built-in dtype, such as FP64 from a
record.
@eriknw
Test ret_dtype with lazy and parameterized registration for every op …
aa304fb
…class

Only UnaryOp was tested, so dropping ret_dtype from BinaryOp's or
IndexUnaryOp's deferred (lazy=True) registration, or BinaryOp's
parameterized check, left every test passing. The new test runs both over
UnaryOp, BinaryOp, IndexUnaryOp and, where SuiteSparse has it,
IndexBinaryOp; each of the three mutations now fails it.
test_udt_ret_dtype_index_ops also runs its IndexUnaryOp and IndexBinaryOp
on a Vector and checks the values; it only checked return_type.
@eriknw
Type a UDF that raises on stand-ins as before; only the literal rule …
d80aa33
…reads it

"Refuse a UDT UDF that raises for its operand types when the op is typed"
refused a UDF that raised a non-arithmetic exception on stand-in operands
of ones and of twos. Such stand-ins are degenerate, and the index and
thunk stand-ins do not vary, so valid UDFs that work on real data were
refused (r612):

    UnaryOp(np.linalg.inv) on FP64[2, 2]          (a matrix of ones is singular)
    UnaryOp: if x[0] >= x[1]: raise ...; x * 2     (equal fields)
    IndexUnaryOp: x.reshape((int(t), -1)).sum(0), and
                  if t < 1.0: raise ...; x[:2] * t  (the thunk stand-in is 0.5)
    on 542713c1: UdfParseError when typed
    after:       typed and run, as at #613's head

_run_udf_probe now only reports: it returns (ran, error), where error is
the exception raised on both stand-ins, and typing never raises for it.
_finalize_udt_op records such an op in _RAISES_ON_SAMPLES. The literal rule
reads that record: beside an op whose UDF raised on every stand-in
element, a number is typed as itself if the op types with the number's
type (its stand-in is a fixed sample, not the number, so it need not pass
the probe). So v.apply(iu, 3.0) still gives [0, 3], and so does the thunk
check above with a literal 3.0.

test_udt_udf_raising_for_its_operand_types_raises_when_typed becomes
test_udt_udf_raising_on_stand_ins_still_types, with r612's four UDFs.
@eriknw
Write 0 (False) into a UDF's built-in output before calling it
c283f4d
Numba prints and swallows an exception raised in a cfunc, so a UDF that
raised left its output as it was. For a select that is SuiteSparse's
"bool keep", an uninitialized local (GB_select_generic_bitmap.c), which
then went into the bitmap and the entry count: r612 saw nvals from 6 to
256 in a Vector of size 2 when a user SelectOp's UDF raised.

Every wrapper of a user's UDF with a built-in output (UnaryOp, BinaryOp,
IndexUnaryOp and SelectOp, IndexBinaryOp; built-in operand types, and the
UDT wrapper's built-in outputs) now reads its operands, writes 0, then
calls the UDF. Operands are read first because the output can be one of
them (z = f(z, y) in a reduce); writing 0 first broke a dozen reduce tests.

Expected and now checked, for a UDF that raises for negative values:
    INT64 [-1, 5, 2, -3, 7], select x > 3   -> nvals 2, indices [1, 4],
                                               values [5, 7]
    the same with apply x * 10              -> [0, 50, 20, 0, 70]
    a record UDT select whose UDF raises for entries 0 and 3
                                            -> nvals 1, index [1]
Those held by chance here before (the stray byte was 0). A second test
calls a UDT wrapper directly on an output of 0xAB bytes: it now leaves 0,
for a bool and for an FP64; it left 0xAB.

A UDT output is not zeroed: it can also be an operand, copying a UDT
operand first would copy every element, and Numba cannot re-raise, so
catching the exception would silence it. _compose_wrapper_body says so.
@eriknw
Probe a select UDF that returns an array, so a literal thunk can type…
13069b4
… as itself

A user's SelectOp UDF that uses its thunk as a number, such as
x[0] > t, gets an element of the UDT for a literal thunk, so it returns
three bools where the select needs one, and the wrapper raised for every
entry:

    v.select(first_above, 0)    (v of an INT8[3] UDT: [1, 2, 3], [-4, 5, 6])
    before: nothing kept (with a stray byte deciding, before the previous commit)
    after:  [0] kept, as with Scalar.from_value(0); also for np.int8(0) and 0.5

SelectOp._compile_udt now hands the UDF's Numba return type to the
wrapper, and _check_udf_fills_output runs the UDF on stand-ins when it
returns an array for a scalar output. One that raises, or returns more than
one value, is recorded in _RAISES_ON_SAMPLES (not refused: the shape may
depend on the values), so the literal rule types the number as itself. A
select that takes an element ((x == t).all()) still gets one.
@eriknw
Let a complex return be declared as ret_dtype=BOOL again
769c233
"Say that a complex return cannot be written as a real ret_dtype" also
refused BOOL, which worked: the wrapper writes bool(...) for a BOOL output.

    BinaryOp.register_anonymous(lambda x, y: x * 1j + y, is_udt=True,
                                ret_dtype=BOOL), on 1.5 and 0.0
    #613's head: True
    before:      UdfParseError: ... cannot be written as ret_dtype=BOOL
    after:       True
@eriknw
Say in udt.rst what a UDF that raises gets, at typing and when it runs
33a8bc0
The shape check's sentence said a UDF that raises on the sample values is
not checked, which holds again after "Type a UDF that raises on stand-ins
as before; only the literal rule reads it"; it now says why (it may raise
only for some values), and what an exception at run time leaves: the
element is not written, and a built-in output (a select's bool) is 0 or
False.
@eriknw
Keep a UDF's stand-in failure as text, so the op it records can be freed
354f38c
_RAISES_ON_SAMPLES kept the exception the UDF raised on its stand-ins,
and its traceback holds the frames of _check_udf_fills_output,
_get_udt_wrapper and _compile_udt, which hold the typed op and its
parent, so the weakly keyed record kept its own key alive (r612):

    5 IndexUnaryOps (x[:2] * t, ret_dtype FP64[2]) applied with a literal
    thunk, then dropped and gc.collect()
    before: 5 of 5 ops alive, with their GrB handles; the record holds 5
    after:  0 of 5 alive; the record is empty

The probe now returns the failure as "Type: message", which the wrapper
and the record keep instead; only membership is read. A test drops three
recorded ops and checks that weak references to each op and typed op die.
@eriknw
Test that each built-in-type UDF wrapper writes 0 before calling the UDF
84836e8
Nothing pinned the zero writes in the 16 wrappers a user's UnaryOp,
BinaryOp, IndexUnaryOp (and SelectOp) and IndexBinaryOp get for built-in
types: deleting every one left the tests passing (r612), since a select
whose UDF raises keeps nothing by chance as often as not. The test catches
the cfuncs from numba.cfunc while each op registers a UDF that always
raises, and calls each with an output of 0xAB bytes: the output must be 0.
With the 16 writes deleted it fails on the first wrapper.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@eriknw
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.