perf(chain): backward pass and output_min_hops optimizations#885
Merged
Conversation
lmeyerov
changed the base branch from
feat/issue-837-cudf-hop-executor
to
master
lmeyerov
force-pushed
the
refactor/df-executor-traversal-primitives
branch
from
3a369dd to
7e15c93
Compare
Make it explicit that the O(n!) oracle path is for testing only and should never be called from production code paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When the start node is filtered, hop labels accurately identify path positions. When unfiltered, all edges become "hop 1" from some start, making labels useless. Fix: In _filter_multihop_by_where, check if start node is filtered: - Filtered: use hop labels to identify start/end nodes (original behavior) - Unfiltered: use alias frames directly as fallback Also renamed _run_oracle to _run_test_only_oracle to prevent accidental production usage of the O(n!) reference implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…iants Based on 5-whys analysis of the unfiltered start bug, add systematic coverage for the interaction matrix of: start filter × edge direction × WHERE clause. New tests: - test_unfiltered_start_multihop_reverse - test_unfiltered_start_multihop_undirected - test_filtered_start_multihop_reverse_where - test_filtered_start_multihop_undirected_where All tests pass, confirming the fix handles all direction variants correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add two local helper functions to df_executor.py: - _build_edge_pairs: normalize edges for BFS traversal based on direction - _bfs_reachability: vectorized BFS with hop distance tracking These helpers consolidate duplicated BFS logic in: - _filter_multihop_edges_by_endpoints (-55 lines) - _find_multihop_start_nodes (-22 lines) Net result: -35 lines in df_executor.py with no new files. All 275 GFQL tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Optimize combine_steps() to avoid re-running forward ops for single-hop edge chains. Instead of calling op() again (which does a full hop traversal), filter edges by valid src/dst endpoints from the backward- validated node sets. Key changes: - Use vectorized merge instead of set + isin for large graph performance - For multi-hop edges, fall back to the original re-run approach Performance improvement for single-hop chains: - medium (10K nodes): 145ms → 41ms (72% faster) - large (100K nodes): 207ms → 126ms (39% faster) Chain is now faster than df_executor for large graphs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Verify that df_executor (with WHERE) produces same features as chain: - Named alias boolean tags - Hop labels (label_edge_hops) - Output slicing (output_min_hops/output_max_hops) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
For single-hop edges without hop labels, use vectorized merge filtering instead of calling op.reverse()() which triggers a full hop() traversal. This saves ~50% of backward pass time. Performance improvement across all graph sizes: - tiny (100 nodes): 31ms → 25ms (19% faster) - small (1K nodes): 33ms → 26ms (21% faster) - medium (10K nodes): 40ms → 33ms (18% faster) - large (100K nodes): 151ms → 109ms (28% faster) Chain is now competitive with df_executor on medium graphs and wins on large graphs. The optimization: 1. Detects simple single-hop edges (min=1, max=1, no hop labels) 2. Filters edges by valid endpoints from wavefront sets 3. Computes result nodes as the backward traversal targets Falls back to full hop() traversal for multi-hop or labeled edges. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ations Comprehensive test coverage for chain.py optimizations: TestBackwardPassOptimization (15 tests): - TestOptimizationEligibility: Verify _is_simple_single_hop correctly identifies eligible edges (single-hop, no labels) vs ineligible (multi-hop, labeled) - TestDirectionSemantics: Verify forward/reverse/undirected return correct nodes - TestEdgeCases: Empty results, disconnected components, self-loops, parallel edges - TestResultCorrectness: Tags and attributes preserved correctly TestCombineStepsOptimization (7 tests): - Single-hop endpoint filtering for all directions - Hop label preservation TestChainDFExecutorParity (6 tests): - Same nodes/edges with and without WHERE - Complex patterns (diamond, filtered mid-node) - WHERE clause filtering 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…h oracle
- output_min_hops filters edges by hop number, includes all their endpoints
- Seeds (hop=0 or NA) excluded when output_min_hops set (not on output edges)
- Edge endpoint coverage ensures valid graph output
- Updated tests to match oracle expectations:
- test_output_min_hops_filters_early_hops: expects {b,c,d}
- test_output_max_hops_filters_late_hops: expects {a,b,c}
- test_output_slice_both_bounds: expects {b,c}
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reuse existing _is_simple_single_hop helper instead of inline check that incorrectly detected multi-hop when hops=None from JSON. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- to_fixed_point=True means unbounded traversal, not single-hop - Added test for to_fixed_point eligibility check 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
to_fixed_point is a direct attribute of ASTEdge, no need for getattr. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add type check to ensure where field is a list before casting, preventing confusing runtime errors from malformed JSON. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add type checks for: - payload is a dict - payload has 'left' and 'right' keys - left/right values are strings Prevents confusing KeyError/TypeError from malformed JSON. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move sequence type check from chain.py into parse_where_json. Remove redundant check and unsafe cast from chain.py. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move from standalone function in chain.py to method on ASTEdge class. This centralizes edge-related logic with the edge class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract common edge filtering logic into _filter_edges_by_endpoint() - Simplify directed edge filtering (forward/reverse) using helper - Condense output_min/max_hops filter block - Tighten fast backward pass code Reduces diff by ~20% while maintaining functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Condense prev/next node lookups to single lines - Simplify apply_output_slice function - Tighten multihop recompute loop - Simplify backward pass fallback - Extract prev_set/target_set upfront in fast backward Reduces insertions from 307 to 166. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove WHERE clause and df_executor functionality from this PR to focus on chain optimizations only. WHERE functionality will be added in a stacked PR. Changes: - Remove same_path_types, same_path_plan, df_executor modules - Remove Chain.where field and WHERE imports from chain.py - Simplify gfql_unified.py by removing _chain_dispatch helper - Remove WHERE-related test files and test classes - Keep all chain optimization tests (78 tests passing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix E127 continuation line over-indented - Fix W504 line break after binary operator 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add assertions to narrow types in fast backward pass: - assert isinstance(op, ASTEdge) for direction access - assert node_id/src_col/dst_col not None for filter helper 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…/compare The same_path_types module was removed in the PR split. Import col and compare from the enumerator module which has equivalent definitions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove WHERE/df_executor entries (moving to stacked PR 886) - Add Performance section for backward pass optimization - Add test entry for 78 chain optimization tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
lmeyerov
force-pushed
the
refactor/df-executor-traversal-primitives
branch
from
b1b115c to
c14d079
Compare
…(belong in PR 886)
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
Changes
Performance
hop()in backward pass for simple single-hop edges (19-28% speedup)combine_stepsfor single-hop chains (14-22% speedup)Bug Fix
output_min_hopsnow correctly filters edges by hop number, includes all their endpointsoutput_min_hopsset (they're not on output edges)Tests
Related
Test Plan
python -m pytest tests/gfql/ref/test_chain_optimizations.py- 81 passedpython -m pytest tests/gfql/ref/test_enumerator_parity.py- 22 passedpython -m pytest tests/gfql/- 359 passed🤖 Generated with Claude Code