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 — Convert from networkx without the scipy detour by eriknw · Pull Request #600 · python-graphblas/python-graphblas Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@ ## 32-udf-typing-only-types #600 +/- ## ========================================================== Coverage 98.88% 98.89% ========================================================== Files 106 106 Lines 32721 32884 +163 Branches 3804 3823 +19 ========================================================== + Hits 32355 32519 +164 + Misses 237 236 -1 Partials 129 129 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
from_networkx always routed nx -> scipy CSR -> Matrix, so every conversion paid for scipy.sparse (importing it alone costs over 100ms) plus an extra coo -> csr materialization. Simple graphs with numeric weights now build COO arrays straight from the nx adjacency and call Matrix.from_coo. The node selection preamble mirrors nx.to_scipy_sparse_array so ordering, subsetting, and error behavior match exactly; undirected graphs are symmetrized with the same diagonal correction nx uses (self-loop entries appear as wt + wt - wt under dup_op=plus, which is exact in IEEE arithmetic). Kept on the scipy fallback automatically: multigraphs (scipy's duplicate-coordinate summation matches exactly) and weights that do not form a 1-D numeric array. That second condition covers non-numeric attributes, including all-string weights, which infer a <U dtype rather than object; the fallback raises the identical ValueError. It also covers array-valued weights: sequence attributes of uniform length infer a 2-D numeric array, which passes a dtype-kind test but which from_coo would read as a UDT, so they keep raising ValueError as before rather than quietly widening what from_networkx accepts. graphblas without scipy installed can now convert simple graphs (previously ImportError); multigraphs still need scipy.
A networkx multigraph fell back to scipy in from_networkx because the direct path could not sum the weights of parallel edges. The directed branch now passes dup_op=plus when the graph is a multigraph, so parallel edges accumulate exactly as scipy's coo -> csr summation does. Simple graphs keep dup_op=None and are unchanged, and the undirected branch already summed because it uses plus for the self-loop diagonal correction. Numeric multigraphs no longer need scipy at all; non-numeric weights still defer to it. Parity with the retained scipy path holds for MultiGraph and MultiDiGraph with parallel edges, parallel self-loops, reciprocal parallel edges, parallel edges whose weights cancel to zero (both sides keep the explicit zero), absent weight attributes (nx defaults to 1), weight=None, bool and int weights, and nodelist permutations and subsets. New tests pin the multigraph diagonal sum and assert the numeric path never reaches the scipy fallback.
Converting through scipy ended in ``from_scipy_sparse``, which checks whether every value is the same and imports one value instead of nvals of them. The direct path added here ends in ``from_coo``, which does not, so an unweighted graph quietly started storing a full values array. Nothing caught it because SuiteSparse was still doing its own post-build iso check, and the result came out iso either way. 10.5.0 dropped that check, at which point ``test_matrix_to_from_networkx`` failed and the regression became visible. The memory it costs was there the whole time. Ask for iso outright, under the same uniformity test ``from_scipy_sparse`` uses. Multigraphs are excluded because ``plus`` sums parallel edges and can make uniform inputs non-uniform. Undirected graphs need no exclusion: without self-loops their coordinates are unique so ``plus`` never fires, and with them the diagonal correction puts both ``wt`` and ``-wt`` in the values array, which fails the uniformity test on its own.
The widening added while stabilizing CI was marked "pragma: no cover (win64 numpy < 2)", but the branch is reachable on every platform: np.int32 scalar edge weights infer an int32 array from the edge sequence anywhere, not only where python ints do. The pragma therefore hid live code from coverage, and the comment implied the change was platform-local when it is user-visible everywhere: int32-weighted graphs that previously round-tripped through scipy as INT32 now come back INT64. The widening itself is kept, deliberately. from_coo widens an int32 dtype inferred from sequence input to int64 on every platform (values_to_numpy_buffer), and from_networkx builds its values from a Python sequence, so identical weights now produce the same dtype through either constructor. Gating the widening to win64 instead would have re-introduced a from_coo/from_networkx disagreement everywhere else. Drop the pragma, restate the comment in terms of the real contract, and pin the behavior: np.int32 weights widen to INT64 and agree with from_coo, an explicit dtype= is preserved exactly, and np.int16 stays INT16 through both constructors. Release-note item: the from_networkx result dtype for int32-weighted graphs changes from INT32 to INT64.
The direct ingestion path symmetrized an undirected graph the way nx.to_scipy_sparse_array does: it mirrored every entry, self-loops included, and appended -wt for each self-loop so that plus would sum the diagonal triple wt + wt - wt back to wt. That arithmetic had four consequences: - The iso test added in a1fa8f3 reads the values array, which then held both wt and -wt, so an unweighted (or uniformly weighted) undirected graph with a self-loop was stored with a full values array. Through scipy it was stored iso, because from_scipy_sparse tested the summed values. Code that branches on A.ss.is_iso (graphblas-algorithms runs BFS instead of Bellman-Ford for an iso graph) took the slow path. - An infinite self-loop came back nan (inf + inf - inf), and a finite one near the float max came back inf. - An unsigned dtype= raised OverflowError under numpy 2, which cannot write -wt in it. - bool weights came back INT64 when the graph had a self-loop (-True is the int -1) and BOOL when it did not. A self-loop is its own mirror, so mirror only the off-diagonal entries. Every stored value is then an edge's own weight, and a simple graph has no repeated coordinate: it builds without an accumulator, and a uniform one takes the iso path whether or not it has self-loops. Multigraphs still sum parallel edges and parallel self-loops with plus. The second pass over the graph for nx.selfloop_edges goes away. nx.to_scipy_sparse_array still sums wt + wt - wt, so from_networkx now differs from it in exactly those four cases. Compared with the parent over 74 graphs (empty, edgeless, isolated nodes, nodelist subsets and reorders, non-integer keys, int, float, bool, complex, int32, missing and nan weights, multigraphs, and the error cases), nothing else changes.
The old diagonal correction negated each self-loop's weight. For an np.uint8 weight that wraps (-np.uint8(1) is 255), and beside a Python int weight the values infer int64, so the self-loop of weight 1 summed to 1 + 1 + 255 = 257, with only a RuntimeWarning. The parent commit fixed this along with the four cases its message lists; pin it too.
from_networkx checks a nodelist as nx.to_scipy_sparse_array does, so that the direct path raises its errors without needing scipy, but no test gave it an empty nodelist, a node not in the graph, or a duplicate. Check each message, and that a missing node is reported before a duplicate.
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.