Skip to content

docs(gfql): print the benchmark numbers pyg-bench publishes, and re-verify them - #1818

Merged
lmeyerov merged 3 commits into
masterfrom
docs/bench-numbers-consumer
Jul 28, 2026
Merged

docs(gfql): print the benchmark numbers pyg-bench publishes, and re-verify them#1818
lmeyerov merged 3 commits into
masterfrom
docs/bench-numbers-consumer

Conversation

@lmeyerov

Copy link
Copy Markdown
Contributor

Do not merge without human review.

Supersedes #1813, which the owner redirected: "i don't get why this big file is here when we also have docs/source/_ext/gfql_bench.py. more broadly, why is this different from the pyg-bench stuff? should all that be there, and all we do is pretty print their latest json? and if we have any boundary needs, ensure there are data contract tests that run on the pyg-bench side, and we re-verify on our side?"

This is the consuming half of that factoring. The producing half is pyg-bench graphistry/pyg-bench#113 (merged) and graphistry/pyg-bench#114.

#1813 here
gate bin/check_bench_numbers.py, 266 lines none — the rules live in pyg-bench
extension 347 + 420 lines across two modules 1 module
data generated JSON, unversioned shape vendored artifact + vendored contract

What this adds

  • docs/source/_data/gfql_benchmarks.json — a vendored copy of pyg-bench's published/docs-numbers.json.
  • docs/source/_data/gfql_benchmarks.contract.json — a vendored copy of the contract it satisfies.
  • docs/source/_ext/gfql_bench.py — resolves :bench: / :bench-diag:, renders bench-provenance / bench-disclosures, and re-verifies the contract before rendering anything.
  • docs/test_bench_numbers.py — the same checks in the ordinary test lane.

Build failures, never silent text: a key that is not published; a run measured longer ago than policy.max_age_days; a non-quotable cell printed as a bare number; a page that drops a referenced number's provenance or disclosures; any contract violation at all. Each of those has a test that watches it fail.

The re-verification is written against the contract document, independently of pyg-bench's implementation. A shared library would fail identically on both sides of the boundary and the re-verification would prove nothing.

benchmark_filter_pagerank.rst

Its figures came from a chart generator reading plans/gfql-gpu-pagerank-benchmark/results/ — a path that has never existed in any commit, and plans/ is gitignored, so nobody could re-derive them. pyg-bench ported the runners unchanged and re-ran them on dgx-spark under the perf lock against pygraphistry 49db91cc, with the raw artifacts committed.

Neo4j + GDS GFQL CPU GFQL GPU GPU vs Neo4j
Twitter (2.4M edges) 11.72 s 1.58 s 0.24 s 49.5x
GPlus (30M edges) did not complete 32.10 s 2.42 s

Every arm reproduces the earlier run's final subgraphs exactly, node for node and edge for edge. The old "46x" measures 49.5x now. The old GPlus ">56x" is gone: that arm lost its connection mid-transaction and never reached a timed run, and a ratio against a run that did not finish is not a speedup.

The two lifecycle SVGs are removed. They render the withdrawn figures as glyph paths and their generator cannot be run.

Verification

  • docs/ci.sh run locally (Dockerised Sphinx): build succeeded, extension live, table and provenance render. A docs-only diff skips test-docs in CI (it needs: python-lint-types), so green CI would not have proven the docs built.
  • pytest docs/test_bench_numbers.py — 13 passed.
  • ruff clean on both new files. .github/workflows/ci.yml untouched.

Not in scope

The hand-typed-literal guard over policy.managed_docs is not here; it would collide with the factual-correction PR currently touching performance.rst / engines.rst. Worth a follow-up once those land. demos/gfql/benchmark_filter_pagerank_cpu_gpu.ipynb still restates the withdrawn Twitter figures (13.51 s / 45x) and needs the same treatment.

lmeyerov and others added 2 commits July 27, 2026 20:43
…erify them

The owner's redirect on #1813: "should all that be there, and all we do is
pretty print their latest json? and if we have any boundary needs, ensure there
are data contract tests that run on the pyg-bench side, and we re-verify on our
side?" This is the consuming half. Measurement, provenance and publishability
live in pyg-bench, which owns the runs. This repository renders them.

`docs/source/_data/gfql_benchmarks.json` is a vendored copy of pyg-bench's
`published/docs-numbers.json`; `gfql_benchmarks.contract.json` is a vendored
copy of the contract it satisfies. The `gfql_bench` extension resolves `:bench:`
and `:bench-diag:` against the artifact and re-verifies the contract before
rendering anything, so the build fails on a key that is not published, a
measurement older than the policy allows, a diagnostic-only figure printed as a
result, or a page that drops a number's provenance or disclosures. The
re-verification is written against the contract DOCUMENT, independently of
pyg-bench's implementation - a shared library would fail identically on both
sides and prove nothing.

`benchmark_filter_pagerank.rst` is the first page to use it. Its numbers came
from a chart generator reading a results directory that has never existed in any
commit, so nobody could re-derive them; pyg-bench re-ran the benchmark on
dgx-spark against a named commit and published the result. Twitter: GFQL GPU
0.24s and CPU 1.58s against Neo4j + GDS 11.72s, 49.5x and 7.4x. GPlus: GFQL GPU
2.42s and CPU 32.10s, 13.3x. The two lifecycle SVGs are removed - they render
the withdrawn figures as glyph paths and their generator cannot be run.

GPlus publishes no Neo4j figure. That arm did not complete, and a ratio against
a run that did not finish is not a speedup.

`docs/test_bench_numbers.py` runs the contract checks in the ordinary test lane
rather than only in the docs build, so a number going stale fails CI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
CI's minimal lane has no docutils, so putting the contract re-verification, the
staleness rule and the point-of-use decision in the Sphinx extension meant the
whole gate could only run in the docs build - one job out of forty, and the
pattern this repository has just finished digging out of.

`gfql_bench_data` is now stdlib-only and holds every rule; `gfql_bench` is the
docutils half that renders them and turns a recorded problem into a failed
build. A test asserts the rules module never grows a docutils or sphinx import
again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
@lmeyerov

Copy link
Copy Markdown
Contributor Author

CI: 57 SUCCESS / 11 SKIPPED / 1 CANCELLED.

The one cancelled cell is test-polars (3.12), which hit the 10-minute step timeout at 10m16s. It is cancelled on master itself at 49db91c, the commit this branch is based on (run 30324476067) — the budget defect #1814 addresses, not this diff, which touches no polars code.

test-docs ran and passed here because the diff includes a Python file; a purely docs-only diff would have skipped it (it needs: python-lint-types). docs/ci.sh was also run locally against the final tree: Dockerised Sphinx build succeeded, notebook validation passed, exit 0.

One correction landed after the first CI round, worth flagging to a reviewer: the first draft put the contract rules inside the Sphinx extension, so the minimal lane could not import them (no docutils) and the entire gate would have run in the docs build alone. The rules now live in a stdlib-only gfql_bench_data, with a test that stops a docutils import creeping back in.

@lmeyerov
lmeyerov merged commit 76bdee6 into master Jul 28, 2026
69 checks passed
lmeyerov added a commit that referenced this pull request Jul 28, 2026
…a too

#1818 landed the pyg-bench docs-numbers contract and its data file, which
ships graphbench.{20k,100k}.q8.polars and .q8.polars_vs_kuzu as
board_quotable=true with an empty disclosures list. That is the same claim
this PR withdraws from the CHANGELOG, in a second location, so it is
corrected the same way rather than left to be found later.

The correction is expressed through the contract's own mechanism, not around
it. Two invariants define the shape:

  quotable_implies_comparable      board_quotable implies comparison_allowed
  caveat_travels_with_the_number   status != 'ok' or comparison_allowed is
                                   false implies disclosures is non-empty

So the four cells become comparison_allowed=false (a warm-memo figure is not
a fair one-shot comparison), board_quotable=false (forced by the first
invariant), and carry a disclosure recording the warm/cold/fresh arms, the
bind_only control, and the one-shot loss factors. Re-checked all 119 cells
against cell_required, both quotable invariants, caveat_travels_with_the_number
and cell_names_a_known_run: zero violations. Diff is exactly those 4 cells;
kuzu and pandas cells are untouched.

This is an INTERIM correction on the consumer side. The durable fix is to
regenerate the run in pyg-bench and re-copy the artifact (#1827) -- these
figures are from graphbench-q1q9-*-20260726 and also predate #1828's q4
result, so the whole graphbench block wants a fresh run, not four hand-edits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
lmeyerov added a commit that referenced this pull request Jul 28, 2026
#1828 landed the low-cardinality count(*) gate in gfql_fast_paths.py, which is
the same file this branch adds its fused two-hop count lane to, so this needed a
real 3-way rather than the CHANGELOG-only resolution the earlier merges took.

git merge-file returns rc=0 with zero conflict markers, and the arithmetic
checks out: 2687 merged lines = 2538 (branch) + 149 (master-added), so nothing
was dropped. Structurally the two additions are disjoint -- this branch dispatches
the two-hop lane from _execute_two_hop_count_fast_path, while #1828 gates a
SINGLE-hop grouped count on _LOWCARD_COUNT_MAX_GROUPS / _MAX_INPUT_ROWS. They do
not gate on the same shape.

A clean textual merge is not on its own proof of semantic compatibility -- the
sibling PR #1819 hit exactly that trap, where a marker-free merge silently put
a new fused lane in front of a guarded twin. Flagging it here so review checks
the interaction rather than trusting rc=0; CI is the arbiter.

CHANGELOG resolved structurally: master file taken whole, this branch entry
re-inserted at the top of the same section it occupied.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant