Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 47-ss-export-full-message #641 +/- ##
=============================================================
+ Coverage 98.65% 98.66% +0.01%
=============================================================
Files 105 105
Lines 29280 29389 +109
Branches 3447 3446 -1
=============================================================
+ Hits 28885 28997 +112
- Misses 252 257 +5
+ Partials 143 135 -8 🚀 New features to boost your workflow:
|
eriknw
force-pushed
the
47a-take-ownership-allocator
branch
2 times, most recently
from
October 20, 2026 22:28
fea0693 to
59c75dc
Compare
eriknw
force-pushed
the
47a-take-ownership-allocator
branch
2 times, most recently
from
October 21, 2026 08:38
78db402 to
3c9e06a
Compare
eriknw
force-pushed
the
47a-take-ownership-allocator
branch
from
October 28, 2026 04:29
3c9e06a to
41926fe
Compare
eriknw
force-pushed
the
47a-take-ownership-allocator
branch
from
October 28, 2026 13:21
41926fe to
61513fb
Compare
A take_ownership import gave GraphBLAS any input array with owndata and writeable set. owndata says the array owns its data, not which allocator made it: an array from a custom NumPy memory handler (NEP 49) owns its data too, and GraphBLAS frees what it owns with the allocator it was initialized with. With suitesparse-graphblas 10.5.1.1, NumPy 2.5 and PYTHONMALLOC=debug, importing such an array (one from libc's malloc, carrying its own handler) raised ValueError from unclaim_buffer after GraphBLAS had taken it, and the process aborted when the object was freed. suitesparse-graphblas 10.5.1.1 adds can_unclaim_buffer, which also compares the array's memory handler with GraphBLAS's. Ask it when it exists, and keep the owndata check for older versions. When an array fails the check, copy it into memory from psg's empty, which GraphBLAS allocated, rather than with NumPy: with memory_manager="c" (if psg was initialized before python-graphblas) GraphBLAS frees with libc's free, so a NumPy copy is no better. Before this, with psg 10.5.1.1 and memory_manager="c", every ss import raised that ValueError, including take_ownership=False, whose copy NumPy made. The tests import arrays from a GraphBLAS arena given libc's allocator (a real foreign memory handler, built without compiling anything) and check that they were copied, check that the import asks can_unclaim_buffer, and run imports under memory_manager="c" with PYTHONMALLOC=debug in a subprocess. All three fail on the parent commit with psg 10.5.1.1, and pytest itself then aborts at exit. psg also offers give_buffer, a context manager that copies when needed, commits only if GraphBLAS took the pointer, and unclaims. It would replace this and every unclaim_buffer call, but each import already unclaims only after check_status, so it adds no safety here yet; it restructures all eleven import paths and needs psg 10.5.1.1.
GraphBLAS frees every buffer an import hands it. The imports copied the values when they were the same array as one index array (col_indices, row_indices, indices or bitmap), but not other pairs: the same uint64 array as indptr and col_indices of import_csr, or as rows and col_indices of import_hypercsr, went to GraphBLAS twice, and freeing the Matrix freed it twice (exit 134 on mlx; with suitesparse-graphblas 10.5.1.1 the second unclaim_buffer raises ValueError first). Check every pair, and make the copy as the parent commit does, in memory GraphBLAS can free (np.copy gave a NumPy copy, which GraphBLAS can't own with memory_manager="c"). The test imports csr, hypercsr and hypercsc with an index array given twice; on the parent commit it fails and the process aborts.
NumPy frees the data of an array that owns it but has no NEP 49 memory handler with libc free, which is right only when libc's malloc made it. GraphBLAS allocates with NumPy's allocator, which on NumPy 2.5 is PyMem_RawMalloc, so such a free is wrong under PYTHONMALLOC=debug and on free-threaded Python. NumPy can warn whenever it does one; turn that on for the test session (the same guard suitesparse-graphblas's own tests use), so filterwarnings = error fails the test even where the two allocators happen to agree. The setter is private: numpy._core._multiarray_umath from NumPy 1.26, numpy.core._multiarray_umath on 1.25, and absent on 1.24, where the guard does nothing. It also does nothing with suitesparse-graphblas before 10.5.1.1 (no can_unclaim_buffer), whose claim_buffer makes exported arrays without a handler, so every export test would fail. With suitesparse-graphblas 10.5.1.1, NumPy 2.5.4 and Python 3.15 the whole suite passes with the guard on (2165 passed, 11 skipped with --runslow).
A regression there frees an array twice, and the abort took down the whole pytest run (exit 134) instead of failing the test. Run the imports in a child process that imports this tree, as test_import_with_libc_allocator does, through a helper both tests now share. The child's output is unbuffered, so the report after a crash still shows which graphblas it ran.
…version Without can_unclaim_buffer and empty (suitesparse-graphblas < 10.5.1.1), a take_ownership import gives GraphBLAS an array that owns its data and is writeable, and a NumPy copy of any other. CI ran that rule only on draws of an older suitesparse-graphblas. The new test sets both names to None, as test_take_ownership_asks_suitesparse_graphblas sets the other, and checks that a read-only array and a view are copied and an exported array is taken. It fails if the writeable check or the copy is dropped. The import fallbacks themselves still run only on older versions.
The fallbacks for suitesparse-graphblas < 10.5.1.1, which has neither can_unclaim_buffer nor empty, in utils.py and test_ss_utils.py run only in CI jobs that draw an older version, so Codecov counted them hit or missed by draw. Mark them "import", as the other import fallbacks are; the MAINT note moves to its own line. _libc_arena initializes GraphBLAS's last arena with libc's allocator unless it already is, since GxB_arena_init refuses a second time. Its one caller runs once per process, so the other arm runs only when the test is repeated in the same process.
eriknw
force-pushed
the
47a-take-ownership-allocator
branch
from
October 29, 2026 02:01
61513fb to
4d473ee
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A
take_ownership=Trueimport handed GraphBLAS any input array withowndataandwriteableset.owndatasays an array owns its data, notwhich allocator made it: an array from a custom NumPy memory handler (NEP 49)
owns its data too, and GraphBLAS frees what it owns with the allocator it was
initialized with. python-suitesparse-graphblas's letter of 2026-10-04 found
the process aborting (exit 134) when GraphBLAS freed one. Reproduced here
with psg 10.5.1.1, NumPy 2.5.4 and Python 3.15 under
PYTHONMALLOC=debug:an array from a GraphBLAS arena given libc's allocator (so it carries its own
memory handler) raised
ValueErrorfromunclaim_bufferafter GraphBLAS hadtaken it, and the process aborted in
Vector.__del__.Four commits:
Hand GraphBLAS only arrays its allocator can free. Ask psg's
can_unclaim_buffer(new in 10.5.1.1) when it exists, and keep theowndatacheck for older psg. An array that fails is copied into memoryfrom psg's
empty, which GraphBLAS allocated, not with NumPy: withmemory_manager="c"(psg initialized before python-graphblas) GraphBLASfrees with libc's
free, so a NumPy copy is no better. Before this, withpsg 10.5.1.1 and
memory_manager="c", every ss import raised thatValueError,take_ownership=Falseincluded. The take_ownershipdocstrings say what the array needs.
Copy an array given twice to a take_ownership import. GraphBLAS frees
every buffer it takes. The imports copied the values when they were the
same array as one index array, but not other pairs: the same uint64 array
as
indptrandcol_indicesofimport_csrwas freed twice (exit 134).Fail a test that frees an owned array with no memory handler. psg's own
conftest guard: NumPy's private
_set_numpy_warn_if_no_mem_policy(True)for the session, so
filterwarnings = errorfails such a test even wherethe allocators happen to agree. It is off on NumPy 1.24 (no setter) and
with psg before 10.5.1.1, whose
claim_buffermakes exported arrays withno handler: forced on with an older psg, 296 tests fail and 13 error,
every one on an array that
claim_buffermade. This commit is testinfrastructure only; it can be dropped on its own (its revert applies
cleanly and nothing else depends on it).
Run the one-array-twice import test in a subprocess. A regression there
is a double free, which would abort the whole pytest run (exit 134)
instead of failing one test; the child process imports this tree.
Tests.
test_take_ownership_of_foreign_memoryimports arrays from a libcarena (a real foreign memory handler, built without compiling anything) and
checks they were copied;
test_take_ownership_asks_suitesparse_graphblasmonkeypatches
can_unclaim_bufferto refuse and checks the copy;test_import_with_libc_allocator(slow) runs imports undermemory_manager="c"withPYTHONMALLOC=debugin a subprocess;test_take_ownership_of_one_array_twiceimports csr, hypercsr and hypercscwith an index array given twice (in a subprocess). All four fail on the
parent commit with psg 10.5.1.1. The first and third need psg
10.5.1.1 and are skipped with older psg.
Not done: psg's
give_buffercontext manager. It copies when needed,commits only if GraphBLAS took the pointer, and unclaims, so it would replace
this and all eleven
unclaim_buffersites. Every import already unclaimsonly after
check_status, so it adds no safety yet; it restructures alleleven import paths and needs psg 10.5.1.1 as a floor. Worth it once psg
10.5.1.1 is the minimum, or for the
GxB_IS_READONLYload path.