Repository navigation
Compute the GARS cell from one 5 minute index - #1
Open
SpaceOceanCorp wants to merge 1 commit into
Open
SpaceOceanCorp wants to merge 1 commit into
SpaceOceanCorp wants to merge 1 commit into
Conversation
GARS.from() found the cell by scaling band fractions in steps (x 2 for the quadrant, then x 3 for the keypad). Rounding in those steps put points on a 5 minute boundary in the cell below or to the left: for example 10 05' E, 10 05' N gave 381JJ38 (the cell 10 00' to 10 05' N) instead of 381JJ35. Of 1,000,000 random whole-minute boundary values (degrees + minutes / 60), 555,120 landed in the wrong cell. It also produced strings that parse() rejects: longitude 180 gave band 721 (e.g. 721HN37) and latitude 90 gave band RA (e.g. 361RA37). Compute the 5 minute cell index directly, multiplying before adding the offset as GeographicLib and GEOTRANS do, map longitude 180 to -180 (band 001), and keep latitude 90 in the top band QZ. With this change none of the 1,000,000 boundary values or 1,000,000 random points land in the wrong cell, and every result parses. testPointBounds expected 462RA49 at latitude 90; it now expects 462QZ23, the top cell of band QZ.
This branch has not been deployed
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.
GARS.from()finds the cell by scaling band fractions in steps (x 2 for the quadrant, then x 3 for the keypad). Rounding in those steps puts points that lie on a 5 minute boundary in the cell below or to the left:Of 1,000,000 random whole-minute boundary values written as degrees + minutes / 60, master puts 555,120 in the wrong cell.
The change computes the 5 minute cell index directly, multiplying before adding the offset (as GeographicLib and GEOTRANS do), maps longitude 180 to -180 (band 001, the same meridian), and keeps latitude 90 in the top band QZ. With it, none of those 1,000,000 boundary values or 1,000,000 random points land in the wrong cell (checked against an exact BigDecimal floor for the random points), and every result parses.
testPointBoundsexpected462RA49at latitude 90; RA is not a GARS band andparse()rejects it, so it now expects462QZ23, the top cell of QZ. NewtestCellBoundaries; all 12 tests pass.The same code is in gars-js (separate PR) and gars-ios.
Paul S. Mamakos
Space Ocean Corp