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 — Fix wrong answers from select by A.T, coo import and isclose by eriknw · Pull Request #668 · python-graphblas/python-graphblas

Conversation

@eriknw

@eriknw eriknw commented Nov 12, 2026

Copy link
Copy Markdown
Member

Taken from #663 so that it can be reviewed and merged before the release. These are wrong answers with built-in types, all on main and in 2025.2.0. Each commit has its test and gives before and after in its message.

Select by a transposed comparison as by its mask, not by the untransposed one (on main since #252). A.select(A.T == x) was rewritten to select.valueeq(A, x), dropping the transpose: with S holding (0, 1) = 1 and (1, 0) = 5, S.select(S.T == 1) gave (0, 1) = 1 instead of (1, 0) = 5, and a 2 x 3 matrix gave a result instead of DimensionMismatch. Only A.select(A == x) is rewritten now; anything else takes the mask path. With autocompute=False, A.select(A.T == x) now raises "Use .new()", as A.select(B == x) always has.

Silent data loss in coo import (on main since 2021). Count each row's entries when importing coo, so empty rows keep indptr whole: import_coor and import_cooc, and so import_any of export("coo", sort=True), "coor" or "cooc", moved entries to another row when a row (or column) other than the first was empty. They lost every entry when the last one was, and isequal still said True. An index at or beyond nrows or ncols now raises IndexOutOfBound, where it was written past the end of indptr. Read 2-D coo indices as a flat buffer, and count them with numpy without Numba keeps that working for indices given as a 2-D array, and without Numba.

isclose.

  • Say an infinity is close only to itself in isclose, as math.isclose does. Scalar.from_value(inf).isclose(1.0), .isclose(-inf) and the Vector and Matrix forms were True, with Numba and without (main since 2023); numpy and math.isclose say False.
  • Compute isclose on integers without wrapping in x - y or abs(x). UINT64 0 was close to 2**64 - 1, INT64 -2**63 to 2**63 - 1, and with Numba UINT8 5 and 6 were not close at rel_tol=0.5. Mixed integer types went wrong the same ways, and Scalar.isclose raised OverflowError for a UINT64 above INT64's maximum.
  • Keep the imaginary part of x - y in isclose without Numba: without Numba, a complex difference lost it.

Freed memory after a failed export. Clear an exported object's handle as soon as GraphBLAS frees it: an error after the export left the handle pointing at the freed object, so A.nvals read freed memory, and a collected copy was freed twice (which aborted the process on SuiteSparse 7.4).

Decision taken (easy to reverse)

decision why
isclose on two integers takes the exact difference math.isclose and numpy convert x and y to float first, so isclose(2**62 + 1, 2**62, rel_tol=0) is True for them and False here. The code already did this wherever nothing wrapped, and with zero tolerances isclose then equals isequal. Converting to float first instead is a one-line change.

Differences from the commits in #663

The select and isclose commits leave out what concerns code that is not on main: #663's UDT select test, and its branch of Scalar.isclose for an int beyond 64 bits. #663 keeps those, with a commit that gives that branch the infinity rule from here. "Export an iso UDT matrix as coo" stays in #663 too: it reads the value with the UDT iterator that #605 adds. The export-handle test imports NullPointer, which test_matrix.py did not import on main. The tree at the top of the stack is unchanged by the move.

Checked locally on SuiteSparse:GraphBLAS 10.5.2: the full suite with --runslow, without Numba on both backends, and with C scalars as CI's second pass makes them.

🤖 Generated with Claude Code

Sorry, something went wrong.

Uh oh!

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

All reactions
eriknw added 7 commits November 11, 2026 23:16
@eriknw
Select by a transposed comparison as by its mask, not by the untransp…
792204f
…osed one

select._match_expr rewrote A.select(A.T == x) as select.valueeq(A, x):
its clause "args[0] is parent or args[0].T is parent" matched the
transpose of the parent, and the rewrite then compared A itself, dropping
the transpose and, for a matrix that is not square, the dimension check:

    S = Matrix.from_coo([0, 1], [1, 0], [1, 5], nrows=2, ncols=2)
    S.select(S.T == 1)
    before: (0, 1) = 1
    after:  (1, 0) = 5, as S.select((S.T == 1).new()) gives
    B (2 x 3): B.select(B.T == 1)
    before: a 2 x 3 result
    after:  DimensionMismatch

The clause is now "args[0] is parent"; anything else takes the mask path.
On main since #252.
@eriknw
Clear an exported object's handle as soon as GraphBLAS frees it
14d747a
GraphBLAS frees the Matrix or Vector as it hands the arrays over in an
export. _export gave GraphBLAS a copy of the handle and cleared the
object's own handle only at the end, so an error in between, such as
psg's claim_buffer_2d raising for an array UDT, left that handle
pointing at the freed object:

    A.ss.export(format, give_ownership=True) raising after the export,
    then A.nvals
    before: UninitializedObject (a read of freed memory)
    after:  NullPointer, as after an export that succeeds

A plain export, of a copy, also freed that copy a second time when it
was collected, which can abort the process (it did on SuiteSparse 7.4).
GraphBLAS now gets the object's own handle, which it sets to NULL.
@eriknw
Say an infinity is close only to itself in isclose, as math.isclose does
c849b87
isclose used math.isclose's formula without its infinity check. With an
infinite x or y, abs(x - y) is inf and so is rel_tol * max(abs(x), abs(y)),
so infinity was close to every number, in binary.isclose (Vector and
Matrix isclose with Numba, Scalar.isclose) and in _isclose_recipe (without
Numba):

    Scalar.from_value(inf).isclose(1.0), .isclose(-inf)
    Vector.from_coo([0], [inf]).isclose(Vector.from_coo([0], [1.0]))
    before: True (all three)
    after:  False, as math.isclose and numpy.isclose say

Two infinities of the same sign are equal, and so still close; NaN is
still close to nothing.
@eriknw
Compute isclose on integers without wrapping in x - y or abs(x)
f78f520
isclose computed abs(x - y) and abs(x) in the integer type, where they
wrap: in UINT64, and with Numba in uint64 for any unsigned type (Numba
widens to 64 bits); in INT64 at its extremes, as is abs(-2**63); and
without Numba in every integer type. So, for Vector, Matrix and Scalar:

    UINT64 [0] and [2**64 - 1]:            before True, after False
    UINT8 or UINT32 5 and 6, rel_tol=0.5:  before False, after True
    INT64 -2**63 and 2**63 - 1:            before True, after False
    INT8 -128 and 127, rel_tol=0.5:        before True without Numba,
                                           after False

and Scalar.isclose raised OverflowError for a UINT64 value above INT64's
maximum, because Numba typed the Python int as int64. The difference is
now exact (in uint64, for an integer type), the magnitudes are in FP64,
and Scalar.isclose passes its values in the op's input types, as
ewise_mult does. Mixed types are compared in the type they unify to, as
before, so INT64 with UINT64 is in FP64.
@eriknw
Keep the imaginary part of x - y in isclose without Numba
72486ab
_isclose_recipe, which Vector and Matrix isclose use without Numba,
stored x - y in an FP64 Vector before taking abs, and so kept only the
real part of a complex difference:

    Vector.from_coo([0], [0j]).isclose(Vector.from_coo([0], [1j]))
    before: True without Numba (False with it)
    after:  False, as cmath.isclose says

abs is now taken in the complex type first. abs(x) and abs(y) are taken
in the type x - y is in, as the Numba op does, so an FC32 operand beside
an FP64 one is not rounded to FP32 first.
@eriknw
Count each row's entries when importing coo, so empty rows keep indpt…
4f9c829
…r whole

import_coor and import_cooc build CSR (or CSC) index pointers with
indices_to_indptr, which set indptr only at the rows that appear. Every
empty row after the first one left a 0 in indptr, so an empty row in
the middle moved entries to another row, and an empty last row lost
every entry. Exporting as "coo" with sort=True, as "coor" or as "cooc"
and importing back hit it, on main since 2021:

    A = Matrix.from_coo([0, 2], [1, 0], [1.0, 2.0], nrows=3, ncols=2)
    B = Matrix.ss.import_any(**A.ss.export("coo", sort=True))
    before: B holds (2, 1) and (2, 0), and A.isequal(B) said True
    after:  B holds (0, 1) and (2, 0), as A does
    A = Matrix.from_coo([0, 0], [0, 1], [1.0, 2.0], nrows=3, ncols=2)
    before: the round trip gives an empty Matrix
    after:  both entries

It now counts the entries of each row, then sums them. The indices are
converted to uint64 first, so lists work, and one at or beyond nrows (or
ncols) raises IndexOutOfBound, where it was written past the end of
indptr with no bounds check under Numba.
@eriknw
Read 2-D coo indices as a flat buffer, and count them with numpy with…
f1617b8
…out Numba

The new indptr kernel for import_coor and import_cooc indexed a 2-D rows
(or cols) array by its rows, past the end of its first axis:

    rows = np.array([[0, 1], [1, 2]], np.uint64)
    Matrix.ss.import_coor(rows=rows, cols=[0, 0, 1, 1], values=[1, 2, 3, 4],
                          nrows=3, ncols=2)
    before: Segmentation fault with Numba; IndexError without it
    after:  the Matrix from_coo(rows.ravel(), ...) gives, as other
            imports read 2-D indices

Without Numba the kernel ran in Python, once per entry and once per row
(1.1 s for 10**7 rows and 10**5 entries, where the old kernel took 12 ms).
It now counts with np.bincount and sums with np.cumsum (20 ms).
@eriknw
eriknw added this pull request to stack #670 November 12, 2026 06:17
@eriknw eriknw mentioned this pull request Nov 12, 2026
Fix UDT, dtype and JIT edge cases found in #591's review #663
Open
@codecov

codecov Bot commented Nov 12, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.14660% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.12%. Comparing base (d66165a) to head (f1617b8).

Additional details and impacted files
@@                  Coverage Diff                   @@
##           27-check-mask-move     #668      +/-   ##
======================================================
- Coverage               99.12%   98.12%   -1.01%     
======================================================
  Files                     105      105              
  Lines                   31420    31595     +175     
  Branches                 3661     3689      +28     
======================================================
- Hits                    31145    31002     -143     
- Misses                    175      335     +160     
- Partials                  100      258     +158     
🚀 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.

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.