Repository navigation
Accelerate neck cutoff detection with batched KDTree queries - #49
Conversation
Replace repeated scalar KDTree radius queries with a bounded batched search while preserving exact first-hit, threshold, closest-candidate, tie-break, and refinement semantics. Add detailed geometry timing, detector benchmarks, randomized differential validation, and exact 5,000-step solver parity evidence.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7fd850bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elapsed = perf_counter() - t_neck0 | ||
| timing["neck"] = float(timing.get("neck", 0.0) + elapsed) | ||
| timing["geometry_neck"] = float(timing.get("geometry_neck", 0.0) + elapsed) |
There was a problem hiding this comment.
When a cutoff is found, this elapsed interval also covers plot_cut, save_xy_cut, point-mask construction, array slicing, and subsequent searches. neck_detector_solver_ab._timing_summary nevertheless exposes geometry_neck as neck_detector_seconds and uses it to calculate the detector speedup, so runs containing cutoffs can report mostly output and cutoff-processing time rather than the scalar-versus-batched query cost. Use a separate timer around find_neck_cutoff_kdtree_with_refine for that comparison, or label this metric as the full cutoff phase.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed. geometry_neck remains the full cutoff-processing phase for compatibility, and a new geometry_neck_detector subset now measures only calls to find_neck_cutoff_kdtree_with_refine. The detector subset is excluded from additive geometry accounting.
I reran the exact scalar/batched A/B validation. At 5,000 steps both runs completed with identical state histories, 44 identical cutoff events, identical selected pairs, and identical points removed.
Updated paired timings:
detector only: 106.610 s → 18.785 s (5.675×)
full cutoff phase: 106.796 s → 18.946 s (5.637×)
geometry: 127.566 s → 37.100 s (3.438×)
whole solver: 161.731 s → 66.780 s (2.422×)
Full suite: 391 passed, 1 skipped.
Separate detector-only timing from the full cutoff-processing phase, preserve existing geometry accounting semantics, and report validated detector and cutoff-phase speedups independently.
1 participant
Summary
This PR accelerates neck-cutoff detection in
geometry4by replacing repeated scalarcKDTree.query_ball_point()calls with a bounded batched search.The new implementation preserves the existing detector contract exactly:
ijdomain< dslim3cutoff testjjtie breakThe scalar implementation is retained as a private reference for validation.
The validated production batch size is: