gfql: engage the #1658 seeded index on Polars/cuDF (identity-stable coercion #1726 + small-frontier cost gate)#1727
Open
lmeyerov wants to merge 3 commits into
Open
Conversation
…polars (#1726) _pl_nan_to_null returned a fresh polars frame whenever any Float32/Float64 column existed (keyed on column presence, not actual NaN), churning frame identity so the #1658 CSR index's `source_ref is df` check failed -> polars/polars-gpu direct hops always fell back to the O(E) scan. Fix: probe is_nan().any() per float column (dtype-gated) and return the SAME frame object when no NaN is present; rebuild only NaN-carrying columns when present. Value-identical (fill_nan(None) is a no-op on NaN-free columns); only object identity changes -> the resident index now engages. Verified on the real SNB graph: polars flipped 120/120-scan -> 54-index/12-scan (remaining 12 = cost-gate, separate). Adds pure-polars TestPlNanToNull. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
…y homogeneous slices index on cudf/polars (#1726 follow-up) The seeded-hop planner's cost gate bails to the O(E) scan when frontier_n >= frac * n_keys. frac is engine-aware (pandas 0.5, cudf/polars/ polars-gpu 0.02), so on small / low-cardinality edge slices (per-edge-type homogeneous frames: n_keys <= 1/0.02 = 50) even a single-seed hop tripped the gate on the vectorized engines and scanned O(E) with a resident O(degree) index. On the real SNB graph this left ~12/60 native hops scanning on polars (~6 on cudf) after #1726 made the index engage at all. Fix: an absolute small-frontier floor. A frontier of <= 16 seeds (_COST_GATE_MIN_FRONTIER_DEFAULT, env-overridable via GFQL_INDEX_COST_GATE_MIN_FRONTIER; 0 disables) never bails on the frac gate; larger frontiers keep the existing frac-based bulk-hop protection unchanged. Constant (not a function of n_keys) on purpose: scaling with n_keys is the frac gate's job; the floor bounds absolute per-hop probe work in exactly the regime where frac*n_keys collapses below a handful of seeds. A <=16-seed frontier costs at most 16 searchsorted probes plus a gather of exactly the rows the scan would emit, so the index cannot meaningfully lose; the case the gate protects (a frontier covering most distinct sources) requires frontier > 16, where behavior is unchanged. Uniform across engines — pandas (frac 0.5) only overlaps the floor when n_keys <= 32, micro slices where either path is trivially fast. Correctness-invariant by construction: the gate is a pure routing heuristic — index_seeded_hop is parity-matched to the scan (traverse.py contract, test_index_parity_vs_scan), so this changes which path runs, never the answer. Also surfaces the floor in the explain trace (min_frontier_floor) and extends the bail reason to name it. Adds pure-CPU tests: tiny-slice single-seed hop now indexes on every engine (with scan-parity check), frontier just past the floor still bails, env tuning validation, and floor=0 restoring pure frac gating. Static gates: py_compile + ruff check clean on all touched files; mypy (mypy.ini via bin/mypy.sh) clean except two pre-existing errors in the untouched lazy/engine/polars/degrees.py (present at HEAD). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
… the polars coverage lane The _pl_nan_to_null identity tests (test_engine_coercion.py) only execute where polars is installed, but the sole Engine.py-measuring coverage lane (core py3.14) has no polars extra, so the new eager-DataFrame branch showed 0% changed-line coverage despite having real behavior tests. Run those tests in the polars lane and widen its coverage source from graphistry/compute to the graphistry package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes that let the #1658 seeded adjacency index actually engage on Polars / Polars-GPU (and tidy up cuDF) direct
hop()calls. Both are routing-only — the seeded index and the O(E) scan return identical results by the #1658 doctrine, so parity is invariant; these only change which path runs.1. Identity-stable
_pl_nan_to_null(#1726) —graphistry/Engine.py_pl_nan_to_nullreturned a fresh Polars frame whenever the frame had anyFloat32/Float64column (keyed on column presence, not on an actual NaN).hop()coerces inputs through this before the index hook, so the live edge frame's object identity churned and the resident index'ssource_ref is dfcheck failed → Polars/Polars-GPU seeded hops always fell back to the scan.Fix: probe
is_nan().any()per float column (dtype-gated) and return the same frame object when no NaN is present; rebuild only NaN-carrying columns when present. Value-identical to before (fill_nan(None)is a no-op on NaN-free columns); only object identity changes, so the resident index now engages. Adds pure-PolarsTestPlNanToNull(identity preserved when NaN-free; NaN→null still correct when present; real nulls untouched).2. Small-frontier cost-gate floor —
graphistry/compute/gfql/index/{cost,api,types,__init__}.pyThe seeded-index cost gate bails to scan when
frontier_n >= frac * n_keys(frac 0.5 pandas, 0.02 cuDF/Polars). For a single-node seed on a small/low-cardinality homogeneous slice,0.02 * n_keyscollapses below a handful of seeds, so the gate scans even though the index would be O(degree). Adds an absolute small-frontier floor: bail only whenfrontier_n > MIN_FRONTIER (16) AND frontier_n >= frac * n_keys. Named constant, env-overridable (GFQL_INDEX_COST_GATE_MIN_FRONTIER). The large-frontier bulk-hop protection is unchanged (fires only above the floor). Adds 3 tests (tiny slice indexes; large frontier still scans; floor tunable/disable-able).Correctness / risk
test_index_parity_vs_scan(index==scan, 4 engines × 8 scenarios) andtest_cost_gate_engine_aware_never_loses_to_scan.Verified qualitatively on a large heterogeneous graph: Polars seeded hops flipped from all-scan to index-engaging, results identical.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV