Preserve radius-graph connectivity under neighbour truncation - #2118
Open
teerthsharma wants to merge 1 commit into
Open
Preserve radius-graph connectivity under neighbour truncation#2118teerthsharma wants to merge 1 commit into
teerthsharma wants to merge 1 commit into
Conversation
teerthsharma
marked this pull request as ready for review
July 29, 2026 23:48
teerthsharma
marked this pull request as draft
August 9, 2026 21:14
teerthsharma
force-pushed
the
feat/topology-beta0-neighbor-selection
branch
3 times, most recently
from
August 10, 2026 08:35
6075c92 to
92c8f32
Compare
`get_max_neighbors_mask` keeps the nearest `max_neighbors` edges per atom.
That is a purely local rule, so it carries no guarantee about the graph as a
whole: where two dense regions touch through a single bridging contact, both
endpoints can rank that contact outside their own budget and drop it. The
bridge disappears by local agreement and a physically connected structure
reaches the model as two disconnected components, which nothing downstream
detects.
Measured on two fcc Cu grains 3.2 A apart at cutoff 6 A, the graph splits at
every budget tested from 8 to 30 - including the shipped `max_neighbors: 30` -
into two 32-atom fragments. Uniformly random dense periodic boxes never do
(0/400), so this needs bridge topology to show up.
Adds `preserve_connectivity`, off by default, which re-adds the shortest
dropped edges needed to restore the component count of the untruncated graph:
- component labelling by hooking plus pointer jumping, and Boruvka for the
bridges, both vectorized with `scatter_reduce_` so there is no Python loop
over edges and both run on GPU
- the whole pass is skipped when the retained graph already has one component
per system, which is the common case, since no edge joins two systems and
dropping edges can only add components
- contracting the retained components to the quotient graph bounds the repair
by the component count rather than the atom count, and bounds the merge
rounds by the deepest system rather than the batch
- selection reads only distances and keeps the whole degenerate shell, the
same way the non-strict neighbour budget already does, so the reserved
edges do not depend on atom ordering
- each Boruvka round claims an edge that is shortest for either endpoint;
requiring both would be a mutual-minimum rule under which no round is
guaranteed to merge anything and the round bound stops being a bound
- all three radius-graph versions share the mask, so all three are covered
- `eSCNMDBackbone` takes `preserve_connectivity` and forwards it, so a model
can actually reach the flag; it defaults to False and no shipped config
sets it
Across 16 structures x 4 budgets, 55 of 64 configurations fracture with the flag
off and 0 of 64 with it on. Edge growth is 0% on unbridged bulk and, on bridged
fcc contacts, 2.86-4.76% at `max_neighbors: 30` rising to 13.33% at 8. These are
perfectly symmetric grains, so the degenerate shell that selection deliberately
keeps whole is at its widest; they are an upper bound, not a typical case.
Cost on an RTX 4060 Laptop GPU, CUDA events, 200 timed iterations against the
same batch with the flag off: 1.079x for v1 and 1.050x for v2 on a 16-system /
1728-atom batch with nothing bridged, where the whole cost is the one labelling
pass that proves nothing needs repair, and 1.532x for v1 and 1.374x for v2 on a
16-system / 1552-atom batch with 4 systems bridged.
Mutual-kNN, the ESCAIP-style rule that keeps an edge only when each endpoint is
in the other's top k, fractures 56 of the same 64 configurations - worse than the
55 of plain truncation, since requiring both endpoints drops bridges the budget
alone would keep. Union-kNN gives 55/64, identical to plain truncation.
The correction is strictly an improvement: across 16 structures x 4 budgets the
retained edge set is a superset of the flag-off set 64/64 and the component count
equals the untruncated count 64/64, so no edge is ever removed or replaced and
the result never overshoots.
52 tests in tests/core/graph/test_beta0_neighbor_selection.py pass, including
rigid-motion invariance, uniform-scale equivariance, atom-order invariance,
superset-of-flag-off, bitwise reproducibility on both CPU and CUDA, strict
truncation, mixed-PBC batches and a single-atom system in the batch. A
mutation matrix over 9 targeted defects kills 7; the 2 survivors reduce
pointer-jump counts that the surrounding convergence and round loops already
backstop. An eSCN-MD forward on an RTX 4060 takes two fcc grains from 2 graph
components to 1 and a 4-block chain from 4 to 1, leaves a plain bulk control
edge-for-edge identical, and keeps embeddings and position gradients finite.
The graph shard fails 31/190 on this branch and 31/138 on upstream main at
a73c95b, and tests/core/models/uma fails 3/239 on both, with identical failure
sets; the 31 are Windows-local, from `nvalchemiops` being unavailable and from
checkpoint downloads.
This does not change the atom-order invariance of the existing selection.
`enforce_max_strictly=False` is already invariant; `enforce_max_strictly=True`
is not, and stays that way.
Signed-off-by: teerth sharma <teerths57@gmail.com>
teerthsharma
force-pushed
the
feat/topology-beta0-neighbor-selection
branch
from
August 10, 2026 09:27
92c8f32 to
c1630a3
Compare
teerthsharma
marked this pull request as ready for review
August 10, 2026 09:27
frostedoyster
self-requested a review
August 13, 2026 18:44
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.
Summary
get_max_neighbors_maskkeeps the nearestmax_neighborsedges per atom. Thatis a purely local rule, so it carries no guarantee about the graph as a whole:
where two dense regions touch through a single bridging contact, both endpoints
can rank that contact outside their own budget and drop it. The bridge
disappears by local agreement and a physically connected structure reaches the
model as two disconnected components, which nothing downstream detects.
This adds
preserve_connectivity, off by default, plumbed throughgenerate_graphandget_max_neighbors_mask. When set, it re-adds the shortestdropped edges needed to restore the component count of the untruncated radius
graph. Component labelling (hooking plus pointer jumping) and the bridge
selection (Boruvka) are both vectorized with
scatter_reduce_, so there is noPython loop over edges and both run on GPU.
eSCNMDBackbonetakespreserve_connectivityand forwards it into graphgeneration, so a model can reach the flag rather than it stopping at
generate_graph. That constructor keyword is the only public-API addition; itdefaults to False and no shipped config, checkpoint, or YAML sets it. Default
behaviour is unchanged: with the flag off, the mask, the edge list and the
reported neighbour counts are the same objects as before, and there is no head,
loss, or training-unit change.
The correction is strictly an improvement rather than a different answer. Across
16 structures x 4 budgets the retained edge set is a superset of the flag-off set
in 64/64 configurations and the component count equals the untruncated count in
64/64, so an edge the budget kept is never removed or replaced, and the repair
never overshoots into gluing a real vacuum gap.
The failure mode
Two fcc Cu grains 3.2 A apart, cutoff 6 A, 64 atoms:
max_neighborsAcross 16 bridged and unbridged structures x 4 budgets, 55 of 64 configurations
lose connectivity with the flag off and 0 of 64 with it on. Uniformly random
dense periodic boxes lose it in 0 of 400 trials, so the defect requires bridge
topology — adsorbates, grain boundaries, cluster contacts — and random stress
testing does not surface it.
Edge growth is 0% on unbridged bulk and, on bridged fcc contacts, 2.86-4.76% at
max_neighbors: 30, rising to 13.33% at 8. Those are perfectly symmetric fccgrains, so the degenerate shell that selection deliberately keeps whole is at its
widest; treat them as an upper bound rather than a typical case.
Each Boruvka round claims every edge that is shortest for either endpoint.
Requiring both would be a mutual-minimum rule, under which a component whose
shortest edge points at a component with an even shorter one elsewhere claims
nothing, no round is guaranteed to merge anything, and the round bound stops
being a bound. A chain of blocks with monotonically increasing gaps exhibits
exactly that and is covered by
test_deep_chain_is_fully_reconnected.All three radius-graph implementations (v1, v2, nvidia) call the shared mask, so
all three are covered by the one change.
Guarantees the selection keeps
Selection reads only distances and keeps the whole degenerate shell, the same
way the non-strict neighbour budget already does, so the reserved edges do not
depend on atom ordering: 0 of 54 permutation trials changed the reserved edge
set. This does not change the atom-order invariance of the existing selection —
enforce_max_strictly=Falseis already invariant,enforce_max_strictly=Trueis not and stays that way.
Structures that are genuinely disconnected stay disconnected. The pass restores
the component count of the untruncated radius graph and never goes below it, so
a real vacuum gap is not bridged.
Cost
The repair is skipped when the retained graph already has one component per
system. No edge joins two systems and dropping edges can only add components, so
that is the floor, and reaching it proves equality with the untruncated graph.
That check is not free: it costs one component-labelling pass over the retained
graph, which is what the unbridged rows below measure.
RTX 4060 Laptop GPU, CUDA events, 200 timed iterations after 20 warmup, same
batch with the flag off as the control, at PR head:
Contracting the retained components to the quotient graph bounds the repair by
the component count rather than the atom count, and bounds the merge rounds by
the deepest system rather than the batch. The 4-bridged batch runs 1 merge round;
a single 6-block chain, whose components all sit in one system, runs 3. Against
the atom-count formulation, dispatched aten operations fall from 111 to 80 and
device synchronizations from 10 to 5, and the no-fracture fast path is 25
operations and 3 synchronizations either way. Those operation counts were taken
before the either-endpoint correction above, which changes which edges are
claimed but not which operations are dispatched.
Rejected alternatives
torch.topk(k+1)instead of the full sort in the mask[num_atoms, global_max_degree]paddingValidation
tests/core/graph/test_beta0_neighbor_selection.pyadds 52 tests, all passing:the component labeller against an independent union-find, the fracture itself at
four budgets, restoration at four budgets, both v1 and v2, the no-op fast path on
dense bulk, disjointness of reserved and retained edges, bidirectionality of a
restored bridge, the merge-round bounds on chains of 2 to 6 bridged blocks under
two gap orderings, and the branches the two-grain case does not reach: strict
truncation, a mixed-PBC batch, a single-atom system in the batch, and the
backbone keyword actually arriving at graph generation.
Five properties of the reserved edge set are asserted directly, since selection
reads only distances: invariance under rigid motion (3 random rotations plus
translations), equivariance under uniform scaling of positions and cutoff
together (0.5x, 2x, 10x), invariance under atom relabelling, superset of the
flag-off edge set at four budgets, and bitwise reproducibility across repeated
calls on CPU and, under a
gpumarker, on CUDA.An eSCN-MD forward on an RTX 4060 Laptop GPU takes two fcc grains from 2 graph
components to 1 and a 4-block chain from 4 to 1, leaves a plain bulk control
edge-for-edge identical at 1120 edges, and keeps node embeddings and position
gradients finite in both settings. Weights are random, since
facebook/UMAis agated repository, so this exercises graph generation and the equivariant blocks
rather than any trained prediction.
Correctness is measured by mutation rather than by coverage. Nine targeted
defects were injected into
radius_graph_pbc.pyone at a time and the suite rerunagainst each:
test_deep_chain_is_fully_reconnectedThe two survivors reduce pointer-jump counts only; the labeller's convergence
loop and the Boruvka round count respectively absorb the shortfall on every
input tested, so neither is shown to change an output. They are not shown to be
safe in general either — see Limits.
tests/core/graphfails 31/190 on this branch and 31/138 on upstream main ata73c95b, and
tests/core/models/umafails 3/239 on both, with identicalfailure sets — no test regresses in either.
Limits
The 31 shared failures are a local Windows environment, not a clean run:
nvalchemiopsis not installable there, which fails theradius_pbc_version=3tests, and the checkpoint-downloading tests fail alongside them. They fail the
same way on upstream main, which is why the failure sets are diffed rather than
counted. The timing figures are from a single RTX 4060 laptop GPU and a single
batch shape; wall-clock ratios for this change moved between 1.29x and 3.35x on
that machine for identical code, because the mechanism is kernel-launch and
synchronization count rather than arithmetic, so the operation counts above are
the load-bearing measurement and the timings are indicative. Under exact
geometric degeneracy the minimum spanning forest is not unique (two symmetric
fcc grains gave 112 bitwise-equal candidate bridges); the whole degenerate shell
is kept rather than one representative, so this is not a minimal forest, and it
is what makes the 13.33% worst-case growth figure a property of symmetric test
structures rather than of the algorithm. The two surviving mutants show that the
pointer-jump counts are not tightly bounded by any current test: reducing them
changes no output on the structures tested, so the counts are sufficient but not
demonstrated to be necessary or minimal. The flag is not wired into any shipped
config — enabling it for UMA inference is a separate decision. The
radius_pbc_version=3path is covered by reading rather than by execution:nvalchemiopsdoes not install on the machine used here, so the nvidiawiring — where
c_indexandn_indexshare the global atom index space and theneighbour counts are recomputed from the masked
c_index— has not been run.Graph parallelism with the flag on is likewise untested; v2 leaves
neighbor_indexout under a node partition and v1 reaches the shared mask beforegenerate_graphfilters by partition, but neither has been exercised. TheeSCN-MD forward itself is not bitwise reproducible run to run on CUDA with the
flag off or on, so that nondeterminism sits upstream of this change; the graph it
consumes is reproducible.