Repository navigation
Check that imported values have an element for every entry - #643
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
==========================================
+ Coverage 98.16% 98.75% +0.58%
==========================================
Files 105 105
Lines 30578 30670 +92
Branches 3600 3607 +7
==========================================
+ Hits 30018 30289 +271
+ Misses 327 245 -82
+ Partials 233 136 -97 🚀 New features to boost your workflow:
|
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 19, 2026 07:55
d4703df to
f11c703
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 20, 2026 22:28
f11c703 to
2e69171
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
October 21, 2026 04:14
2e69171 to
1e9240e
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
2 times, most recently
from
October 28, 2026 04:29
0e0e155 to
f5d2961
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
2 times, most recently
from
October 29, 2026 02:01
842d327 to
cdfaec6
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
2 times, most recently
from
November 3, 2026 12:10
4b91734 to
a8136c3
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
November 3, 2026 16:44
a8136c3 to
125101d
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
November 3, 2026 20:07
125101d to
3d834e2
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
November 4, 2026 18:59
3dc1a1e to
40427e0
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
from
November 4, 2026 20:50
40427e0 to
2c5dff3
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
3 times, most recently
from
November 9, 2026 15:51
1a09737 to
e023593
Compare
eriknw
force-pushed
the
47c-import-values-length
branch
8 times, most recently
from
November 11, 2026 01:58
df42073 to
cb05b74
Compare
SuiteSparse compares the byte size of an imported values buffer with the
number of entries, not with the bytes they need (GB_import.c sets
Ax_memsize_for_non_iso to nvals or vlen * vdim), so any buffer of at least
nvals bytes got through and was read past its end:
Vector.ss.import_full(values=np.ones(3), size=5)
built a Vector whose last two values were heap garbage, and so did values
too short for a sub-array dtype, as in
Vector.ss.import_sparse(size=3, indices=[0, 1], values=np.ones(2), dtype="FP64[2]")
Every ss import and pack (sparse, bitmap and full Vectors; csr, csc,
hypercsr, hypercsc, bitmapr, bitmapc, fullr and fullc Matrices, and so
from_dcsr and from_dcsc) now raises
ValueError: values must have 5 or more elements of dtype FP64, not 3
before handing GraphBLAS the buffer. An iso import needs one element.
…lues SuiteSparse:GraphBLAS 10.5.2 fixed the import size check that check_values_size works around: GB_import.c compared the byte size of the values with the number of entries. With check_values_size bypassed, the five short imports tried (import_full, import_sparse of FP64[2], import_bitmapr, and two import_csr) now raise InvalidValue with an empty message on 10.5.2, where 10.5.1 built them from bytes past the end. check_values_size still runs on every version: older ones need it, and its ValueError says what is wrong. Its docstring and the comments of the two test_ss_import_values_size tests now give the versions. Comments only.
Matrix.from_dcsr([0, 1], [0, 1, 2], [0, 1], 1.5, "FP64[2]") gave [[1.5, 1.5], [1.5, 1.5]] without SuiteSparse, and on SuiteSparse raised "values must have 1 or more elements of dtype FP64[2], not 0": its SuiteSparse branch imported the scalar as it was, 8 bytes where one element is 16. It now broadcasts a scalar to one whole element and imports it iso, as from_csr and from_csc do; the same for from_dcsc. check_values_size also says what is wrong in two cases it misreported: - A scalar that does not fill one element of a sub-array dtype, as in Vector.ss.import_full(values=1.5, size=3, dtype="FP64[2]", is_iso=True): "a scalar does not fill one element of dtype FP64[2], of shape (2,)" (it said "not 0"). - Index pointers that give more entries than there are indices, as in import_csr(nrows=1, ncols=3, indptr=[0, 2**62], col_indices=[0], values=[1.0]): "index pointers give 4611686018427387904 entries, more than the number of indices, 1" (it asked for that many values). GraphBLAS rejects such an indptr itself, with an InvalidValue and no message, so the check comes first.
import_fullr(values=1.5, nrows=2, ncols=2) said "a scalar does not fill one element of dtype FP64", though 1.5 is a whole FP64 element and the problem is that four are needed. Keep that message for a scalar shorter than one element of a sub-array dtype. Found by review (r47abc).
eriknw
force-pushed
the
47c-import-values-length
branch
from
November 11, 2026 03:23
cb05b74 to
e464a64
Compare
Member
Author
|
LGTM |
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.
SuiteSparse:GraphBLAS before 10.5.2 checked the size of an imported values buffer in the
wrong units:
GB_importcompared a count of entries (nvals, orvlen * vdimfor bitmap andfull) with
Ax_memsize, which is in bytes. So any buffer of at least nvals bytes wasaccepted, and later reads ran past its end:
returned a Vector whose last two values were heap garbage, and so did values too short for a
sub-array dtype:
10.5.2 fixed that check (SuiteSparse PR #470); there these imports raise InvalidValue with an
empty message.
"Check that imported values have an element for every entry." Every ss import and pack
now checks the values before handing them to GraphBLAS: sparse, bitmap and full Vectors; csr,
csc, hypercsr, hypercsc, bitmapr, bitmapc, fullr and fullc Matrices; and so
Matrix.from_dcsrand
from_dcscon SuiteSparse. They raiseThe check is one helper,
check_values_size, next to the previous PR'scheck_element_shape. It counts what GraphBLAS will read: nvals,indptr[n], the size, ornrows x ncols. An iso import needs one element. A larger buffer is still accepted. It stays
for SuiteSparse 10.5.2 and later too, for its message ("Note that SuiteSparse:GraphBLAS 10.5.2
checks the size of imported values").
"Fill one element with a scalar in from_dcsr and from_dcsc on SuiteSparse."
Matrix.from_dcsr([0, 1], [0, 1, 2], [0, 1], 1.5, "FP64[2]")gave[[1.5, 1.5], [1.5, 1.5]]on suitesparse-vanilla but raised on SuiteSparse. It now broadcasts the scalar toone element and imports it iso, as
from_csrdoes.check_values_sizealso says what is wrong when a scalar does not fill one element of asub-array dtype.
problem. Before, they asked for that many values; GraphBLAS raises an InvalidValue with no
message for them.
Tests:
test_ss_import_values_sizein test_vector.py and test_matrix.py (SuiteSparse only)covers FP64 and
FP64[2]; sparse, bitmap and full formats; an import and a pack; iso importsgiven one element; a scalar for a sub-array dtype; and an indptr past the indices. With the
check bypassed, both fail on 10.5.2 too (InvalidValue does not match), so they test this
check, not SuiteSparse's.
test_ss_import_exportalso checks the second half of a stackedbitmap import. from_dcsr/from_dcsc with a scalar are tested on both backends.