Skip to content

Commit a73518e

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 f817b0b commit a73518e

4 files changed

Lines changed: 41 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2525
- **GFQL `materialize_nodes()` on an edges-only graph under `engine='polars'`**: crashed with `AttributeError: 'Series' object has no attribute 'drop_duplicates'` — the node-id derivation used pandas-only `.drop_duplicates()` / `.reset_index()` / `.rename(columns=)` on a polars Series/DataFrame. Now polars-aware (`.unique(maintain_order=True)`, `.select().rename({...})`), matching the pandas oracle. Surfaced by any `let()` DAG on an edges-only graph under Polars (the DAG pre-materializes nodes). pandas/cuDF paths unchanged.
2626
- **GFQL `engine='polars-gpu'` LazyFrame-input coercion now collects on the GPU executor**: `Engine.df_to_engine(lazyframe, POLARS_GPU)` materialized a `polars.LazyFrame` input with a bare `.collect()` on the CPU default executor — ignoring `gpu_executor()` and not distinguishing `POLARS` from `POLARS_GPU`. It now routes through the target-aware lazy collect, so a LazyFrame coerced under `polars-gpu` collects on the cudf-polars GPU executor (and `polars` honors `cpu_streaming()`). No change for already-materialized frame inputs (the common path).
2727
- **GFQL Polars engine `contains` honors `regex=`/`flags=`**: the native polars `filter_by_dict` lowering of the `Contains` predicate always used `str.contains(..., literal=False)`, so a **literal** request (`contains(pat, regex=False)`) was still regex-interpreted — a pattern with a metacharacter over-matched (e.g. `contains('a.c', regex=False)` matched `'abc'`), diverging from pandas/cuDF. It also dropped `flags=`. Now `regex=False` lowers to a literal match (`literal=True`; case-insensitive literal folds both sides, matching pandas' result), and regex mode maps `case=`/`flags=` (IGNORECASE/MULTILINE/DOTALL/VERBOSE) to a Rust-regex inline flag prefix (`(?ims…)`). +1 engine-parametrized differential-parity test.
28+
- **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.
29+
- **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.)
30+
- **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.)
2831
- **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).
2932
- **GFQL Polars chain dtype-mismatched join keys**: a chain (`MATCH (a)-[]->(b)…`) crashed under `engine='polars'` with `SchemaError: datatypes of join keys don't match` when an edge endpoint column's dtype differed from the node-id dtype across the int↔float boundary — e.g. a null in a `source`/`destination` column promotes it to `float64` while integer node ids stay `int64`. The hop already aligned join keys; the chain fast paths + combine did not. Now aligns endpoint join keys to the node-id dtype for the traversal and restores the original endpoint dtype on the output (matching pandas). No-op when dtypes already match.
3033
- **GFQL `chain()` OTel span placement**: the `gfql.chain` OpenTelemetry span (`@otel_traced`) had landed on the internal `_try_chain_fast_path` probe instead of the public `chain()` — so `chain()` lost its span and the span recorded the wrong function/attributes. Moved the decorator back onto `chain()`.

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)