Skip to content

perf: compute ase2networkx bonds from sparse neighbor pairs - #132

Merged
PythonFZ merged 2 commits into
mainfrom
perf/ase2networkx-memory
Oct 2, 2026
Merged

PythonFZ merged 2 commits into
mainfrom
perf/ase2networkx-memory

Conversation

@PythonFZ

@PythonFZ PythonFZ commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

What changed

The distance-based path of ase2networkx builds bonds from the neighbor list as a sorted (M, 2) pair array:

  • _compute_bonded_pairs keeps i < j pairs between bonding atoms with d <= scale * (r_i + r_j), keeps zero-shift pairs only for pbc=False, and collapses periodic images with np.unique.
  • The graph gets nodes 0..N-1, then add_edges_from(pairs.tolist(), bond_order=None). Node, edge and graph attributes and the edge order match the previous output.
  • The neighbor-list cutoff is one ulp above 2 * max(scale * r) over bonding atoms (Li/Na/K/Rb/Cs/Fr excluded), so pairs at exactly their cutoff are bonded on both backends. For a Cs⁺/PF6⁻/EC box this is 2.57 Å, down from 5.86 Å.
  • Radii come from ase.data.covalent_radii[numbers] * scale, the same values natural_cutoffs returns.

Peak memory goes from about 25 B·N² (dense N x N float and int arrays plus nx.from_numpy_array) to O(N + M). The signature and the atoms.info["connectivity"] path are unchanged.

Behaviour change

In cells small enough that one pair has several periodic images inside the neighbor-list cutoff, the dense matrix kept whichever image the backend emitted last, so the bond depended on the backend. A pair is now bonded when any image lies within its cutoff. Example: Atoms("CH", positions=[[0, 5, 5], [0.7, 5, 5]], cell=[2, 10, 10], pbc=True) (images at 0.7 Å and 1.3 Å, cutoff 1.284 Å) was bonded with vesin and unbonded with ASE on main; both backends bond it now.

Tests

  • test_ase2networkx_matches_reference: edges equal an all-pairs minimum-image reference (ase.geometry.get_distances) for 7 systems (packed, unwrapped, wrapped, mixed pbc [T, F, T], triclinic, Li⁺/PF6⁻ electrolyte, phenol) × pbc {True, False} × scale {1.0, 1.2, 1.5} × backend {vesin, ASE}.
  • test_ase2networkx_memory_is_linear on both backends: peak memory ratio between 8,232 and 648 atoms must stay below 2× the atom ratio (limit 25.4). This PR: 12.7 (vesin, 0.6 → 7.6 MB) and 11.5 (ASE, 7.9 → 90.4 MB); main: 146–160.
  • test_ase2networkx_bonds_at_cutoff_boundary: a C–C pair at exactly 2 * 1.2 * r_C is bonded (fails on main).
  • test_ase2networkx_neighbor_cutoff_ignores_ions: the neighbor-list cutoff for PF6⁻ + Cs⁺ is 2 * 1.2 * r_P (main: 2 * 1.2 * r_Cs).
  • test_ase2networkx_bonds_through_any_periodic_image (fails on main with ASE), test_ase2networkx_skips_periodic_self_images, test_ase2networkx_only_non_bonding_ions, test_ase2networkx_graph_contract.
  • Mutation check: dropping the self-image filter, the all-ion guard, the pbc=False shift filter, np.unique, .tolist(), the ion filter or scale, switching <= to <, or sizing the cutoff over all atoms, each fails at least one new test.
  • uv run pytest: 560 passed. uvx prek --all-files: clean.

Benchmark

Periodic water box (100 H₂O, 14.4 Å) tiled with Atoms.repeat, vesin 0.4.2 backend, macOS arm64, Python 3.11, one fresh process per cell. Peak is the tracemalloc peak during the ase2networkx call.

atoms main peak main wall this PR peak this PR wall
2,400 144 MB 0.041 s 2.2 MB 0.009 s
8,100 1,641 MB 0.335 s 7.5 MB 0.022 s
19,200 ≈ 9.2 GB (25 B·N² estimate) 17.6 MB 0.047 s
64,800 ≈ 105 GB (estimate) 60.6 MB 0.186 s
102,900 ≈ 265 GB (estimate) 98.3 MB 0.292 s
1,012,500 ≈ 25.6 TB (estimate) 948 MB 3.80 s

At 1M atoms most of the 948 MB is the returned nx.Graph (one attribute dict per node and edge).

Closes #115

🤖 Generated with Claude Code

@PythonFZ PythonFZ added bug Something isn't working p-2 Priority: normal effort-2 Effort: moderate labels Oct 2, 2026
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 95ddd59e-f2b6-4aee-a164-f42e4f6cf893

📥 Commits

Reviewing files that changed from the base of the PR and between f4d4cb4 and 2237a5a.

📒 Files selected for processing (2)
  • src/molify/ase2x.py
  • tests/integration/test_ase2x.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

PythonFZ and others added 2 commits October 2, 2026 13:39
The distance-based path of ase2networkx turns the vesin/ASE neighbor list
into a sorted (M, 2) array of bonded pairs and adds those edges to a graph
with nodes 0..N-1. Peak memory scales with atoms plus bonds: 7.5 MB for
8,100 atoms, down from 1.64 GB.

The neighbor-list cutoff spans bonding atoms only, and a pair is bonded
when any periodic image lies within its cutoff, so tiny periodic cells
give the same bonds with vesin and ASE.

Tests compare edges against an all-pairs minimum-image reference across
wrapped, unwrapped, mixed-pbc, triclinic, ionic and molecular systems on
both backends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Pass one ulp above the bonding cutoff to the neighbor list so the
  inclusive pair filter decides pairs at exactly the cutoff.
- Store the non-bonding ions as a tuple and read it directly in
  _add_node_properties.
- Test ase2networkx with scale 1.0, 1.2 and 1.5 against the reference.
- Test that the neighbor-list cutoff comes from bonding atoms only.
- Measure peak memory on both neighbor-list backends at two box sizes and
  assert near-linear growth.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@PythonFZ
PythonFZ force-pushed the perf/ase2networkx-memory branch from 95d1c24 to 2237a5a Compare October 2, 2026 12:40
@PythonFZ
PythonFZ merged commit 367fe78 into main Oct 2, 2026
22 checks passed
@PythonFZ
PythonFZ deleted the perf/ase2networkx-memory branch October 2, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working effort-2 Effort: moderate p-2 Priority: normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ase2networkx is extremly space hungry for large systems

1 participant