Open
Conversation
Extract _vec3dToClosestFace, _vec3dToHex2d, _vec3dToFaceIjk from LatLng wrappers. Add _vec3dAzimuthRads (3D tangent-plane azimuth) and _hex2dToVec3 (inverse via Rodrigues' rotation). Add vec3ToCell and cellToVec3 public API. Existing LatLng API is unchanged — wrappers delegate to the new Vec3d functions. From uber#1052.
ajfriend
commented
Apr 1, 2026
| if (err) { | ||
| return err; | ||
| } | ||
| printf("%.10lf\n", length); |
Collaborator
Author
There was a problem hiding this comment.
Slightly different floating point math on Mac vs Linux. Reducing to 8 digits gets tests to pass on both platforms. (We do an exact string comparison in edgeLengthM.txt)
This was referenced Apr 1, 2026
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
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
Vec3d refactor
Based on @holoskii 's work in #1052, we cherry pick out just the
Vec3dindexing refactor.The indexing path on
mastercurrently operates on "geo" lat/lng coordinates:The new path in this PR converts from lat/lng to
Vec3dand does more operations in that representation:Additional changes
In addition to what was done in #1052, this PR makes everything go through the new
Vec3dlogic, for a complete migration:latLngToCellcellToLatLngcellToBoundarydirectedEdgeToBoundaryvertexToLatLngMoving everything let us delete all the "to geo" and "geo to" code, e.g.,
_faceIjkToGeo.Also:
vec3d.hprovided a nice win:latLngToCellwent from ~11% slower to ~3% faster thanmasterCell assignment tests
Compares this PR to what's on
master.Code in
src/apps/benchmarks/dumpCoreApi.candstress.py(will be removed before landing). Run withuv run stress.pyNote: These are on top of the existing tests,
bc*centers.txtandrand*centers.txtintests/inputfiles/.latLngToCell: random points10,000 random points (uniform on sphere, fixed seed) tested at 8 resolutions (0, 1, 2, 3, 4, 7, 10, 15) = 80,000 tests.
Result: 0 mismatches
latLngToCell: boundary pointsAll cell boundary vertices and edge midpoints at resolutions 0–3, tested at 8 resolutions = 4,654,080 tests.
Result: ~12% mismatches. But this is expected, since we arbitrarily break ties between cells, and slightly different floating point math may break ties differently.
cellToLatLngandcellToBoundaryAll cells at resolutions 0--4.
Result: all agree to within ~0.001 mm
Benchmarks
Code in
src/apps/benchmarks/benchmarkCoreApi.candbench.py. Run withuv run bench.py. (Will be removed before landing.)latLngToCellcellToLatLngcellToBoundarydirectedEdgeToBoundaryvertexToLatLngNote that one of the bottlenecks of the new
cellsToMultiPolygoncode isdirectedEdgeToBoundary, so speedups here will help that function as well!Follow-ups
Renaming
To help me understand the code, I had renamed a bunch of things, including using
Vec2andVec3consistently ("hex2d" and "vec2d" were used interchangeably, and function names didn't always match struct names). While that helped in working on the PR, it made this diff hard to read. I'll revisit in the future, since I think better naming would be nice if we continue to work on this code.Header-only allows for helper function optimizations
Compiler optimizations across translation units makes a difference. By moving
vec3d.hto be header-only andstatic inline, thelatLngToCellbenchmarks go from +11% (slower) to -3.4% (faster), compared tomaster. We can do the same elsewhere:coordijk.chas lots of small functions that get called in other.cfiles (translation units). I tested movingcoordijk.cto be header-only, and gotlatLngToCellto be -9.7% (faster) compared tomaster, andcellToLatLngandcellToBoundarywere both -33% (faster). We should do this in a follow-up PR. And there might be other files worth considering, likevec2d.h.Trig-free rotation math
The rotation math in
_vec3ToVec2translates from 3d to angle and back to 3d. That rotation can be done without any trig functions. Instead of storing angles infaceAxesAzRadsCII(and note that we store all three, but only use the first!), we could instead store theVec3i-axis for each icosahedron face. This removes 5 trig function calls per indexing operation. Bigger change, separate PR.Note, we can also remove some trig from this path in
_vec3ToHex2d:by writing
I tried this, but didn't see much movement in the benchmarks.