GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |
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
|
Codecov Report❌ Patch coverage is 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:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
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.
_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.
…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.
_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.
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.
…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.
_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.
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".
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.
…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.
…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.
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.
… 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.
"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
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.
_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.
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.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.