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 — Handle dtypes (esp. UDTs) better in ewise_union by eriknw · Pull Request #517 · python-graphblas/python-graphblas

Conversation

@eriknw

@eriknw eriknw commented Oct 30, 2023

Copy link
Copy Markdown
Member

I first introduced these improvements in #501, but I'm splitting them into a new PR b/c they should go into main, and it will make it easier to review (and revert if necessary) #501.

I plan on merging once CI passes. I don't think there's anything particularly controversial or dangerous here, but I'll be happy to answer any questions even after merging.

Sorry, something went wrong.

Uh oh!

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

All reactions
@eriknw
Handle dtypes (esp. UDTs) better in ewise_union
a14fa33
@codecov

codecov Bot commented Oct 30, 2023 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Codecov Report

Merging #517 (a14fa33) into main (bbb1f4f) will increase coverage by 0.62%.
The diff coverage is 86.00%.

@@            Coverage Diff             @@
##             main     #517      +/-   ##
==========================================
+ Coverage   98.96%   99.59%   +0.62%     
==========================================
  Files          95       95              
  Lines       21254    21291      +37     
  Branches     3987     3999      +12     
==========================================
+ Hits        21035    21205     +170     
+ Misses        153       56      -97     
+ Partials       66       30      -36     
All reactions

Sorry, something went wrong.

Uh oh!

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

@eriknw
eriknw merged commit 7935e50 into python-graphblas:main Oct 30, 2023
@eriknw eriknw mentioned this pull request Oct 30, 2023
Add semiring(A @ B @ C) that applies semiring to both matmuls #501
Merged
4 tasks done
@eriknw eriknw mentioned this pull request Oct 28, 2026
Fix Scalar infix expressions with a literal operand #654
Merged
eriknw added a commit that referenced this pull request Nov 11, 2026
@eriknw
Fix Scalar infix expressions with a literal operand (#654)
8aef9c4
* Keep a literal on its own side of a Scalar's `&` and `|`

Behavior fix. Since 2023.1.0, `x & s` and `x | s`, with a literal x and
a Scalar s, built the expression with s on the left and x on the right,
so the op applied to it ran with its operands swapped, and every op that
does not commute gave wrong values, silently:

    s = Scalar.from_value(5)
    binary.minus(1 & s)      # was 4, now -4
    binary.truediv(1 | s)    # was 5.0, now 0.2
    binary.first(1 & s)      # was 5, now 1
    binary.lt(1 & s)         # was False, now True

So did rminus, cdiv, rdiv, pow, the other comparisons and
is-comparisons, and the rest of the ops that do not commute. So did the
defaults of an ewise_union, each of which went with the other operand:
with an empty Scalar e, `binary.minus(1 | e, left_default=0,
right_default=10)` was -1 and is now -9. And so did chains:
`binary.minus(1 & (2 & s))` was 2 and is now 4.

The expression now keeps the literal where it was written, as it is, and
the op applied to it types it: Scalar's ewise_mult, ewise_add and
ewise_union take a literal on the left as they take one on the right,
typed beside the Scalar by literals._literal_operand. A Python number is
weak, as beside `-`: `binary.minus(2 & int8_s)` is INT8 and
`binary.minus(0.5 & int8_s)` is FP64, as `2 - int8_s` and `0.5 - int8_s`
are. Beside a UDT, a lifted op such as minus types the literal as numpy
would and `first` makes it an element of the UDT.

How a literal is typed depends on the op (300 beside INT8 is an
OverflowError for `-` and is compared as it is by `<`), so the literal
is not converted when `&` or `|` builds the expression. Converting it
then, with its own dtype, would get the typing wrong: the 2 of
`int8_s & 2` would be INT64 under every op, where `int8_s - 2` keeps it
INT8, and `0.5 & int8_udt_s` would cut 0.5 to 0 in the UDT.

`op(x & y)` now asks the expression to call its left operand's method
(InfixExprBase._call_left), so a Scalar expression can pass a literal
there.

Two more errors with a literal operand are fixed:

- Computing a BOOL expression with a literal by itself, as
  `(s & True).new()` and `(False | s).new()` do, raised AttributeError:
  'bool' object has no attribute 'dtype'. It now types the literal for
  LAND or LOR and computes, or raises the usual TypeError for a dtype
  other than BOOL.
- `s & object()` gave an expression that failed only when an op was
  applied; it now raises TypeError at once, as `s + object()` does. The
  check types the literal as `==` would and drops it. Beside an infix
  expression, as in `(s & t) & object()`, it raises when the op is
  applied.

* Apply `lit - s` and the like as minus(lit, s), not as rminus(s, lit)

Behavior fix. Since 2023.1.0, a Scalar's reflected operators (`lit - s`,
`lit / s`, `lit ** s` and so on, with a literal on the left) computed
the op's commutes_to with the Scalar first, rminus(s, lit). That fails
where no such op exists:

    1 - udt_s, 1 / udt_s, 1 // udt_s, (1, 2) - udt_s
        # KeyError: 'rminus does not work with ...'; no reflected op
        # takes a UDT, though `udt_s - 1` and `1 - udt_vector` work
    7 % s     # TypeError for every dtype: numpy.mod has no reflected op
    2 ** s    # TypeError without Numba: rpow is a Numba function

Now the literal becomes a Scalar on the left, typed beside s as in
`binary.minus(1 & s)`, and the op itself is applied: `1 - udt_s` is
[-4, -6] for udt_s = [5, 7], and `7 % int8_s` is 2 for 5.

`lit ** s` now gives what `binary.pow(lit, s)` and `lit ** vector` give.
That changes four kinds of result, where rpow, a Numba function,
differed:

- an integer power out of range saturates, as GraphBLAS's pow does,
  where rpow wrapped: `3 ** int8_s` for 5 was -13 and is now 127, and
  `0 ** int8_s` for -3 was 0 and is now 127;
- `False ** bool_s` is BOOL, not INT8;
- `0 ** complex_s` is 0, not nan (for 1+2j), and other complex powers
  may differ in the last bit;
- a Python int beyond INT64 beside a BOOL Scalar, as in
  `(2**64 - 1) ** bool_s`, raises OverflowError, as
  `bool_s ** (2**64 - 1)` and `(2**64 - 1) ** bool_vector` do, where it
  gave a UINT64.

Nothing else changes: for `+ - * / // % ** ^`, nine Scalars (INT8,
INT64, UINT8, FP32, FP64, BOOL, empty) and eight literals, every result
the previous code gave is the same but those of `**`; and for the seven
that are not empty, `lit OP s` now matches `binary.op(lit, s)` for each
of `+ - * / // % **` (it did not in 66 of 392 cases).

* Show a Scalar's `&` and `|` with a literal or a non-BOOL dtype

Behavior fix. Since 2023.1.0, showing a Scalar `&` or `|` expression
raised in two cases:

    repr(s & 1), repr(1 | s), repr(t & True), and their _repr_html_
        # AttributeError: 'int' object has no attribute 'name'
    repr(s & s)   # s not BOOL, with autocompute on (the default)
        # TypeError: Bad dtypes for `x & y`!  Automatic computation ...

A literal operand now shows its value, as an unnamed Scalar does in an
expression, and its own type where a dtype goes, since it has no dtype
until an op types it:

    gb.ScalarEwiseMultExpr  left_dtype  right_dtype
    s_1 & 1                       INT8          int

The text repr now computes the expression to show its result only where
the HTML repr, and the reprs of Vector and Matrix infix expressions,
do: for `x @ y`, and for `x & y` and `x | y` of BOOL operands. A literal
counts as BOOL when it is a bool, so `t & True` shows its result.
A chain such as `(s & t) & u` still raises in a repr, as it does for
Vector and Matrix.

* Raise DimensionMismatch, not TypeError, for a size mismatch beside an infix expression

A behavior fix. When an operand of `&`, `|` or `@` was itself an infix
expression and the sizes did not match, building the error crashed in
_mismatched, which called _dummy(obj) while _dummy took a second
parameter, obj_type, that it never used (since #517). Before:

    >>> (v & v) & w    # v of size 3, w of size 4
    TypeError: _dummy() missing 1 required positional argument: 'obj_type'

After:

    >>> (v & v) & w
    DimensionMismatch: ...

The same held with the expression on the right (w & (v & v)), for `|`,
for Matrix operands ((A & A) & B), and for `@` ((A @ A) @ w). _dummy
now takes only obj. Tests cover Vector and Matrix with the expression
on either side, for `|`, `&` and `@`.

* Let numpy's operators defer to graphblas objects

Behavior change. A numpy scalar or 0-d array on the left of an infix
operator was computed by numpy, which converted a Scalar with __array__
and could not convert a Vector or Matrix. graphblas objects now set
__array_priority__, so numpy's operators return NotImplemented and Python
calls the graphblas reflected method, which takes the numpy value as a
literal operand, typed as it is on the right:

    s = Scalar.from_value(5, dtypes.INT8)
    np.int16(2) - s     # was np.int16(-3), now an INT16 ScalarExpression
    np.int16(2) < s     # was np.True_, now a BOOL ScalarExpression
    np.int16(5) == s    # was np.True_, now True (Scalar's ==)
    np.int16(2) - v     # was a TypeError, now an INT16 VectorExpression
    np.int16(2) == A    # was a TypeError, now a BOOL MatrixExpression

The same holds for expressions, A.T and index expressions. `&`, `|` and
`^` now go to graphblas too: `np.int16(2) & s` was numpy's bitwise and,
np.int16(0), and is now the TypeError of `2 & s` (only BOOL computes).

With an empty Scalar, numpy computed with nan; the result is now what a
Python float on the left gives:

    e = Scalar(float)
    np.float64(5) + e   # was nan, now 5.0, as 5.0 + e is (and `-`)
    np.float64(5) * e   # was nan, now an empty Scalar, as 5.0 * e is
    np.float64(5) < e   # was np.False_, now an empty Scalar
    np.float64(5) == e  # was np.False_, now False

What numpy computed and graphblas does not now raises TypeError:

    np.array([1, 2]) - s    # was array([-4, -3])
    arr -= s                # was array([-4, -3]), in place
    np.int16(2) << s        # was np.int16(64); graphblas has no `<<`

On numpy < 2, where a size-1 array beside a Scalar is a literal (on the
right too), `np.array([1]) - s` is now an INT64 ScalarExpression.

Ufuncs still use __array__: np.sin(s), np.subtract(np.int16(2), s) and
np.asarray(s) give numpy values as before.

* Type a literal on the left of a Scalar op as the op's left operand

_literal_scalar types a literal on either side of a Scalar op, as in
op(1 & s) or 1 - s, through _literal_operand, which now takes left=True
for a left operand: a typed op takes each operand in its own input type
("Type a literal beside a typed op in the op's own input type", in #591).
_literal_scalar did not pass it, so with an op typed for two different
types, a literal on the left was typed as the right operand. Every
left-side caller passes argname="left", so _literal_scalar passes
left=argname == "left". An untyped op, as every infix operator is, types
the literal the same either way.

Behavior change (before -> after), with A a record of two float64 fields,
s = Scalar.from_value((3.0, 4.0), A) and add_a a UDT BinaryOp of
x + y.ll_a:

- add_a[INT64, A](2 & s): GraphBLAS's DomainMismatch (2 became an A element
  where the op takes an INT64) -> 5.0.

* Tidy the helpers for a literal operand of a Scalar infix expression

No behavior change.

- ScalarInfixExpr._call_left is a method that falls back to
  InfixExprBase._call_left, in place of a module function assigned on
  ScalarEwiseAddExpr and ScalarEwiseMultExpr.
- _typed_operands asks _is_literal of each operand, without a check for
  a Scalar expression first: a Vector or Matrix infix expression never
  holds a literal.
- formatting._is_bool, which had one caller, is folded into
  _computes_itself.
- call_op names the method for an error only where a literal is on the
  left.
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.