Skip to content

Commit 399a5a6

Browse files
lmeyerovclaude
andcommitted
fix(gfql/index): engine-aware cost gate — resident index never loses to scan (F1)
The seeded-hop planner falls back to a full scan once the frontier covers too large a fraction of source keys (past the crossover, scanning all edges once beats many index probes). That crossover is engine-dependent: a vectorized-scan engine (polars/cuDF/GPU) scans far faster than pandas, so its crossover is much smaller. The single 0.5*n_keys threshold made a resident index under index_policy='use' run ~2x SLOWER than plain scan on polars for mid-size frontiers (measured ~frac 0.02-0.5; correct result, just slow). Now per-engine: _COST_GATE_FRAC={PANDAS:0.5}, default 0.02 (vectorized). GPU values provisional pending large-graph measurement on dgx. Small-frontier wins (the point of the index) unchanged. Data-driven (CPU, N=1e5 deg8): located polars crossover at ~frac 0.02 (pandas still winning 1.5x at 0.15). Post-fix sweep: polars mid-band flips to scan at ~1.0x (was 0.5x index); pandas unchanged; index==scan match everywhere. +1 engine-parametrized regression test (direct-hop path). Index suite 43 passed; index+chain+policy 120 passed CPU; ruff+mypy clean. Also logs benchmark receipts (RESULTS.md): CPU seeded-index proof (pandas 28x-2451x, polars 7x-145x, warm flat in N) and the explicit-vs-auto decision (default 'use'). Separate finding filed as #1670 (pandas Cypher RETURN is ~O(N)). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent af875ff commit 399a5a6

4 files changed

Lines changed: 39 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2121
- **GFQL Cypher `count(*)` short-circuit — O(1) instead of O(N) materialize**: a lone `RETURN count(*)` over a single node or edge pattern (`MATCH (n) RETURN count(*)`, `MATCH ()-[r]->() RETURN count(*)`) previously materialized the entire matched frame and ran a constant-key `group_by` just to count its rows. The lowering now emits a new `count_table` row op that reads the scanned table's height directly (or, when the pattern applies a filter, sums the boolean alias-mask column) in a single reduction — no full-frame copy, no group_by. Applies only to the provably-equivalent shapes: exactly one non-DISTINCT `count(*)`, no group keys / post-aggregate exprs / row-level `WHERE` / `UNWIND` / paging / multi-relationship binding, and either a pure node scan (`relationship_count == 0`) or a single relationship counted on its edge alias — every other shape (node-alias-over-relationship, multi-hop paths, `count(DISTINCT …)`, grouped counts) falls through to the general aggregate path unchanged. Engine-polymorphic across pandas/cuDF/polars/polars-gpu; differential parity verified on all four engines (`count_all_nodes`/`count_all_edges` cypher conformance cases + a `count_table` row-op subject case, chain and DAG surfaces). No change to any result value — only the execution path.
2222

2323
### Fixed
24+
- **GFQL adjacency-index cost gate is engine-aware (never slower than scan)**: the seeded-hop planner falls back to a full scan once the frontier covers too large a fraction of the source keys (past that, scanning all edges once beats many index probes). That crossover fraction is *engine-dependent* — a vectorized-scan engine (polars/cuDF/GPU) has a far faster scan, so its crossover is much smaller than pandas'. The gate previously used a single `0.5·n_keys` threshold, so on polars a **resident index under `index_policy='use'` ran ~2× slower than the plain scan** for mid-size frontiers (measured ~frac 0.02–0.5; correct result, just slow). The gate now uses a per-engine crossover (pandas ~0.5, vectorized engines ~0.02; GPU provisional pending large-graph measurement), so a resident index never loses to the un-indexed path on any engine. Small-frontier wins (the point of the index) are unchanged. +1 engine-parametrized regression test.
2425
- **GFQL `ne()` / `<>` on NULL now follows openCypher/SQL 3-valued logic (pandas)**: `n({"col": ne(x)})` and cypher `WHERE n.col <> x` over a NULL/NA cell used to KEEP the null row on the pandas engine (`NaN != x` → True), diverging from cuDF and the polars engine (both drop it) — and even from pandas' own `WHERE NOT n.col = x` path. Per openCypher/SQL three-valued logic, `null <> x` is `null` (an unknown value cannot be proven unequal to `x`), so a null cell is **not** a match and the row is excluded — consistent with `eq`/`gt`/`lt`/`IN` (which already dropped nulls). Fixed the `NE` predicate to mask out nulls; this corrects both the `filter_dict` predicate path and the single-entity cypher `<>` WHERE path on pandas. cuDF/polars/polars-gpu were already conformant. Verified across all four engines (`ne`, `<>`, `NOT =`, `NOT IN` all drop the null). Note: this is a behavior change for `ne()` on nullable columns under the default pandas engine. (Broader openCypher null-semantics alignment + docs tracked in #1664.)
2526
- **GFQL membership filter (`n({col: [..]})` / `IN`) on NULL follows openCypher 3VL (cuDF)**: a list/membership filter over a NULL cell — e.g. `n({"kind": ["x", "z", None]})` — used to KEEP the null row on the **cuDF** engine (cuDF `isin` matched a null cell against a `None` list element), diverging from pandas and polars (both exclude it). Per openCypher/SQL 3VL, `null IN [...]` is `null` → not a member → excluded. Fixed in `filter_by_dict` (`& notna()` on the membership branch); a no-op for pandas/polars, a fix for cuDF. (Part of the #1664 openCypher-conformance sweep.)
2627
- **GFQL `engine='polars-gpu'` silent CPU fallback removed (NO-CHEATING)**: the GPU collect used `pl.GPUEngine(raise_on_fail=False)`, so any plan node the cudf_polars backend can't execute would silently run **on CPU** and still be reported as a `polars-gpu` result — making `engine='polars-gpu'` indistinguishable from `engine='polars'` whenever the plan isn't fully GPU-capable (a benchmark showing near-identical `polars`/`polars-gpu` timings is exactly this tell). Flipped to `raise_on_fail=True` and translate the cudf_polars failure into a clear `NotImplementedError` pointing at `engine='polars'` for native CPU. `polars-gpu` is now **GPU-or-error**: any timing it produces is real on-device work, never CPU mislabeled as GPU. Verified on dgx-spark (LiveJournal 35M): the seeded-frontier `hop`/2-hop chain plan executes fully on GPU without raising (nvidia-smi 92% util during the loop), so existing GPU timings are unchanged — only the honesty guarantee is added. +1 regression test (the GPU-collect error path is translated, not swallowed).

benchmarks/gfql/RESULTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,6 @@ Summary-only log for notable benchmark runs. Raw per-scenario outputs live in
8585
| 2026-03-15 | DGX exploratory (feat/gfql-gpu-pagerank-benchmark) | `neo4j_pipeline_probe.py` Neo4j+GDS analog on SNAP GPlus | GPlus exact analog (`degree_q=0.995`, `pagerank_q=0.9995`) exceeded `3m07s` before the main transaction even finished closing, even after batching seed/core expansion to avoid transaction-memory OOM. | Raw output: `plans/gfql-gpu-pagerank-benchmark/results/neo4j_summary.md` |
8686
| 2026-03-15 | wip (`feat/gfql-gpu-pagerank-benchmark`) | `load_prepare_cpu_gpu.py` on SNAP Twitter (`degree_q=0.99`) | Cached prep median: CPU `0.2756s` vs GPU `0.1013s` (`2.72x`). Read `0.2156s` vs `0.0862s`; node prep `0.0620s` vs `0.0148s`; bind negligible. | Raw output: `plans/gfql-gpu-pagerank-benchmark/results/twitter_load_prepare_infer.json` |
8787
| 2026-03-15 | wip (`feat/gfql-gpu-pagerank-benchmark`) | `load_prepare_cpu_gpu.py` on SNAP GPlus (`degree_q=0.995`) | Cached prep median: CPU `8.7160s` vs GPU `3.9323s` (`2.22x`). Read `6.9096s` vs `3.0395s`; node prep `1.8097s` vs `0.8613s`; bind negligible. | Raw output: `plans/gfql-gpu-pagerank-benchmark/results/gplus_load_prepare_infer.json`; explicit integer-dtype probe rejected because Twitter regressed slightly and GPlus pandas overflowed. |
88+
| 2026-07-01 | 1257dac9 (dev/gfql-seeded-traversal-index) | `index_perf.py` ENGINES=pandas,polars NS=1e4,1e5,1e6 DEG=8 REPS=10 SEEDS=1 (local CPU, small-run proof) | Seeded warm FLAT ~0.30–0.33ms across N=1e4→1e6 (edges 80k→8M); scan O(E): pandas 8.6→808ms (28×→2451×), polars 2.2→45.5ms (7×→145×). warm floor engine-independent (~0.3ms=numpy searchsorted). Parity via 41-passed test suite. | Raw: `plans/gfql-1658-seeded-index/receipts/p1-{pandas,polars}.jsonl` |
89+
| 2026-07-01 | 1257dac9 (dev/gfql-seeded-traversal-index) | `explicit_vs_auto_sweep.py` (frontier sweep, pandas+polars, N=1e5 E=8e5, guarded match=True) | Index crossover engine-dependent: pandas wins to ~frac 0.5, polars scan ~18× faster → crossover ~frac 0.02. **DECISION: default `use`.** Finding F1(IMPORTANT): cost gate 0.5·n_keys miscalibrated for polars → resident index 0.4–0.5× (2× slower) in F≈1k–50k mid-band; fix=engine-aware gate. | Decision: `plans/gfql-1658-seeded-index/receipts/decision-explicit-vs-auto.md`; raw: `…/p3-explicit-vs-auto.jsonl` |
90+
| 2026-07-01 | 1257dac9 (dev/gfql-seeded-traversal-index) | `lookup_probe.py` [D-index-wiring] point/range, N=500k M=2M id=1..N, pandas+polars CPU | Bug confirmed (node_id index ignored by filter path, 1.01× w/ index) BUT filter is sub-ms (pandas 0.79ms/polars 0.34ms); the point-lookup cost is the **Cypher RETURN row-pipeline**: RETURN a = pandas 94.7ms vs polars 1.2ms. DECISION: **REJECT node_id→filter wiring** (saves <1ms vs ~94ms RETURN); real lever = pandas RETURN path / use polars-cudf. | `plans/gfql-1658-seeded-index/receipts/lookup-finding.md` |

graphistry/compute/gfql/index/api.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@
2121

2222
REGISTRY_ATTR = "_gfql_index_registry"
2323

24+
# Index-vs-scan crossover fraction (of distinct source keys): past this frontier a
25+
# full scan is cheaper than per-seed index probes, so `use` falls back to scan and
26+
# never loses to the un-indexed path. Engine-aware because vectorized-scan engines
27+
# (polars/cudf/GPU) scan far faster than pandas, so their crossover is much smaller
28+
# (F1). Measured N=1e5 deg8: pandas ~0.5, polars ~0.02. GPU values provisional
29+
# (dgx-gated) — conservatively grouped with polars.
30+
_COST_GATE_FRAC = {Engine.PANDAS: 0.5}
31+
_COST_GATE_FRAC_DEFAULT = 0.02
32+
2433
# --- lightweight, thread-local index decision trace (for gfql_explain) -------
2534
import threading as _threading
2635
_TRACE = _threading.local()
@@ -315,7 +324,8 @@ def maybe_index_hop(
315324
elif policy != "force":
316325
try:
317326
frontier_n = int(nodes.shape[0])
318-
if idx0.n_keys > 0 and frontier_n >= 0.5 * idx0.n_keys:
327+
frac = _COST_GATE_FRAC.get(engine, _COST_GATE_FRAC_DEFAULT)
328+
if idx0.n_keys > 0 and frontier_n >= frac * idx0.n_keys:
319329
return None
320330
except Exception:
321331
pass

graphistry/tests/compute/gfql/index/test_index.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ def test_index_policy_force_and_explain(graph, engine):
135135
assert _sig(r_scan) == _sig(r_force)
136136

137137

138+
@pytest.mark.parametrize("engine", ENGINES)
139+
def test_cost_gate_engine_aware_never_loses_to_scan(engine):
140+
"""F1: the index-vs-scan crossover depends on scan speed, so the cost gate
141+
(maybe_index_hop) is engine-aware. A mid-frontier seeded hop (~frac 0.25 of source
142+
keys) still beats the slow pandas scan (→ index path) but would lose to the fast
143+
vectorized scan on polars/cudf/GPU (→ scan fallback), so a resident index never
144+
runs slower than the un-indexed path. Result is identical on either path."""
145+
from graphistry.compute.gfql.index import index_trace
146+
rng = np.random.default_rng(1)
147+
N, deg = 4000, 8
148+
m = N * deg
149+
edf = pd.DataFrame({"src": rng.integers(0, N, m), "dst": rng.integers(0, N, m)})
150+
ndf = pd.DataFrame({"id": np.arange(N)})
151+
g = graphistry.nodes(ndf, "id").edges(edf, "src", "dst")
152+
seeds = pd.DataFrame({"id": np.arange(N // 4, dtype=np.int64)}) # frac ~0.25 of n_keys (~N)
153+
gi = g.gfql_index_all(engine=engine)
154+
with index_trace() as steps:
155+
out = gi.hop(nodes=seeds, engine=engine, hops=1, direction="forward")
156+
took_index = any(s.get("path") == "index" for s in steps)
157+
assert took_index is (engine == "pandas"), (engine, steps) # vectorized engines → scan
158+
scan_out = g.hop(nodes=seeds, engine=engine, hops=1, direction="forward") # no resident index
159+
assert _sig(out) == _sig(scan_out), engine # correctness path-independent
160+
161+
138162
def test_column_mismatch_raises_not_silent(graph):
139163
# A custom column that doesn't match the binding must raise, not silently no-op.
140164
with pytest.raises(NotImplementedError):

0 commit comments

Comments
 (0)