Conversation
…e +/- periodicity
… down only from grids where fixLonAcrossDateline && averageLonAtPole (the flags that already meant "gnomonic cubed sphere" on Grid_t, just never passed to the locator before). Wired into mnt_vectorinterp_buildLocator only — deliberately not into PolylineIntegral/RegridEdges, since their line-intersection code (collectIntersectionPoints) does its own independent flat-straight-line math that a spherical containsPoint would put out of sync with — that's a separate, deeper fix (great-circle arc intersection) I've documented at both call sites rather than half-applying. Spherical bilinear patch (sphericalBilinearMap, invertSphericalBilinearPatch, containsPointCubedSphere) — for a cubed-sphere face, containment and parametric coordinates now come from one consistent model (double-slerp patch + Gauss-Newton inverse), instead of pairing an accurate spherical containment test with vtkQuad::EvaluatePosition's unrelated flat model — which is what caused the interpolation blow-ups. Performance: each face's XYZ corners/centroid/radius are precomputed once in BuildLocator, and a cheap centroid-distance pre-filter (proven safe — 2× margin above the measured worst case) rejects the large majority of candidate cells before ever running Newton. Net: the originally-reported test went from 2s → 928s (suite) → back to 2s / 44s suite-wide.
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
2 participants
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.
Cause B is now fully fixed, with all tests green (109 passed, ~44s — back to baseline speed).
What changed (src/vmtCellLocator.h, src/vmtCellLocator.cpp):
setCubedSphere(bool) — a new, explicit flag on the locator, threaded down only from grids where fixLonAcrossDateline && averageLonAtPole (the flags that already meant "gnomonic cubed sphere" on Grid_t, just never passed to the locator before). Wired into mnt_vectorinterp_buildLocator only — deliberately not into PolylineIntegral/RegridEdges, since their line-intersection code (collectIntersectionPoints) does its own independent flat-straight-line math that a spherical containsPoint would put out of sync with — that's a separate, deeper fix (great-circle arc intersection) I've documented at both call sites rather than half-applying.
Spherical bilinear patch (sphericalBilinearMap, invertSphericalBilinearPatch, containsPointCubedSphere) — for a cubed-sphere face, containment and parametric coordinates now come from one consistent model (double-slerp patch + Gauss-Newton inverse), instead of pairing an accurate spherical containment test with vtkQuad::EvaluatePosition's unrelated flat model — which is what caused the interpolation blow-ups.
Performance: each face's XYZ corners/centroid/radius are precomputed once in BuildLocator, and a cheap centroid-distance pre-filter (proven safe — 2× margin above the measured worst case) rejects the large majority of candidate cells before ever running Newton. Net: the originally-reported test went from 2s → 928s (suite) → back to 2s / 44s suite-wide.
Verified: target test passes with 0 bad cells; full suite (109 tests) passes with no regressions; the earlier catastrophic RegridEdges/PolylineIntegral blow-ups are gone because those locators now simply keep their original, unmodified flat behavior.
Background shell failed
python -m pytest mint/tests/ -q > /private/tmp/claude-502/-Users-apletzer-work/7041a9d5-c242-44a7-a484-486318749792/scratchpad/full_test_run.log 2>&1
That's a stale notification from the pre-performance-fix run I kicked off earlier (killed, exit 137) — superseded by the rerun I did after adding the caching/pre-filter fix, which completed cleanly (full_test_run2.log: 109 passed in 44s). Nothing to act on here; the current state is the one I already reported: cause B fully fixed, full suite green, performance back to baseline.