perf(gfql): identity-stable _pl_nan_to_null + clean-cache (kills O(E) per-hop NaN re-scan on polars)

CHANGELOG.md

@@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - **GFQL engine-selection docs (pandas / polars / cuDF / polars-gpu)**: New :doc:`Choosing a GFQL Engine <gfql/engines>` page — a numbers-first, persona-tested guide to the four interchangeable engines. Adds the one-keyword `engine='polars'` speedup (up to ~38× over pandas on real graphs, no GPU), a motivating warm-median comparison table on real public graphs (LiveJournal 35M / Orkut 117M), a decision matrix (workload shape × size × hardware → engine, with the measured ~10K-edge CPU crossover, GPU-work-bound rule, polars-gpu memory-pressure caveat, and GPU-or-error contract), a cuDF-vs-polars-gpu disambiguation (eager-op vs fused-lazy; cuDF is not deprecated), an honest "when *not* to use Polars" section, the differential-parity guarantee, and a methodology + reproducer-script disclosure. Rewrote the top of `gfql/performance.rst` to lead with the engine comparison (de-marketed the prose), wired the new page into the GFQL toctree + recommended paths, and added Polars/polars-gpu to the engine examples in `gfql/quick.rst` and `gfql/about.rst` (previously only pandas/cuDF were documented). Driven by 4-persona doc user-testing (pandas DS, RAPIDS user, perf engineer, skeptical evaluator).
 
 ### Fixed
+- **GFQL polars/polars-gpu seeded traversal no longer pays an O(E) NaN re-scan per hop on float-column graphs (identity-stable `_pl_nan_to_null` + clean-cache)**: every `hop()` runs `_coerce_input_formats` → `_pl_nan_to_null`, which scanned `is_nan().any()` over every float column on the (unchanged) resident edge frame **on every call** — so repeated seeded hops on a resident graph (the seeded-Search / native-hop pattern) grew O(E) with edge count on polars while pandas stayed flat. Measured: an indexed polars `g.hop` on 8M edges with one float column was **33.8 ms and growing**; it is now **0.20 ms and FLAT** (~140× faster; O(degree)), matching pandas. `_pl_nan_to_null` now probes an eager polars frame for real NaN once, returns a clean frame UNCHANGED (restoring the #1726 identity guard that #1731 reverted — so frame-identity caches like the #1658 index keep engaging), rewrites only the columns that genuinely carry NaN (values identical to the old unconditional `fill_nan`), and caches the id of frames verified clean so later calls skip the probe (recycle-safe via `weakref.finalize`; a rebound/new frame re-probes, so NaN→null semantics are unchanged). Regression: full 4-engine `test_index.py` parity + new `TestPlNanCleanCache` cases (NaN-present cleaned, clean-frame cached same-object, distinct frames independent).
 - **GFQL Cypher `CASE` with mixed-dtype branches is now engine-consistent on cuDF (no more `GFQLTypeError`)**: pandas coerces the two `CASE` branches to a common type, but cuDF's `.where` raised `TypeError: cudf does not support mixed types`, surfaced as a hard `GFQLTypeError`. This bit `CASE WHEN path IS NULL THEN -1 ELSE length(path) END` over an UNREACHABLE `shortestPath` (int `-1` branch vs an object/null hops branch) — pandas returned `-1`, cuDF errored. The row-AST `CASE` evaluator now unifies the branch dtypes and retries, so cuDF returns the same value pandas does. Regression test `test_case_mixed_dtype.py`.
 - **GFQL polars engine raises a clean typed error (not an opaque polars `InvalidOperationError`) for a string predicate on a non-string column**: `WHERE col STARTS WITH/CONTAINS/regex ...` on a Categorical/Enum/numeric column raised a clean `GFQLSchemaError` ("string predicate used on non-string column") on pandas/cuDF but leaked polars' internal `InvalidOperationError: expected String type, got: cat` on polars. `filter_by_dict_polars` now raises the same `GFQLSchemaError` (categorical treated as non-string, exactly as `filter_by_dict`), so all three engines agree. Regression test `test_polars_string_predicate_nonstring.py`; also confirms connected-join predicate pushdown is parity-correct on polars for string/numeric/eq/bool columns.
 - **GFQL string predicates (`Contains`/`Startswith`/`Endswith`/`Match`/`Fullmatch`) are now value-safe on non-string columns**: applying a string predicate to a numeric/temporal/bool column raised an opaque `AttributeError: Can only use .str accessor with string values!` on pandas and cuDF. They now follow openCypher semantics — a string op over a non-string value is null → excluded (matching the established per-cell behavior on an object column holding non-strings) — and never stringify the column (which would diverge pandas↔cuDF, e.g. wrongly matching `5 CONTAINS '5'`). String, mixed-object, and all-null columns are unchanged. Regression tests in `test_str.py` across dtypes; cuDF parity confirmed.

bin/test-polars.sh

@@ -23,6 +23,7 @@ POLARS_TEST_FILES=(
     graphistry/tests/compute/gfql/test_polars_string_predicate_nonstring.py
     graphistry/tests/compute/gfql/cypher/test_order_by_null_placement.py
     graphistry/tests/compute/gfql/test_conformance_ledger.py
+    graphistry/tests/compute/gfql/test_polars_nan_clean.py
     # index tests exercise the seeded-index hook in the polars hop entry (hop.py) — without
     # them the hook dominates the now-thin file and trips its per-file coverage floor
     graphistry/tests/compute/gfql/index/test_index.py

graphistry/Engine.py

@@ -207,21 +207,6 @@ def active_frames_are_polars(g: Any) -> bool:
     return is_polars_df(g._edges)
 
 
-def _pl_nan_to_null(df):
-    """Convert NaN -> null in float columns of a polars frame.
-
-    Matches ``pl.from_pandas(nan_to_null=True)`` (the pandas-input path) so a *native*
-    polars / Arrow / cuDF input carrying genuine NaN is treated as MISSING like the pandas
-    oracle (which skipna/dropna's NaN). Without this, ``engine='polars'`` on a frame with a
-    real NaN keeps rows a filter/aggregation should drop (silent divergence from pandas).
-    No-op when there are no float columns."""
-    import polars as pl
-    float_cols = [c for c, dt in df.schema.items() if dt in (pl.Float32, pl.Float64)]
-    if not float_cols:
-        return df
-    return df.with_columns([pl.col(c).fill_nan(None) for c in float_cols])
-
-
 def df_to_engine(df, engine: Engine, *, validate: Optional[ValidationParam] = None, warn: bool = True):
     """Convert ``df`` to ``engine``'s frame type.
 
@@ -279,6 +264,9 @@ def df_to_engine(df, engine: Engine, *, validate: Optional[ValidationParam] = No
         return dd.from_pandas(df, npartitions=1)
     elif engine in POLARS_ENGINES:
         import polars as pl
+        # polars-engine-specific NaN->null coercion lives with the polars engine; local
+        # import (lazy/... never imports Engine at module load -> no cycle)
+        from graphistry.compute.gfql.lazy.engine.polars.nan_clean import _pl_nan_to_null
         if isinstance(df, pl.DataFrame):
             return _pl_nan_to_null(df)
         if isinstance(df, pl.LazyFrame):

graphistry/compute/ComputeMixin.py

@@ -106,7 +106,8 @@ def _is_already_correct(df: Any) -> bool:
     # should drop (silent divergence from the pandas oracle, which treats NaN as missing).
     # _pl_nan_to_null is idempotent, so re-running it on a just-converted frame is a no-op.
     if engine in POLARS_ENGINES:
-        from graphistry.Engine import _pl_nan_to_null, is_polars_df
+        from graphistry.Engine import is_polars_df
+        from graphistry.compute.gfql.lazy.engine.polars.nan_clean import _pl_nan_to_null
         if g._edges is not None and is_polars_df(g._edges):
             g = g.edges(_pl_nan_to_null(g._edges), g._source, g._destination)
         if g._nodes is not None and is_polars_df(g._nodes):

graphistry/compute/gfql/lazy/engine/polars/nan_clean.py

@@ -0,0 +1,66 @@
+"""NaN -> null coercion for polars frames entering GFQL (pandas missing-semantics parity).
+
+pandas treats float NaN as MISSING (skipna/dropna drop it); polars distinguishes NaN from
+null. Frames entering the GFQL surface are coerced here so polars/Arrow/cuDF input carrying
+genuine NaN is treated as MISSING like the pandas oracle. Without this, ``engine='polars'``
+on a frame with a real NaN keeps rows a filter/aggregation should drop (silent divergence
+from pandas). Polars imported lazily (optional dependency), per engine convention.
+"""
+from __future__ import annotations
+
+import weakref
+from typing import TYPE_CHECKING, Set
+
+if TYPE_CHECKING:
+    import polars as pl
+    from .dtypes import PolarsT
+
+
+# Ids of polars frames already verified NaN-free (or produced NaN-free by cleaning).
+# Recycle-safe: a weakref.finalize on each cached frame evicts its id on GC, so a reused
+# id can never be a stale hit while the original frame is alive. This turns the repeated
+# per-hop NaN probe on a RESIDENT graph (seeded Search / native-hop hammers the same edge
+# frame every call) from O(E)-per-call into O(1) after the first check — the dominant
+# per-call cost for polars/polars-gpu seeded traversal on float-column (i.e. real) graphs.
+_PL_NAN_CLEAN_IDS: Set[int] = set()
+
+
+def _mark_pl_nan_clean(df: "pl.DataFrame") -> None:
+    key = id(df)
+    _PL_NAN_CLEAN_IDS.add(key)
+    try:
+        weakref.finalize(df, _PL_NAN_CLEAN_IDS.discard, key)
+    except TypeError:  # pragma: no cover - pl.DataFrame is weakref-able; guard anyway
+        _PL_NAN_CLEAN_IDS.discard(key)  # can't track lifetime -> don't cache (stay correct)
+
+
+def _pl_nan_to_null(df: "PolarsT") -> "PolarsT":
+    """Convert NaN -> null in float columns of a polars frame.
+
+    Matches ``pl.from_pandas(nan_to_null=True)`` (the pandas-input path) so a *native*
+    polars / Arrow / cuDF input carrying genuine NaN is treated as MISSING like the pandas
+    oracle (which skipna/dropna's NaN). No-op when there are no float columns.
+
+    Identity-stable + O(1)-repeat: an eager DataFrame is probed once for real NaN
+    (``is_nan().any()`` per float column). A frame verified clean is returned UNCHANGED
+    (same object) and its id is cached so subsequent calls skip the O(E) probe entirely;
+    only columns that genuinely carry NaN are rewritten (values identical to the old
+    unconditional ``fill_nan`` — it never touches non-NaN cells). This restores the #1726
+    identity guard (reverted by #1731) AND removes the per-call O(E) re-scan that made
+    polars/polars-gpu seeded Search grow with edge count (see plans/gfql-benchmark-numbers)."""
+    import polars as pl
+    float_cols = [c for c, dt in df.schema.items() if dt in (pl.Float32, pl.Float64)]
+    if not float_cols:
+        return df
+    if isinstance(df, pl.DataFrame):
+        if id(df) in _PL_NAN_CLEAN_IDS:
+            return df
+        nan_cols = [c for c in float_cols if df.get_column(c).is_nan().any()]
+        if not nan_cols:
+            _mark_pl_nan_clean(df)
+            return df
+        cleaned = df.with_columns([pl.col(c).fill_nan(None) for c in nan_cols])
+        _mark_pl_nan_clean(cleaned)
+        return cleaned
+    # LazyFrame (rare): no cheap eager NaN probe -> keep the unconditional rewrite.
+    return df.with_columns([pl.col(c).fill_nan(None) for c in float_cols])

graphistry/tests/compute/gfql/coverage_baselines/ci-polars-py3.12.json

@@ -9,6 +9,7 @@
     "graphistry/compute/gfql/lazy/engine/polars/dtypes.py": 92.0,
     "graphistry/compute/gfql/lazy/engine/polars/hop.py": 87.0,
     "graphistry/compute/gfql/lazy/engine/polars/hop_eager.py": 90.0,
+    "graphistry/compute/gfql/lazy/engine/polars/nan_clean.py": 90.0,
     "graphistry/compute/gfql/lazy/engine/polars/pattern_apply.py": 67.0,
     "graphistry/compute/gfql/lazy/engine/polars/predicates.py": 83.0,
     "graphistry/compute/gfql/lazy/engine/polars/projection.py": 65.0,

graphistry/tests/compute/gfql/test_polars_nan_clean.py

@@ -0,0 +1,60 @@
+"""_pl_nan_to_null: NaN->null correctness + identity-stable + O(1)-repeat clean cache."""
+import pytest
+
+pl = pytest.importorskip("polars")
+
+from graphistry.compute.gfql.lazy.engine.polars.nan_clean import (  # noqa: E402
+    _PL_NAN_CLEAN_IDS,
+    _pl_nan_to_null,
+)
+
+
+def test_no_float_cols_is_noop_same_object():
+    df = pl.DataFrame({"a": [1, 2, 3], "s": ["x", "y", "z"]})
+    out = _pl_nan_to_null(df)
+    assert out is df
+
+
+def test_nan_present_is_cleaned_to_null():
+    df = pl.DataFrame({"w": [1.0, float("nan"), 3.0]})
+    out = _pl_nan_to_null(df)
+    # NaN -> null: null_count reflects the converted cell; no NaN remains.
+    assert out.get_column("w").null_count() == 1
+    assert not bool(out.get_column("w").is_nan().any())
+
+
+def test_clean_float_frame_identity_stable_and_cached():
+    df = pl.DataFrame({"w": [1.0, 2.0, 3.0]})  # float col, no NaN
+    out1 = _pl_nan_to_null(df)
+    assert out1 is df                        # clean -> unchanged (identity-stable)
+    assert id(df) in _PL_NAN_CLEAN_IDS       # cached
+    out2 = _pl_nan_to_null(df)
+    assert out2 is df                        # O(1) cache hit -> same object
+
+
+def test_distinct_frames_do_not_cross_contaminate():
+    clean = pl.DataFrame({"w": [1.0, 2.0]})
+    _pl_nan_to_null(clean)                          # caches `clean`
+    dirty = pl.DataFrame({"w": [float("nan"), 2.0]})  # different object, has NaN
+    out = _pl_nan_to_null(dirty)                    # must NOT be skipped by the cache
+    assert out.get_column("w").null_count() == 1
+
+
+def test_cleaned_output_is_cached_and_gc_evicts():
+    import gc
+
+    dirty = pl.DataFrame({"w": [float("nan"), 2.0]})
+    cleaned = _pl_nan_to_null(dirty)
+    assert id(cleaned) in _PL_NAN_CLEAN_IDS  # output cached too (resident-graph reuse)
+    assert _pl_nan_to_null(cleaned) is cleaned
+    key = id(cleaned)
+    del cleaned
+    gc.collect()
+    assert key not in _PL_NAN_CLEAN_IDS      # weakref.finalize evicted -> no stale id reuse
+
+
+def test_lazyframe_keeps_unconditional_rewrite():
+    lf = pl.DataFrame({"w": [1.0, float("nan")]}).lazy()
+    out = _pl_nan_to_null(lf)
+    assert isinstance(out, pl.LazyFrame)
+    assert out.collect().get_column("w").null_count() == 1

graphistry/tests/compute/test_engine_coercion.py

@@ -686,3 +686,4 @@ def test_chain_dask_edges(self):
 
 if __name__ == "__main__":
     unittest.main()
+