Repository navigation
h3: keep cell areas exact across the antimeridian - #176
Merged
Merged
Conversation
Coverage Report for CI Build 39681503510Coverage remained the same at 100.0%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
cagnoli subtracted the raw longitudes of an edge's endpoints. For an edge crossing +-180 degrees the difference is close to +-360 and the true, tiny longitude step is lost to the rounding of a value that large, so cells straddling the antimeridian came out too small: 1e-8 relative at res 7, 0.1% at res 13 and 9% at res 15. Shifting both longitudes to the same side of the antimeridian before subtracting keeps the subtraction between two nearby values; sin and cos are 2pi-periodic, so nothing else changes. This is upstream uber/h3#1274, which the pinned release does not yet contain. The conformance suite could not see the defect: its seeded sample has straddling cells only at resolutions 0 and 1, where the loss is far below tolerance. The generator now carries the upstream change as a second patch and writes floats/curated.jsonl, the upstream test pairs at res 13 to 15 plus the cell containing (lat, 180) at every resolution for three latitudes, which the floats/cells checks run. The fifteen straddling res 0 and 1 cells already in floats/cells.jsonl move by at most 4e-14 relative; every other file, the digests included, is byte for byte unchanged. Three tests port the upstream ones: straddling cells agree with their non-straddling neighbours, a cell on the antimeridian keeps its area when rotated onto the prime meridian, and a small triangle across +-180 has the area of the same triangle across 0. All three fail without the shift. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
justinhwang
force-pushed
the
fix/area-antimeridian
branch
from
October 21, 2026 21:46
dfd2153 to
99510a8
Compare
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
CellAreaRads2,CellAreaKm2andCellAreaM2returned too small an area for cells that straddle the antimeridian. The per-edge term subtracts the endpoints' longitudes; for an edge crossing ±180° the raw difference is close to ±360°, and the true, tiny longitude step is lost to the rounding of a value that large. Measured by comparing a straddling cell's boundary with the same boundary rotated 180° about the polar axis:The fix shifts both longitudes to the same side of the antimeridian before subtracting.
sinandcosare 2π-periodic, so the result is mathematically unchanged, and edges that do not cross take the old path. This is upstream uber/h3#1274, still open, which the pinned v4.5.0 does not contain.The conformance suite was blind to this: its seeded sample has straddling cells only at resolutions 0 and 1, where the loss is around 1e-13. So:
conformance/internal/gen/patches/0002-cellArea-antimeridian.patchcarries the upstreamarea.chunk, applied the same way as the atan2 patch and to be deleted once a pinned release contains it.floats/curated.jsonl: the three straddling and non-straddling pairs from the upstream tests at res 13 to 15, plus the cell containing (lat, 180°) at every resolution for latitudes 0°, 45° and −60°, 54 subjects.TestFloatCellsruns it with thefloats/cellschecks.Test plan
go generate ./conformance, thendigests.sh 7) addsfloats/curated.jsonl, moves the area of the fifteen straddling res 0 and 1 cells already infloats/cells.jsonlby at most 4e-14 relative, and updates the manifest; every other file, the digests included, is byte for byte unchangedgo test -race ./...green, root and conformance at 100% coveragebuild.shwithGEN_STRICT_CFLAGS='-Wall -Wextra -Wconversion -Wpedantic -Werror'succeedsgolangci-lint run ./...clean