Conversation
`blockdiag` renumbered the right-hand graph's free slots along with its
real neighbors, turning every `NONEIGHBOR` marker into a valid vertex
index. The slots were leaked permanently, and `nde` silently went wrong:
a later `rem_vertices!` recounts by scanning for markers and would see
six edges where there were four.
The same operation leaves every vertex of the right-hand graph with an
offset into the middle of the edgelist, which broke `_add_directed_edge!`.
It used `isone(idx)` to mean "this vertex has no neighborlist yet", which
only holds while `g.v[i]` is still zero, so a degree-0 vertex planted its
list on top of somebody else's. It now decides on `g.d[i]`, and checks
the vertex's own next slot first so the common cases keep their locality.
`rem_vertices!` validated nothing up front, and `deleteat!` throws partway
through its compaction rather than before it, so unsorted or repeated
indices left the graph in pieces. The indices are checked before anything
is mutated.
Two allocation sizes were wrong. An undirected graph stores each edge in
both directions, but the conversion and edge-list constructors reserved
room for one, so every reverse edge grew the list by an O(E) shift. The
matrix constructor sized itself with `sum(isone, A)` while inserting on
`A[i, j] != 0`, so a weighted matrix reserved nothing at all. Vertex
offsets were also built by `zeros(n)`, a `Vector{Float64}` that was
converted to `Csize_t` and thrown away.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three paths were superlinear in the graph size. Starting a vertex's neighborlist rescanned the whole edgelist for a free slot, so building a graph cost O(n*E) even when the vertices were filled in order. A `_freeslot` cursor records that no slot before it is free, which every operation that frees or fills one maintains, making the total scan O(E). Building from an edge list was quadratic on top of that, and sorting the input did not help: an undirected graph stores each edge in both directions, and the reverse edges always arrive out of order, so each one shifted the whole edgelist. The neighborlists are now laid out in a single pass over the sorted, deduplicated directed edges. `rem_vertices!` made one full pass per removed vertex, plus an O(E*k) renumbering sweep. It now takes one pass: a lookup table gives each surviving vertex its new index, and the surviving neighborlists are gathered into a packed edgelist. Renumbering is what needs the table -- a neighborlist is in no particular order, so each entry's shift has to be looked up rather than counted along. Also drops a `has_edge` call from the undirected edge iterator. The neighbor was read out of the vertex's own list, so the check was always true, and it cost an O(degree) scan per edge. At n=4000: building from a graph 10.4 ms -> 0.08 ms, from an edge list 20.0 ms -> 0.33 ms, and removing a quarter of the vertices 3.9 ms (at n=2000, where the old code was already too slow to extend) -> 0.04 ms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rem_vertices!` rebuilt the edgelist on every call, which gave back the freed memory but allocated a replacement each time. It now writes each surviving neighborlist back over itself by default and leaves what it frees as slots for later insertions, so the edgelist is never replaced. `compact=true` asks for the old behavior, which is still worth it when the freed memory matters more than the allocation. Both modes run the same loop and differ only in where a surviving list is written, so neither is faster than the other; the choice is only about memory. The renumbering table is the one thing left that has to be allocated. It is now a `buffer` argument defaulting to a fresh one, so a loop that shrinks a graph repeatedly can hoist it and allocate nothing at all: 1000 removals on a graph with 20000 vertices go from 98 MB to zero. The table is also built by walking `inds` alongside the vertices, writing every entry, so a reused buffer needs no clearing first, and it is `Cint` rather than `Int` since nauty caps the vertex count there anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_rem_vertices!` carried a sortedness check that could never fire: `lastind` was initialized to zero and never reassigned, so `ind < lastind` was false on every iteration. What actually rejected unsorted indices was `deleteat!`, and it throws partway through its own compaction rather than before it, so the graphset was left with rows already removed and the column shifts never applied. The indices are now checked up front. This also makes the dense `rem_vertices!` atomic, since it deletes the labels only after `_rem_vertices!` returns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`increase_padding!` inserted the new padding words one at a time, and each `insert!` memmoves the whole array, so widening a graphset cost O(n^2*m). Growing a graph one vertex at a time triggers that every `wordsize(W)` vertices, making `add_vertex!` O(n^4) overall: 500 vertices took 0.12 ms and 4000 took 535 ms. It now resizes once and spreads the rows apart back to front, so a row is only ever moved into space its successor has already vacated. The same 4000 vertices take 1.4 ms, and 8000 take 10 ms where they would previously have taken around nine seconds. `_add_vertices!` also built a throwaway array to append zeros with, which `resize!` and `fill!` do without one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`_rem_vertices!` shifted the whole graphset left by one bit for every removed vertex, so removing k of them cost O(k*n*m). Removing half the vertices of a graph on 4000 took 154 ms, and the cost grew as n^3. The shift a surviving column needs is not uniform across a row: it is a step function with one step per removed column, which is why `partial_leftshift` had to be applied once per removal. Moving whole runs of surviving columns instead needs a different primitive, one that copies an arbitrary bit range from one offset to another. `_readbits`, `_writebits!` and `_movebits!` provide that, and `partial_leftshift`, which had no other caller, goes away. The cost is now O(n*(m+k)) and the same removal takes 9 ms. Single-vertex removal regresses from 0.32 ms to 0.51 ms at n=4000, because that is the one case the old primitive handled with aligned whole-word shifts where this does unaligned run copies. Vacated columns are explicitly cleared, since `active_words` and nauty both read whole words and would otherwise see stale bits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`blockdiag` assembled its result into a `Graphset` and handed it to
`DenseNautyGraph{D,W}(gset; vertex_labels)`. There was no method taking a
`Graphset` for the two-parameter form, only for `DenseNautyGraph{D}`, so
the call landed on the `AbstractMatrix` constructor instead. That ran an
O(n^2) `issymmetric` check and then rebuilt the whole graphset one bit at
a time through `getindex`/`setindex!`, throwing away the one it had just
been given.
Adding the missing method is most of the fix. The blocks are also copied
with the bit-range primitives now, whole words at a time, rather than by
broadcasting over the destination; graphs of differing word types keep
the elementwise path, which is the only thing that works there.
`blockdiag` of a graph on 4000 vertices with itself goes from 158 ms to
37 ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`compact` reads as a description of the result rather than something the call does. `compactify` says it performs an action, which is what the keyword actually selects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removing vertices shrinks a graphset's height but never its width, so a graph that has shrunk keeps as many words per vertex as it had at its largest. Going from 8000 vertices to 200 leaves 125 words per vertex where 4 would do: 25000 words instead of 800. That width is read by every later nauty call and every hash, so a shrunk graph stays slower and heavier than the same graph built directly -- `canonical_id` on it allocated 239632 bytes against 3792 for its equal. `decrease_padding!` and `minimize_padding!` were stubbed out; they now close the gaps between rows front to back, mirroring `increase_padding!`. `resize!` hands back the length but keeps the buffer, so a large drop takes a new array rather than holding the old one. `rem_vertices!` takes `compactify` to ask for this, matching the sparse format's keyword. It defaults to false, so nothing changes unless asked. Keeping the width is not pure waste: it is reserved capacity. A graph that grows back reuses it and allocates nothing, where a compactified one has to widen and reallocate. The two formats differ only in what holding that capacity costs. Free slots in a sparse edgelist are never read, while graphset padding is read on every nauty call and hash, so `compactify` is the right call for a graph that stays small and the wrong one for a graph that grows back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ne` on an undirected sparse graph counted self-loops by asking `has_edge(g, i, i)` for every vertex, which walks that vertex's neighborlist, so an accessor Graphs.jl treats as O(1) was O(E). It ran in 62 us on a graph with 16000 vertices, and `show` and the edge-iterator comparisons all go through it. The count is now maintained on the graph, alongside `nde`, and `ne` reads it. `indegree` on a directed sparse graph cannot be made better than O(E): the format stores only forward adjacency, so nothing indexes in-edges, and finding them means looking at all of them. What it does not need is a `has_edge` scan per vertex. Every entry of the edgelist that names `v` is an edge into `v`, and free slots hold `NONEIGHBOR` and never match, so one flat pass over the edgelist answers it. That is 5.7x faster at n=8000 and still O(E). `inneighbors` is left alone. It is already lazy, and turning edgelist positions back into source vertices would need a search through `v`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #76 +/- ##
==========================================
- Coverage 95.21% 94.81% -0.41%
==========================================
Files 8 8
Lines 941 1080 +139
==========================================
+ Hits 896 1024 +128
- Misses 45 56 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`hash`, `==`, `collect(edges(g))` and `canonical_id` all sorted the graph's
neighborlists on the way out, through nauty's `sortlists_sg`. That made
them mutations: two tasks hashing one graph, or querying a shared
`Set{SpNautyGraph}`, raced on `g.e` while a C qsort was running over it.
Sorting is now an invariant the mutating operations maintain, so the read
paths leave the graph alone. Adding an edge sifts the new entry into place,
which costs nothing when the neighbors arrive in order and is what every
constructor does; removing one closes the gap rather than swapping the last
entry into it; and `_unsafe_copyfromsparsegraphrep!` sorts once, since
nauty hands back its own layout. Construction is unchanged at 0.09 ms for
16000 vertices from a graph and 0.33 ms from an edge list.
This also removes the repeated work. The lists were re-sorted on every
read, which cost 89 us on a graph with 16000 vertices, about an eighth of
a full hash, however many times in a row it was called.
An `issorted` flag would have skipped the redundant sorting but not fixed
the race, since the first read after any modification still mutated. The
invariant needs no flag.
Neighbor order is now stable across reads. It was not before: sorting
reordered `outneighbors` on 297 of 400 random graphs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The graph types are meant to be safe to read from several tasks at once, and the tests that check that spawn tasks. Under the default single thread they run one after another, so they confirm that reading a graph is deterministic and leaves it alone but never actually overlap two readers. `JULIA_NUM_THREADS` reaches the process `Pkg.test` spawns, so setting it on the test step is enough. It is a matrix axis rather than a fixed value, so single-threaded runs stay covered too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mxhbl
commented
Aug 24, 2026
|
|
||
| Remove vertex `i` from `g`. Return `false` without modifying `g` if `i` is not a vertex of `g`. | ||
|
|
||
| See [`rem_vertices!`](@ref) for what `compactify` does. |
Member
Author
There was a problem hiding this comment.
Suggested change
| See [`rem_vertices!`](@ref) for what `compactify` does. | |
| See also [`rem_vertices!`](@ref). |
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
1 participant
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.
Significant speedups for graph accessor/modify functions.