Skip to content

perf(gfql): undirected hop skips the whole-frame dedup of the doubled edge frame (#2026) - #2036

Merged
lmeyerov merged 2 commits into
masterfrom
perf/gfql-undirected-pairs-no-dedup
Sep 5, 2026
Merged

perf(gfql): undirected hop skips the whole-frame dedup of the doubled edge frame (#2026)#2036
lmeyerov merged 2 commits into
masterfrom
perf/gfql-undirected-pairs-no-dedup

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Stacked on #2028#2024. The #2026 follow-up, with a corrected root cause.

Finding. Profiling the undirected 2-hop chain after #2024 (pandas, 1M nodes / 8M edges, 50 hub seeds): the seed-rediscovery rule is 0.36 s of 10 s; the undirected pairs build in hop.py is 5.5 s. It concatenates both edge orientations over every edge (2E rows) and hash-deduplicates that frame on (from, to, edge id), once per hop() call and three times per chain (forward, reverse, final), independent of the neighborhood size.

Fix. Every consumer of the doubled frame already dedups on the edge id (or the node id), so the frame is now the plain concat of both orientations, which is what the polars hop does. A review-wave differential (2,300 randomized direct-hop cases and 750 chains on pandas and cuDF: self-loops, parallel edges, null endpoints, string/float/int ids, duplicate indexes and bound edge ids, hops 1–3, fixed point, hop windows, wavefront, labels) found zero divergences from the previous implementation.

Measured locally (not a published number): the 2-hop chain 9.1 s → 3.3 s on pandas; the block alone on the 8M-edge frame 7.1 s → 1.4 s pandas, 50 ms → 17 ms cuDF. The published LiveJournal rows are the polars lane, which this does not change; the LJ re-measure adds the pandas arm.

Tests: test_undirected_pairs_2026.py (set-oracle parity with self-loops, parallel edges and hub seeds for 1- and 2-hop on pandas, polars, cuDF; self-loops kept once through the multi-hop block; categorical endpoints with differing category sets on pandas). Hop/chain suites (test_hop, hop semantics pins, boundary matrix, seed rediscovery, scaling pin, chain, #1918 pins): 866 passed. Guards green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WwMmVFo44ADiRRj5cxh1i1

@lmeyerov

lmeyerov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Review wave 1 (quiet, 2,300 randomized direct-hop + 750 chain differentials on pandas/cuDF): the reverse-orientation self-loop filter compared the two endpoint columns and broke categorical endpoints with differing category sets; the filter is not load-bearing because every consumer dedups on the edge id, so 6745cd3 builds the plain concat of both orientations (what the polars hop does), which is also faster (block alone on 8M edges, local, unpublished: pandas 7.1 s → 1.4 s; cuDF 50 ms → 17 ms). Pins: categorical endpoints (pandas; cuDF cannot concat such categoricals on the base either), self-loop pin now exercises the multi-hop block, docstring/CHANGELOG corrected (entry moved under Performance, numbers labeled local).

@lmeyerov

lmeyerov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

READY (946586a): 75/75 + RTD; rebased on master after #2024/#2028; pandas/cuDF no-dedup fix with the scaling pin on master as its gate (this PR is what keeps the 30x pin stable).

@lmeyerov

lmeyerov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

On the three questions (full analysis in the private plan, reviews/pygraphistry-2036-dedup-audit/analysis.md):

  1. Asymptotics: the removed drop_duplicates(subset=[FROM, TO, EDGE_ID]) could only ever drop self-loop twins — the two orientations of any other edge differ in (FROM, TO). So it hashed 2E rows per hop call (three calls per 2-hop chain) to remove #self-loops rows, and it eliminated no later work: every consumer of pairs dedups on a key already (_domain_unique(TO) / _domain_unique(EDGE_ID) in the fast loop, .drop_duplicates() / .drop_duplicates(subset=[EDGE_ID]) in the full loop), and the seed-rediscovery rule reads the matched edges by EDGE_ID in one orientation, never the doubled frame. The polars twin has built the doubled frame without a dedup since hop(): #1892's own invariant still false in the unfiltered arm, plus 3 more pandas silent-wrongs (round-011) #1918. Receipt: the tests(gfql): scaling pin for undirected 2-hop (ratio vs two plain joins) #2028 scaling pin (undirected 2-hop ≤ 30× two plain joins) measured 47–52× on the 0.59.0 tree and 6–20× here. The "earlier dedup prunes later work" case is the node side (frontier before isin), which is unchanged.

  2. Load-bearing: contracts touched = one output row per matched edge, self-loops once (also through multi-hop), hub seeds, categorical endpoints, the perf(gfql): undirected multi-hop is ~30x slower since #1918 — pure-Python O(E) seed-rediscovery loop #2023/perf(gfql): numpy form of the undirected seed-rediscovery rule (#2023) #2024 rediscovery semantics, and the cost bound — each has a pin (test_undirected_pairs_2026.py, test_seed_rediscovery_2023.py, hop semantics 1918, test_hop_scaling_pin.py). Residual risk is a future consumer of pairs that skips the key-dedup; the one-line invariant at the concat site and the self-loop pin are the guards.

  3. Generalization: audited every drop_duplicates/unique in hop.py, chain.py, the fast-path modules and the polars hop. No other O(E)-for-nothing op. One different-class finding: the full loop re-dedups the accumulated matches_edges (and node labels) every hop — O(hops × matches) on labeled/min_hops shapes, where the fast loop already uses set unions. Filing that as a follow-up with the scaling pin extended to a 4-hop labeled shape rather than widening this PR.

@lmeyerov

lmeyerov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

READY (eed7de8) — rebased on master 9226829 after #2035 landed (the PR was CONFLICTING on CHANGELOG and bin/test-polars.sh; union-resolved, one registration line kept). The rebase also tidies the unreleased CHANGELOG block into one Tests / Fixed / Performance / Changed sequence: master had carried two "Fixed" sections and the union put this PR's "Performance" between them. Diff vs master is unchanged in substance: hop.py (3 lines), test_undirected_pairs_2026.py, one CHANGELOG bullet, one test-lane registration.

The three design questions (asymptotics, load-bearing contracts, generalization) were answered on 2026-09-05 13:39Z from the audit; nothing new since.

Receipts at eed7de8

@lmeyerov
lmeyerov force-pushed the perf/gfql-undirected-pairs-no-dedup branch from eed7de8 to 26cde1d Compare September 5, 2026 18:35
@lmeyerov

lmeyerov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

CI at the rebased head 26cde1d: 77/77 check-runs success (tck-gfql, changed-line coverage, polars lane, hygiene guards included). Diff vs master unchanged in substance (hop.py 3 lines, the undirected-pairs pins, one CHANGELOG bullet, one test-lane registration); GPU receipt from eed7de8 carries over (product code identical). Ready to merge.

lmeyerov and others added 2 commits September 5, 2026 12:18
…ting the doubled edge frame

Every consumer of the doubled frame dedups on the edge id, so the whole-frame
drop_duplicates over 2E rows (once per hop call, three times per chain) is redundant; the
frame is now the plain concat the polars hop uses. Pins: set-oracle parity with self-loops,
parallel edges and hub seeds on pandas, polars and cuDF; self-loops kept once through the
multi-hop path; categorical endpoints with differing category sets on pandas.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WwMmVFo44ADiRRj5cxh1i1
@lmeyerov
lmeyerov force-pushed the perf/gfql-undirected-pairs-no-dedup branch from 26cde1d to 6abfb92 Compare September 5, 2026 19:18
@lmeyerov

lmeyerov commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

READY (6abfb92) — restacked onto master 8b208a6 after #2038 landed (the PR had gone CONFLICTING on the polars test-lane registration list next to #2038's line; union-resolved, one line each). The graphistry/ diff is patch-id-identical to 26cde1d; CHANGELOG stays one Fixed + one Performance section. CI on the new head: 77/77 check-runs success. Receipts from the audit + dgx GPU lane (1337 passed, 3 failed = the #2043 cudf-26.02 divergences identical on master) carry over unchanged.

@lmeyerov
lmeyerov merged commit 5a6586f into master Sep 5, 2026
78 checks passed
@lmeyerov
lmeyerov deleted the perf/gfql-undirected-pairs-no-dedup branch September 5, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant