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 — Follow-ups to #591: Monoids on UDT pairs, out-of-range ints, select.row bounds, wording by eriknw · Pull Request #661 · python-graphblas/python-graphblas Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## main #661 +/- ## ========================================== + Coverage 97.96% 98.72% +0.75% ========================================== Files 104 104 Lines 29973 30157 +184 Branches 3534 3567 +33 ========================================== + Hits 29364 29772 +408 + Misses 360 247 -113 + Partials 249 138 -111 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
20fa136 to
a204404
Compare
Five tests reach the lines of this PR that no test ran (local coverage of the full suite with --runslow: suitesparse and vanilla, with and without Numba): - Text in a record's duration field, or text numpy cannot read as a date or a duration, takes the field's type, and converting it raises numpy's error (literals.py: _as_numpy_date and leaf_dtype). - Accumulating a Vector into a UDT Scalar raises the TypeError a built-in Scalar raises (udt.py: _scalar_accum_operand). - A NaT of no unit does not fit a one-byte field: b"N" and NaT cast to each other neither way (literals.py: _converts_back). - The error for an array of records holding an array of bool names the bool array (dtypes.py: _numba_untyped_array_element). - Without Numba, a NaT duration in an array field of a record stays NaT in a coarser unit (dtypes.py: _nats_in_leaf_units). The try/except around lookup_dtype in _weak_builtin_literal_dtype is removed: where the backend has no complex types (vanilla), lookup_dtype registers a complex literal's dtype as a UDT rather than raise, so the except that returned None never ran on any backend.
monoid.plus["INT8", "INT16"] is monoid.plus["INT16"]: the Monoid takes the type the pair promotes to. Main did the same for two UDTs (it unified them with numpy), but since UDT pairs stopped being unified, every UDT pair went to Monoid._compile_udt, which raises TypeError "Monoid inputs must be the same dtype". The comment beside that branch said indexing "still raises". Now an indexed Monoid on a pair with a UDT takes the type its BinaryOp gives the pair, the per-leaf promotion that v.ewise_mult(w, monoid.plus) already uses. Behavior change (before -> after), with R8 and R16 records of one int8 and one int16 field of the same name: - monoid.plus[R8, R16] and monoid.plus[R16, R8]: TypeError -> monoid.plus[R16], as on main. - monoid.plus[R8, "INT16"]: TypeError -> monoid.plus[R16], as binary.plus[R8, "INT16"] gives R16 (main raised TypeError). - Records whose fields do not pair: TypeError -> the KeyError the BinaryOp gives for that pair.
…er type An int beyond the operand's range is typed exactly, not refused, when the op keeps no result in the operand's type: a comparison, or an op that gives another type anyway (int8_vec / 300 is FP64). _gives_another_type decided the second only for a named op it could index, so it said no for a parameterized op (binary.isclose has no __getitem__), for the ops a parameterized op makes (binary.isclose(0.1) is anonymous), and for a user's anonymous op. Those raised OverflowError. Main typed the int as INT64 and gave False, as numpy's isclose does. Now any op counts by the type it gives on the operand's dtype, and a parameterized op is typed with its default parameters, as get_typed_op types it. An op that gives the operand's type (plus, or a user's x + y) still raises. Behavior change (before -> after), with v an INT8 Vector of [100, -100]: - v.apply(binary.isclose, right=300), left=-300, and with binary.isclose(0.1): OverflowError -> [False, False], as on main. - v.ewise_union(w, binary.isclose, 0, 300): OverflowError -> False where the default is used. - v.apply(gt, right=300) with gt a user's anonymous x > y: OverflowError -> [False, False]. - int64_vec.apply(binary.isclose, right=2**63): main OverflowError -> compared in FP64, as numpy's isclose compares it.
The field rules ended with "Strings and Python objects are not supported", though a bare text, datetime64 or timedelta64 UDT registers, stores, and compares with ==, != and isequal, and text fields compare in records. The bullet on arrays Numba cannot type left out arrays of str and bytes, which register_anonymous refuses with the same ValueError.
…aves Its docstring said the generated expression operates on the whole sub-array. Binary ops now compute an array field element by element, as they compute an array UDT; only unary ops still apply the expression to the whole sub-array.
Scalar == [1] raised "Bad type for keyword argument `other=` in Scalar.ewise_mult(op=binary.eq)", the method == calls inside; main named Scalar.isequal, also not what the user wrote. Now == converts a literal as ewise_mult does, and the error names Scalar.__eq__ (or Scalar.__ne__ for !=), for built-in and UDT Scalars.
Storing or accumulating an object whose type does not cast, such as s(accum=binary.plus) << Scalar.from_value(0.5) into a UDT Scalar, raised "cannot store a result of type FP64 ... Use .new() for an object of the result's type, or compute the result in the type you want to store". The FP64 Scalar is a value, not a result, and .new() does not help it. Now _store_cast_op takes is_value, which the stores of an object as it is pass (w << v, C << A.T, assigning or accumulating a Scalar, and _cast_for_store's other callers): "cannot store a value of type FP64 in an object of type INT8[3]: a UDT casts only to another UDT." A computed result keeps the old message and its advice.
The previous commit typed an indexed Monoid by its BinaryOp's result on the pair. For plus, times, min and max that is the promoted type, as on main, but binary.any on two UDTs keeps the second operand's type, so monoid.any[R16, R8] became monoid.any[R8], and v16.reduce(monoid.any[R16, R8]) raised DomainMismatch where main gave an R16 Scalar. Main unified the pair in either order. monoid.any now takes the type lifted arithmetic (plus) gives the pair. With R8 and R16 records of one int8 and one int16 field: - monoid.any[R16, R8]: monoid.any[R8] -> monoid.any[R16], as on main (and in the other order, as before). - monoid.any[R8, "INT16"]: the built-in monoid.any[INT16] -> monoid.any[R16], as monoid.plus gives (main raised TypeError). - Records that do not pair: monoid.any[R8] -> plus's KeyError, as for monoid.plus, min and max (main raised TypeError).
… like one An int beyond an integer operand's range is compared as infinity of its sign by the built-in comparisons. The check was the op's name alone, and register_anonymous names an op after its function, so a user's op made from a function named ge or eq got the int replaced by infinity. Before the previous PR's out-of-range rule such an op raised OverflowError first; since it, int8_vec.apply(register_anonymous(ge), right=300), with ge returning x * 1.0 - y, gave [-inf, -inf], and an eq returning y - x > 250 gave [True, True]. Main gave [-200.0, -400.0] and [False, True], which they give again: the check now also asks that the op is not anonymous, as the exact typing beside it does.
…t type The previous PR's rule types an int beyond the operand's range exactly beside any op whose result has another type. For a user's op beside INT64 or UINT64, that exact type (UINT64 for 2**63, FP64 for 2**64) unifies with the operand to FP64, which rounds the int: a register_anonymous x < y gave [False, True] for int64_vec [2**63 - 1, 0] with 2**63, where numpy and binary.lt give [True, True]. #591 already did so for a register_new op; main raised OverflowError. Built-in comparisons are given infinity instead, and the other built-ins that give another type (truediv, isclose, the float ufuncs) compute in float anyway, as numpy does. Now a user's op raises OverflowError, as #591 did, where the exact type and the operand would unify to a float type. Where they share an integer type (300 beside INT8 is INT16) it still takes the int exactly. Built-in ops are told apart by _STANDARD_OPERATOR_NAMES, and the ops a built-in parameterized op makes (binary.isclose(0.1), which is anonymous) by their parent. types.rst states the rule. Behavior change (before -> after), with an anonymous or register_new x < y or x != y: - int64_vec.apply(op, right=2**63) and right=-2**63 - 1, uint64_vec with 2**64, the Scalar, Matrix and ewise_union paths: a wrong answer -> OverflowError (main: OverflowError). - int64_vec.apply(binary.isclose, right=2**63): unchanged, [True, False] as numpy's isclose.
An array UDT Scalar or Vector compared with a dict (a == {}) raised
AttributeError: 'NoneType' object has no attribute 'items', since the
comparison read the dict as a record of the array dtype's fields, which
it has none of. Main raised the TypeError for a value that is not a
literal. Now the dict is read as a record only for a record dtype, as
_literal_type does, and a == {} raises "Bad type for argument `other` in
Scalar.__eq__(...)" (Vector: the usual Vector.apply TypeError).
udt.rst's bullet on text, datetime64 and timedelta64 UDTs listed ==, !=, isequal, first, second, pair and any as working. Without Numba, a Vector's ==, isequal, first, second and any raise KeyError; only a Scalar's ==, != and isequal (compared in numpy) and pair work.
select.row(A < k) reads its thunk from the comparison A < k, which since #591 types a literal beside the matrix's values (NEP 50): beside INT8 or UINT8 values, 300 is infinity, and beside FP32, 2**24 + 1 rounds. That is right for a value comparison, but the helpers used the same thunk for an index. On a 400-row INT8 matrix, select.row(B < 300) kept all 400 rows and select.row(B > 299) none; on FP32 with 2**24 + 3 rows, select.row(F < 2**24 + 1) dropped row 2**24. Main was right. Now an expression remembers the right operand as the caller gave it (_right_literal, set by Vector.apply, Matrix.apply, Scalar.apply and Scalar's ewise_mult, which Scalar comparisons use), and the row, column and index helpers compare indices with that number, or with the thunk Scalar's value when a Scalar was given. As indices are whole numbers, the bound is made one: i < t is i <= ceil(t) - 1, i >= t is i > ceil(t) - 1, and i <= t and i > t take floor(t). GraphBLAS truncated a float thunk toward zero, so select.row(A > -0.5) dropped row 0 and select.row(A < 2.5) row 2 (on main too). A whole bound beyond INT64 is clamped to it, which holds every index. The Scalar unwrap now comes first, so select.row(s < 1) works (it raised AttributeError). Covered: Vector, Matrix (also transposed, through select.column) and Scalar operands, INT8, UINT8 and FP32, fractional and infinite bounds, and a Scalar given as the bound.
…included The previous PR's _is_builtin_op asks _STANDARD_OPERATOR_NAMES whether an op is python-graphblas's own, and that set held only the names register_new recorded during initialization. BinaryOp._initialize makes cdiv, truediv and rtruediv directly, with the semirings on cdiv, and binom is registered on first use, so none of them was there. A user's op beside a 64-bit int raises OverflowError where the exact type would compute in float, and truediv was taken for one: uint64_vec / -1, int64_vec / 2**63 and int8_vec / 2**70 raised, where #591 (and, for the first, main) and numpy compute them; binary.numpy.true_divide and divide are truediv too. Now, once the operators are initialized, every op the namespaces hold, and every name queued for first use, is recorded as standard. That covers whatever _initialize makes, now or later, with no list to keep in step; it adds exactly cdiv, truediv, rtruediv, binom and the five semirings on cdiv. They now also pickle by name, as the other built-ins do; their old pickles still load.
A bound given as a Scalar expression (w[0] with autocompute), a 0-d array or np.True_ was recorded as the caller's literal, then found not to be a number, so the helpers shifted lt to le without shifting the bound: select.row(A < np.array(3)) kept rows 0 to 3, and select.row(A >= w[0]) dropped row 3. Use the caller's literal only when it is a real number; otherwise the thunk, made from that operand with its own type, holds the bound. Found by review (rA).
"Take the thunk's value as an index bound that is not a number" read the
thunk's value whatever it was, and for A.apply(binary.le, left=3) the
second operand is the Matrix:
select.row(A.apply(binary.le, left=3))
before: AttributeError: 'Matrix' object has no attribute 'value'
after: TypeError: Bad type for keyword argument `thunk=` in
Vector.select(op=select.rowle), as on main
_index_bound reads a value only from a Scalar thunk; any other goes to the
op as it is, which says what is wrong. Found by rA (Round 3, R3-3).
Every comparison with NaN is False, but NaN went to rowle or rowgt as it
was, and its cast to the INT64 thunk made it 0:
A of 4 rows: select.row(A < nan), (A <= nan)
before: row 0 kept (main too)
select.row(A > nan), (A >= nan)
before: rows 1, 2 and 3 kept (main too)
after: nothing kept, also for Scalar(nan) and np.float32(nan)
_index_bound now gives a NaN bound as le -1 or gt INT64_MAX, which no
index meets. Found by r604 (port review, P2).
numpy 2.5 deprecates the generic unit, so np.datetime64("NaT") warns, and
CI (which turns warnings into errors) failed this test on every job that
drew numpy 2.5. A NaT in seconds reaches the same check: the literal still
does not fit the S1 field, with the same error.
test_python_number_is_weak_beside_builtin_dtypes registers it in the binary namespace, so test_binarytypes failed whenever pytest-randomly ran it after that test.
A date given as text is parsed by np.datetime64, which reads "NaT" (in any
case) and empty text as NaT with no unit. numpy 2.5 deprecates that unit, so
comparing a date UDT, or a record's date field, with "NaT" warned (an error
under the test suite's filters), where main compares without a warning:
s == "NaT" # s a Scalar of an M8[s] UDT
before (numpy 2.5): DeprecationWarning: The 'generic' unit for NumPy
timedelta is deprecated
after: False, as numpy's NaT equals nothing
Such text is now NaT in the field's unit. Other text keeps the unit it is
written to, so a finer date still does not fit a coarser field. Found by
rA's review of the numpy 2.5 CI fixes.
a204404 to
47c00c9
Compare
monoid.any[X, Y] found the type the pair promotes to by compiling
binary.plus on the pair (a Numba cfunc and a GrB_BinaryOp that nothing
used) and reading its return type. _monoid_pair_type, in udt.py beside
the other UDT result-type rules, runs the same check and promotion
(_check_udt_pair, _udt_op_types) and compiles nothing. It gives the type
the compiled ops give on every pair tried. Without Numba,
monoid.any[X, Y] now raises the KeyError of monoid.any[XY] ("any does
not work with ..."), not one that names plus.
Also drop a test comment that said main gave monoid.any[R8] for
monoid.any[R16, R8]: main gives monoid.any[R16], as this branch does.
…object
"Call a stored or accumulated object a value in the store-cast error"
had _cast_for_store pass is_value=True for everything, but indexed
assignment hands it expressions too:
v[:] << w.apply(unary.ainv) # w is FP64, v of an INT8[3] UDT
before: cannot store a value of type FP64 in an object of type ...
after: cannot store a result of type FP64 in an object of type ...
Use .new() for an object of the result's type, or ...
A computed result keeps the result message, as that commit meant; only
a Scalar, Vector or Matrix stored as it is is a value.
There was an error while loading. Please reload this page.
|
As per typical of AI, much of the prose in this PR needs refined. Will do later. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.