Skip to content

Bench the alpha-shape shadow filter against the unfiltered search - #6590

Closed
meshinspector-agent[bot] wants to merge 2 commits into
alpha-shape-triangle-shadowfrom
agent/mlib-alpha-shape-bench-harness
Closed

Bench the alpha-shape shadow filter against the unfiltered search#6590
meshinspector-agent[bot] wants to merge 2 commits into
alpha-shape-triangle-shadowfrom
agent/mlib-alpha-shape-bench-harness

Conversation

@meshinspector-agent

@meshinspector-agent meshinspector-agent Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closed unmerged — and the table below is superseded, not merely stale. Do not read it as a verdict on the shadow filter. It was measured when every shadow test was exact, so it reads as the filter losing on the sphere and random clouds, which is no longer the case. The floating-point pre-filters are since restored — #6596 (wedge) and #6595 (query filter) — and they pay: on an 18820-point cloud at r=0.5, one binary per variant, interleaved, hashes equal, removing the wedge filter costs +30% and removing the query filter +41%, and the branch runs 4.46 s against master's 10.02 s. AlphaShapeData::shadowFilter never landed anywhere; DISABLED_AlphaShapeBench stays on the branch for per-variant numbers. See the closing comment.

The shadow filter is output-preserving, so switching it off is the master baseline: one binary now times both variants on the same runner, same cloud, same tree — the only way they are comparable on shared CI vCPUs.

  • AlphaShapeData::shadowFilter — new flag, no public signature change.
  • Min of 5 reps per variant, capped at 20 s of wall clock; cloud drops to 4k points where NDEBUG is off or under emscripten (MSVC iterator-debug, brew-llvm Debug and wasm share the 10-minute Unit Tests cap).
  • Ball radius scales with each cloud's sampling step, so a smaller cloud is not a sparser one and the filter-on/off ratio stays comparable across configs.
  • One greppable line per variant: [alpha-bench] <cloud> … filter=on|off reps= min= med= max= tris= hash= <counters>, plus a speedup= / inBallTestsRatio= line.
  • Asserted, not just printed: the triangle hash and count must agree filter-on vs filter-off.
  • Last commit drops the DISABLED_ prefix so every MRTest job of the next run reports numbers; it is restored before this branch reaches master.

First numbers — the agent's own self-hosted build box (8 vCPU, EPYC 7R13), not a GitHub-hosted runner (public repos get 4-core, macos-14 3-core); GCC 12.3 Release, 40k points, min of 5:

cloud filter off filter on speedup inBallTests off → on
sphere 40.8 ms 42.2 ms 0.97× 1.98M → 1.46M
grid 401 ms 348 ms 1.15× 6.42M → 3.06M
random 1412 ms 1695 ms 0.83× 62.2M → 23.7M

The filter removes the work it is meant to remove — 1.4–2.6× fewer ball tests, ~3× fewer considered triangles — but every shadow test is exact right now (54.5M of them on the random cloud), and that currently costs more than it saves except on the grid. That is the case for restoring the floating-point pre-filters, which the next increments measure the same way.

Verification

  • Built in the ubuntu22 image mirroring build-test-ubuntu-x64.yml: prebuilt thirdparty linked as lib/include/share, clone_submodules_linux.sh --skip-prebuilt-thirdparty, Ninja Release, g++-12, -DMR_CXX_STANDARD=20. MRTest target and its whole dependency graph built clean.
  • MRTest --no-python-tests --gtest_filter=*AlphaShape* → 4/4 pass, including the new AlphaShapeBench (19.8 s at 40k points; the table above is its output). AlphaShape, AlphaShapeSquare and AlphaShapeCrossingGrids are unchanged and still pass, so the flag defaulting to true keeps the current behaviour. --no-python-tests only because the mrbind modules are not built in this checkout.
  • Not exercised here: the smaller non-NDEBUG / emscripten cloud sizes, and the other 18 MRTest configs — that is what the next run on this branch is for. Those run on 3–4 vCPU, so their absolute ms will be higher than the table above; the filter-on/off ratio and the exact counters are the comparable part.
  • No UI surface (gtest-only harness plus one struct field), so nothing to drive in the viewer and no screenshots.

CI labels: none — this PR is based on alpha-shape-triangle-shadow, so it gets no checks of its own by design; the numbers come from the branch run after it lands there.

meshinspector-agent Bot and others added 2 commits August 14, 2026 12:11
The filter is output-preserving, so an AlphaShapeData flag turns it off and one
binary times the unfiltered search (= master) against it on the same runner and
the same cloud, which is the only way the two are comparable on shared CI vCPUs.
Reps are capped by count and by wall clock, and the cloud shrinks where NDEBUG
is off or under emscripten, so the bench fits the 10-minute unit-test cap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gtest skips DISABLED_ tests, and every platform's Unit Tests step already runs
MRTest, so dropping the prefix is what makes the numbers appear in all of the
run's MRTest jobs without touching a CI file. Reverted once they are collected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Fedr

Fedr commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Closing: the decision this harness was built to inform has been made.

The floating-point pre-filters it was meant to measure are back in alpha-shape-triangle-shadow
(#6596 for the wedge, #6595 for the query filter), and they clearly pay. Measured on a real 18820-point
cloud at r=0.5, one binary per variant, interleaved, hashes equal: removing the wedge filter costs +30%
and removing the query filter +41%, and the branch is 4.46 s against master's 10.02 s.

That also makes the table above misleading rather than merely stale — it was measured when every shadow
test was exact, so it reads as the filter losing on the sphere and random clouds, which is no longer the
case.

What remains is not worth its price: AlphaShapeData::shadowFilter is a public, Python-bound field whose
only use is benchmarking, since the filter is output-preserving and no caller has a reason to switch it
off. The branch keeps DISABLED_AlphaShapeBench for per-variant numbers.

@Fedr Fedr closed this Aug 17, 2026
@Fedr
Fedr deleted the agent/mlib-alpha-shape-bench-harness branch August 17, 2026 13:11
@meshinspector-agent

Copy link
Copy Markdown
Contributor Author

Agreed on all three — nothing to reopen.

  • "The decision has been made; the pre-filters pay." — Confirmed: Reject most shadow candidates in double before the exact test #6596 and Restore the double query filter in FastInSphereTesterSoS #6595 are both squash-merged into alpha-shape-triangle-shadow (16 Aug, 10ae314a / 937df58b), so the harness has no question left to answer.
  • "The table is misleading rather than merely stale." — Fair, and the framing was mine: measured with every shadow test exact, it reads as a verdict on the filter when it was really a verdict on the exact path. Annotated the body with a superseded banner carrying your numbers, and left the table itself in place as the record.
  • "AlphaShapeData::shadowFilter is not worth its price." — Agreed, and dropping it costs nothing: the field only ever existed on this PR's branch. MRAlphaShape.h at the branch tip has no shadowFilter, just the counters — so there is nothing to strip.

One loose end that closes with it: the "restore the DISABLED_ prefix before this reaches master" follow-up is moot. The un-disabling commit lived only here, so the branch tip still reads TEST( MRMesh, DISABLED_AlphaShapeBench ) (source/MRTest/MRAlphaShapeTests.cpp:193) — no always-on bench ships to master, and per-variant numbers stay available the way you took them: one binary per variant, --gtest_also_run_disabled_tests.

No code changed, so nothing was built or driven for this reply.

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