Skip to content

Reject most shadow candidates in double before the exact test - #6593

Closed
meshinspector-agent[bot] wants to merge 1 commit into
agent/mlib-fastint-to-doublefrom
agent/mlib-shadow-double-prefilter
Closed

Reject most shadow candidates in double before the exact test#6593
meshinspector-agent[bot] wants to merge 1 commit into
agent/mlib-fastint-to-doublefrom
agent/mlib-shadow-double-prefilter

Conversation

@meshinspector-agent

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

Copy link
Copy Markdown
Contributor

The shadow filter evaluated every candidate in exact arithmetic: the port to FastInt lost the bignum-to-double conversion the pre-filter needed. #6592 brings it back, so the pre-filter returns.

  • up/uq are computed once per pair in double via toDouble; bp = dot( d, up ) and bq = dot( d, uq ), and a negative sign of either rejects the candidate before any bignum work.
  • Error bound, which is what makes this safe: differences of points are below 2^31, so pp/pq are below 2^64 and the components of up/uq below 2^96. toDouble is correctly rounded, so each rounding costs 2^-53 of the running magnitude and the total error stays below 2^78 — a double below -2^78 is thus negative exactly as well. The tolerance is 2^84, 64x that margin; candidates in between fall through to the exact path unchanged.
  • Base is agent/mlib-fastint-to-double (Add toDouble for the FastInt family #6592), not alpha-shape-triangle-shadow: toDouble is a hard compile dependency. Retarget once Add toDouble for the FastInt family #6592 lands on the branch.

The exact path itself is untouched — this only decides which candidates reach it.

Verification

Built in the ubuntu22 image mirroring build-test-ubuntu-x64.yml: clone_submodules_linux.sh --skip-prebuilt-thirdparty, prebuilt thirdparty linked as lib/include/share, Ninja Release, g++-12, -DMR_CXX_STANDARD=20. MRTest target and its dependency graph built clean, no new warnings.

MRTest --no-python-tests --gtest_also_run_disabled_tests --gtest_filter=*AlphaShape*:*FastInt*:*PrecisePredicates*:*InSphere*25/25 pass.

A/B on the same treeDISABLED_AlphaShapeBench run with the pre-filter, then the file stashed and rebuilt incrementally so only this diff differs (same clouds, same binary otherwise):

cloud triangle hash shadowTests exactShadowTests before → after rejected
sphere 40k 17403291826481596167 unchanged 487416 487416 → 54994 88.7%
grid 40k 635805191690857954 unchanged 2058881 2058881 → 186448 90.9%
random 40k 11472133144093973002 unchanged 54564333 54564333 → 11466269 79.0%

consideredTris, touchableTris, inBallTests and shadowedNeis are identical on all three clouds in both runs, and so are the triangle hashes — the pre-filter is output-preserving, as its bound claims. inBallTests (1461420 / 3055058 / 23736199) also matches the filter-on column of #6590's table.

No timings here on purpose — this box is not a GitHub-hosted runner, so its milliseconds are not comparable to the reported set. The counters above are exact and platform-independent; the payoff in wall clock comes from the branch run.

Not exercised: the other 17 MRTest configs (MSVC, arm64, wasm, macOS) — this PR gets no checks of its own, the numbers come from the branch run after it lands. No UI surface (library-internal predicate, no MCP-exposed tool and no viewer widget), so nothing to drive and no screenshots; the three structurally different clouds x filter-on/off are the state coverage instead.

CI labels: none.

The shadow filter evaluated every candidate in exact arithmetic, because the
port to FastInt lost the double conversion the pre-filter needed. toDouble
brings it back, and the sign of one of bp, bq now rejects 79-91% of the
candidates before any bignum work, with the exact path kept for the few
falling within the error bound.
@meshinspector-agent

Copy link
Copy Markdown
Contributor Author

Superseded by #6596: same step, rebased onto alpha-shape-triangle-shadow and computing pp/qq/pq as double dot products, so it no longer depends on toDouble. Force-push to agent/* is blocked, hence a new branch rather than a rebase in place.

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.

0 participants