Skip to content

Commit dc4f9da

Browse files
lmeyerovclaude
andcommitted
garden(gfql/index): team-polish — dead code, IF EXISTS, docstrings, bench honesty
Review-wave findings on the index PR: - Delete dead engine_arrays.to_backend/cp_to_numpy (never called) + unused get_registry import (explain.py); remove index_ddl_smoke.py 'if False' scaffolding + unused variable. - DropIndex.missing_ok was serialized but never honored. Now real IF EXISTS semantics: 'DROP GFQL INDEX [IF EXISTS] <name|FOR kind>' — plain DROP of a missing index raises (SQL-style, matching the existing custom-name test), IF EXISTS / missing_ok=true is a no-op; wire default flipped to false (field is new in this PR — no consumers). +1 test covering DDL both forms, wire JSON both ways, and resident-drop. - Replace internal review-wave codes (F1/B1/B2/B3/I4/I5/I6/P0-1) in production comments with plain prose or in-repo test-name pointers (test-file section labels stay — they title the regression tests). - ComputeMixin: real docstrings on create_index/drop_index/show_indexes/ gfql_index_edges/gfql_index_all/gfql_explain (were one-liner-pointer or missing); gfql() docstring now documents index_policy (was invisible to help() despite being the flagship knob). - index_bulk_olap_bench.py: polars-gpu cell used raise_on_fail=False — the exact silent-CPU-fallback pattern the CHANGELOG bans; now GPU-or-error. - Label index_smoke/index_ddl_smoke as container-runnable mirrors of the canonical pytest suite; add benchmarks/gfql/README.md catalog for the 7 index_* harnesses (which pair is canonical for published claims). Validated: index suite 44 pass CPU-lane (cudf/polars-gpu lanes = local no-GPU artifact, dgx-verified separately); ruff + mypy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6a01e2a commit dc4f9da

15 files changed

Lines changed: 126 additions & 51 deletions

File tree

benchmarks/gfql/README.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,30 @@ So the honest current comparison is:
382382
- Neo4j is workable for the smaller Twitter analog, but already materially slower than both Graphistry CPU and GPU on the exact same shape.
383383
- On the selected GPlus benchmark shape, Neo4j is already dramatically slower than Graphistry CPU (`83.61s`) and Graphistry GPU (`3.41s`) before teardown/cleanup is even done.
384384
- Raw notes: `plans/gfql-gpu-pagerank-benchmark/results/neo4j_summary.md`
385+
386+
## GFQL physical-index harnesses (`index_*.py`)
387+
388+
Catalog for the seeded-traversal adjacency-index work (PR #1658; see
389+
`docs/source/gfql/index_adjacency.rst` for the user guide + published numbers):
390+
391+
- `index_smoke.py` / `index_ddl_smoke.py` — correctness smokes: differential
392+
parity (index path == scan path) and DDL/wire/`index_policy`/`gfql_explain`
393+
round-trips. Container-runnable MIRRORS of the canonical pytest suite
394+
(`graphistry/tests/compute/gfql/index/test_index.py`).
395+
- `index_perf.py` — microbenchmark: seeded 1-hop/2-hop index-vs-scan across
396+
engines and frontier sizes; the cost-gate calibration numbers come from here.
397+
- `index_takeover_bench.py` + `index_vs_kuzu_prepared.py`**canonical
398+
competitor pair** behind the published "9–36x vs Kuzu" seeded-traversal
399+
claims: guarded timings (path-taken assertions + result==scan oracle),
400+
prepared-statement fairness on the Kuzu side.
401+
- `index_vs_dbs.py` — broader 3-way sweep (GFQL / Kuzu / Neo4j) over the same
402+
seeded shapes; superset of the pair above, heavier setup (dockerized Neo4j).
403+
- `index_bulk_olap_bench.py` — bulk group-by/degree OLAP shapes across the 4
404+
engines (answers "does the index help bulk scans?" — it does not; scans win).
405+
- `index_largegraph_bench.py` — scale probe on large real graphs
406+
(flat-in-N behavior of the seeded path up to ~80M edges).
407+
408+
Methodology for all: warm medians, one engine per process for large runs,
409+
NO-CHEATING guards (a timing is void unless the intended path was taken AND
410+
the result matches the scan oracle). Run on an idle box; GPU rows need
411+
`--gpus all` and RAPIDS.

benchmarks/gfql/index_bulk_olap_bench.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ def degall(edf, engine, reps, warm):
114114
import polars as pl
115115
df = pl.from_pandas(edf)
116116
if engine == "polars-gpu":
117-
eng = pl.GPUEngine(executor="in-memory", raise_on_fail=False)
117+
# raise_on_fail=True: GPU-or-error — a silent CPU fallback would report
118+
# CPU time as a polars-gpu result (banned; see the polars-gpu CHANGELOG fix)
119+
eng = pl.GPUEngine(executor="in-memory", raise_on_fail=True)
118120
fn = lambda: df.lazy().group_by("src").len().collect(engine=eng)
119121
else:
120122
fn = lambda: df.group_by("src").len()

benchmarks/gfql/index_ddl_smoke.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#!/usr/bin/env python3
2-
"""Smoke test: Cypher DDL + JSON wire + index_policy auto/force + gfql_explain."""
2+
"""Smoke test: Cypher DDL + JSON wire + index_policy auto/force + gfql_explain.
3+
4+
Container-runnable MIRROR of graphistry/tests/compute/gfql/index/test_index.py
5+
(the pytest suite is canonical); kept for quick in-container smoke without pytest.
6+
"""
37
from __future__ import annotations
48

59
import sys
@@ -63,9 +67,7 @@ def main():
6367

6468
# --- index_policy auto/force build-on-demand + explain ---
6569
gp = make_graph() # fresh, no resident indexes
66-
exp_use = gp.gfql_explain([], index_policy="use") if False else None # explain needs a real query
67-
# explain with a seeded 1-hop chain expressed in cypher
68-
q = "MATCH (a)-[e]->(b) RETURN b" # not seeded -> not coverable; use python chain instead
70+
# explain with a seeded 1-hop chain (explain needs a real, seeded query)
6971
from graphistry.compute.ast import n, e_forward
7072
seeded_chain = [n({"id": 0}), e_forward(hops=1)]
7173
rep_off = gp.gfql_explain(seeded_chain, index_policy="off")

benchmarks/gfql/index_smoke.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#!/usr/bin/env python3
22
"""Differential parity smoke test for GFQL physical indexes.
33
4+
Container-runnable MIRROR of graphistry/tests/compute/gfql/index/test_index.py
5+
(the pytest suite is canonical); kept for quick in-container smoke without pytest.
6+
47
Oracle: the index fast path MUST return the same subgraph (node-id set + edge
58
multiset) as the scan/join path, across engines / directions / hops / wavefront.
69
A fast wrong answer is not a win.

graphistry/compute/ComputeMixin.py

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,23 +514,42 @@ def hop(self, *args, **kwargs):
514514

515515
# ---- GFQL physical indexes (pay-as-you-go seeded-traversal acceleration) ----
516516
def create_index(self, kind, *, column=None, name=None, engine='auto'):
517+
"""Build a GFQL physical index for O(degree) seeded traversal.
518+
519+
:param kind: 'edge_out_adj' (forward hops), 'edge_in_adj' (reverse hops), or 'node_id' (node lookup)
520+
:param column: column to index (defaults to the binding for the kind, e.g. the edge source column)
521+
:param name: optional custom index name (defaults to 'kind:column')
522+
:param engine: 'auto' | 'pandas' | 'cudf' | 'polars' — array backend for the index
523+
:returns: new Plottable with the index resident (original is untouched)
524+
525+
**Example**
526+
::
527+
528+
g2 = g.create_index('edge_out_adj')
529+
g2.gfql([n({'id': 0}), e_forward(hops=2)], index_policy='use')
530+
531+
See :doc:`gfql/index_adjacency` for policies, cost gating, and benchmarks.
532+
"""
517533
from graphistry.compute.gfql.index import create_index as _ci
518534
return _ci(self, kind, column=column, name=name, engine=engine)
519-
create_index.__doc__ = "Build a GFQL physical index (edge_out_adj|edge_in_adj|node_id). See graphistry/compute/gfql/index/api.py."
520535

521536
def drop_index(self, kind=None):
537+
"""Drop one resident GFQL index (by kind) or all (kind=None). Idempotent; returns a new Plottable."""
522538
from graphistry.compute.gfql.index import drop_index as _di
523539
return _di(self, kind)
524540

525541
def show_indexes(self):
542+
"""Return a pandas DataFrame describing resident GFQL indexes (name, kind, column, valid). Empty if none; ``valid=False`` marks a stale index after a frame rebind."""
526543
from graphistry.compute.gfql.index import show_indexes as _si
527544
return _si(self)
528545

529546
def gfql_index_edges(self, direction='both', engine='auto'):
547+
"""Convenience: build the edge adjacency index(es) — 'forward', 'reverse', or 'both'. Returns a new Plottable."""
530548
from graphistry.compute.gfql.index import gfql_index_edges as _gie
531549
return _gie(self, direction, engine=engine)
532550

533551
def gfql_index_all(self, engine='auto'):
552+
"""Convenience: build all GFQL physical indexes (both edge adjacencies + node_id). Returns a new Plottable."""
534553
from graphistry.compute.gfql.index import gfql_index_all as _gia
535554
return _gia(self, engine=engine)
536555

@@ -581,9 +600,19 @@ def gfql(self, *args, **kwargs):
581600
g = _copy.copy(self)
582601
g._gfql_index_policy = policy
583602
return gfql_base(g, *args, **kwargs)
584-
gfql.__doc__ = gfql_base.__doc__
603+
gfql.__doc__ = (gfql_base.__doc__ or "") + """
604+
605+
**GFQL physical indexes**
606+
607+
:param index_policy: 'off' (never use indexes), 'use' (use resident,
608+
cost-gated; default planner behavior), 'auto' (build on demand), or
609+
'force' (always probe the index). Also accepts index DDL strings
610+
(``CREATE GFQL INDEX ...``) / wire ops as the query — routed to the
611+
index registry. See :meth:`create_index` and :doc:`gfql/index_adjacency`.
612+
"""
585613

586614
def gfql_explain(self, query, *, index_policy='use', engine='auto'):
615+
"""Explain how the GFQL planner would run ``query``: per-hop index-vs-scan choice, cost-gate numbers, and resident-index validity. Read-only (no execution). Returns a report object; print it for a human-readable plan."""
587616
from graphistry.compute.gfql.index.explain import gfql_explain as _ge
588617
return _ge(self, query, index_policy=index_policy, engine=engine)
589618

graphistry/compute/gfql/index/api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
# full scan is cheaper than per-seed index probes, so `use` falls back to scan and
2626
# never loses to the un-indexed path. Engine-aware because vectorized-scan engines
2727
# (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
28+
# (see test_index_cost_gate_engine_aware). Measured N=1e5 deg8: pandas ~0.5, polars ~0.02. GPU values provisional
2929
# (dgx-gated) — conservatively grouped with polars.
3030
_COST_GATE_FRAC = {Engine.PANDAS: 0.5}
3131
_COST_GATE_FRAC_DEFAULT = 0.02
@@ -332,7 +332,7 @@ def maybe_index_hop(
332332

333333
# Honor max_hops: the scan resolves the hop count as ``max_hops or hops``
334334
# (compute/hop.py); the index must run the SAME number of accumulating hops.
335-
# (B1: max_hops was passed through *rest and silently ignored — the index ran
335+
# (regression: max_hops was passed through *rest and silently ignored — the index ran
336336
# `hops` (default 1) while the scan ran max_hops → wrong answer.)
337337
_max_hops = rest.get("max_hops")
338338
eff_hops = _max_hops if _max_hops is not None else hops

graphistry/compute/gfql/index/build.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def build_node_id_index(
7878
row position per unique key (``row_positions[group_offsets[:-1]]``, length U,
7979
aligned with ``unique_keys``) and (b) REFUSE (return None) when ids aren't unique:
8080
a unique-key CSR can't reproduce the scan's "all rows per id" semantics, so the
81-
caller falls back to the correct ``select_by_ids`` isin path. (B2: a non-unique
81+
caller falls back to the correct ``select_by_ids`` isin path. (Regression guard: a non-unique
8282
node-id index dropped reached nodes / emitted unrelated rows.)"""
8383
xp, backend = array_namespace(engine)
8484
keys = col_to_array(nodes, node_col, engine)

graphistry/compute/gfql/index/cypher_ddl.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
"""Targeted recognizer for GFQL index DDL Cypher statements.
22
33
CREATE GFQL INDEX [<name>] FOR <kind> [ON <column>]
4-
DROP GFQL INDEX <name>
5-
DROP GFQL INDEX FOR <kind> [ON <column>]
4+
DROP GFQL INDEX [IF EXISTS] <name>
5+
DROP GFQL INDEX [IF EXISTS] FOR <kind> [ON <column>]
66
SHOW GFQL INDEXES
77
88
The mandatory ``GFQL`` token disambiguates from standard property ``CREATE INDEX``
@@ -25,12 +25,12 @@
2525
re.IGNORECASE,
2626
)
2727
_DROP_FOR_RE = re.compile(
28-
r"^\s*DROP\s+GFQL\s+INDEX\s+FOR\s+" + _KIND
28+
r"^\s*DROP\s+GFQL\s+INDEX\s+(?P<ifexists>IF\s+EXISTS\s+)?FOR\s+" + _KIND
2929
+ r"(?:\s+ON\s+(?P<col>[A-Za-z_]\w*))?\s*;?\s*$",
3030
re.IGNORECASE,
3131
)
3232
_DROP_NAME_RE = re.compile(
33-
r"^\s*DROP\s+GFQL\s+INDEX\s+(?P<name>[A-Za-z_][\w:]*)\s*;?\s*$",
33+
r"^\s*DROP\s+GFQL\s+INDEX\s+(?P<ifexists>IF\s+EXISTS\s+)?(?P<name>[A-Za-z_][\w:]*)\s*;?\s*$",
3434
re.IGNORECASE,
3535
)
3636
_SHOW_RE = re.compile(r"^\s*SHOW\s+GFQL\s+INDEXES\s*;?\s*$", re.IGNORECASE)
@@ -54,10 +54,11 @@ def parse_index_ddl(query: str) -> Optional[Any]:
5454
name=m.group("name"))
5555
m = _DROP_FOR_RE.match(query)
5656
if m:
57-
return DropIndex(kind=m.group("kind").lower(), column=m.group("col"))
57+
return DropIndex(kind=m.group("kind").lower(), column=m.group("col"),
58+
missing_ok=bool(m.group("ifexists")))
5859
m = _DROP_NAME_RE.match(query)
5960
if m:
60-
return DropIndex(name=m.group("name"))
61+
return DropIndex(name=m.group("name"), missing_ok=bool(m.group("ifexists")))
6162
if looks_like_index_ddl(query):
6263
raise ValueError(
6364
f"Malformed GFQL INDEX DDL: {query!r}. Expected e.g. "

graphistry/compute/gfql/index/engine_arrays.py

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,27 +51,6 @@ def ids_to_array(ids: Any, col: str, engine: Engine) -> Any:
5151
return col_to_array(ids, col, engine)
5252

5353

54-
def to_backend(arr: Any, backend: str) -> Any:
55-
"""Move a host/device array to the index backend (numpy<->cupy)."""
56-
import numpy as np
57-
58-
if backend == "cupy":
59-
import cupy as cp # type: ignore
60-
61-
return cp.asarray(arr)
62-
if "cupy" in type(arr).__module__:
63-
return cp_to_numpy(arr)
64-
return np.asarray(arr)
65-
66-
67-
def cp_to_numpy(arr: Any) -> Any:
68-
try:
69-
return arr.get()
70-
except Exception:
71-
import numpy as np
72-
73-
return np.asarray(arr)
74-
7554

7655
def take_rows(df: Any, positions: Any, engine: Engine) -> Any:
7756
"""Positionally gather rows of ``df`` by an integer array ``positions``.

graphistry/compute/gfql/index/explain.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@
33
Seeded queries are cheap once indexed, so explain *executes* the query under a
44
decision trace and reports the real per-hop index-vs-scan path (plus resident
55
indexes). This makes "it silently scanned" detectable and assertable rather than
6-
a mystery — the top human-factors need from the design review (P0-1).
6+
a mystery — the top human-factors need from the design review.
77
"""
88
from __future__ import annotations
99

1010
from typing import Any, Dict, cast
1111

1212
from graphistry.Engine import resolve_engine
13-
from .api import index_trace, get_registry, show_indexes
13+
from .api import index_trace, show_indexes
1414

1515

1616
def gfql_explain(g: Any, query: Any, *, index_policy: str = "use", engine: str = "auto") -> Dict[str, Any]:

0 commit comments

Comments
 (0)