diff --git a/c/openlocationcode_test.cc b/c/openlocationcode_test.cc index 935c8e4f..d7e4f807 100644 --- a/c/openlocationcode_test.cc +++ b/c/openlocationcode_test.cc @@ -176,12 +176,8 @@ TEST_P(EncodingChecks, OLC_LocationToIntegers) { OLC_LatLon loc = OLC_LatLon{test_data.lat_deg, test_data.lng_deg}; OLC_LatLonIntegers got; OLC_LocationToIntegers(&loc, &got); - // Due to floating point precision limitations, we may get values 1 less than - // expected. - EXPECT_LE(got.lat, test_data.lat_int); - EXPECT_GE(got.lat + 1, test_data.lat_int); - EXPECT_LE(got.lon, test_data.lng_int); - EXPECT_GE(got.lon + 1, test_data.lng_int); + EXPECT_EQ(test_data.lat_int, got.lat); + EXPECT_EQ(test_data.lng_int, got.lon); } INSTANTIATE_TEST_SUITE_P(OLC_Tests, EncodingChecks, diff --git a/c/src/olc.c b/c/src/olc.c index ef7202ea..d54a3de0 100644 --- a/c/src/olc.c +++ b/c/src/olc.c @@ -82,13 +82,25 @@ int OLC_IsFull(const char* code, size_t size) { return is_full(&info); } +// Apply floor after multiplying by precision, with correction for +// floating-point errors. Due to floating-point representation, multiplying +// a value like 129.7 by 8192000 may produce 1062502399.9999999 instead of the +// exact 1062502400. A simple floor() would incorrectly return 1062502399. +// This function detects and corrects such cases. +static long long int corrected_floor(double value, long long int precision) { + long long int n = (long long int)floorl(value * (double)precision); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if ((double)(n + 1) / (double)precision <= value) { + return n + 1; + } + return n; +} + void OLC_LocationToIntegers(const OLC_LatLon* degrees, OLC_LatLonIntegers* integers) { - // Multiply degrees by precision. Use lround to explicitly round rather than - // truncate, which causes issues when using values like 0.1 that do not have - // precise floating point representations. - long long int lat = floorl(degrees->lat * kGridLatPrecisionInverse); - long long int lon = floorl(degrees->lon * kGridLonPrecisionInverse); + // Multiply degrees by precision with correction for floating-point errors. + long long int lat = corrected_floor(degrees->lat, kGridLatPrecisionInverse); + long long int lon = corrected_floor(degrees->lon, kGridLonPrecisionInverse); // Convert latitude to positive range (0..2*degrees*precision) and clip. lat += OLC_kLatMaxDegrees * kGridLatPrecisionInverse; diff --git a/cpp/openlocationcode.cc b/cpp/openlocationcode.cc index d128e423..5fb09205 100644 --- a/cpp/openlocationcode.cc +++ b/cpp/openlocationcode.cc @@ -48,8 +48,22 @@ const int kPositionLUT['X' - 'C' + 1] = {8, -1, -1, 9, 10, 11, -1, 12, -1, -1, 13, -1, -1, 14, 15, 16, -1, -1, -1, 17, 18, 19}; +// Apply floor after multiplying by precision, with correction for +// floating-point errors. Due to floating-point representation, multiplying +// a value like 129.7 by 8192000 may produce 1062502399.9999999 instead of the +// exact 1062502400. A simple floor() would incorrectly return 1062502399. +// This function detects and corrects such cases. +int64_t correctedFloor(double value, int64_t precision) { + int64_t n = static_cast(floor(value * static_cast(precision))); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if (static_cast(n + 1) / static_cast(precision) <= value) { + return n + 1; + } + return n; +} + int64_t latitudeToInteger(double latitude) { - int64_t lat = floor(latitude * kGridLatPrecisionInverse); + int64_t lat = correctedFloor(latitude, kGridLatPrecisionInverse); lat += kLatitudeMaxDegrees * kGridLatPrecisionInverse; if (lat < 0) { lat = 0; @@ -60,7 +74,7 @@ int64_t latitudeToInteger(double latitude) { } int64_t longitudeToInteger(double longitude) { - int64_t lng = floor(longitude * kGridLngPrecisionInverse); + int64_t lng = correctedFloor(longitude, kGridLngPrecisionInverse); lng += kLongitudeMaxDegrees * kGridLngPrecisionInverse; if (lng <= 0) { lng = lng % (2 * kLongitudeMaxDegrees * kGridLngPrecisionInverse) + diff --git a/cpp/openlocationcode_test.cc b/cpp/openlocationcode_test.cc index 21d4edd2..44a8d8c1 100644 --- a/cpp/openlocationcode_test.cc +++ b/cpp/openlocationcode_test.cc @@ -210,13 +210,9 @@ TEST_P(EncodingChecks, OLC_EncodeIntegers) { TEST_P(EncodingChecks, OLC_LocationToIntegers) { EncodingTestData test_data = GetParam(); int64_t got_lat = internal::latitudeToInteger(test_data.lat_deg); - // Due to floating point precision limitations, we may get values 1 less than - // expected. - EXPECT_LE(got_lat, test_data.lat_int); - EXPECT_GE(got_lat + 1, test_data.lat_int); + EXPECT_EQ(test_data.lat_int, got_lat); int64_t got_lng = internal::longitudeToInteger(test_data.lng_deg); - EXPECT_LE(got_lng, test_data.lng_int); - EXPECT_GE(got_lng + 1, test_data.lng_int); + EXPECT_EQ(test_data.lng_int, got_lng); } INSTANTIATE_TEST_CASE_P(OLC_Tests, EncodingChecks, diff --git a/dart/lib/src/open_location_code.dart b/dart/lib/src/open_location_code.dart index 715a1eb6..2bbe6341 100644 --- a/dart/lib/src/open_location_code.dart +++ b/dart/lib/src/open_location_code.dart @@ -258,13 +258,27 @@ String encode(num latitude, num longitude, {int codeLength = pairCodeLength}) { return encodeIntegers(integers[0], integers[1], codeLength); } +// Apply floor after multiplying by precision, with correction for +// floating-point errors. Due to floating-point representation, multiplying +// a value like 129.7 by 8192000 may produce 1062502399.9999999 instead of +// the exact 1062502400. A simple floor() would incorrectly return 1062502399. +// This function detects and corrects such cases. +int _correctedFloor(num value, int precision) { + var n = (value * precision).floor().toInt(); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if ((n + 1) / precision <= value) { + return n + 1; + } + return n; +} + // Convert latitude and longitude in degrees to the integer values needed for // reliable conversions. List locationToIntegers(num latitude, num longitude) { // Convert latitude into a positive integer clipped into the range 0-(just // under 180*2.5e7). Latitude 90 needs to be adjusted to be just less, so the // returned code can also be decoded. - var latVal = (latitude * finalLatPrecision).floor().toInt(); + var latVal = _correctedFloor(latitude, finalLatPrecision); latVal += latitudeMax * finalLatPrecision; if (latVal < 0) { latVal = 0; @@ -273,7 +287,7 @@ List locationToIntegers(num latitude, num longitude) { } // Convert longitude into a positive integer and normalise it into the range // 0-360*8.192e6. - var lngVal = (longitude * finalLngPrecision).floor().toInt(); + var lngVal = _correctedFloor(longitude, finalLngPrecision); lngVal += longitudeMax * finalLngPrecision; if (lngVal < 0) { // Dart's % operator differs from other languages in that it returns the diff --git a/dart/test/encode_test.dart b/dart/test/encode_test.dart index 1d7b8f85..dc7e6b21 100644 --- a/dart/test/encode_test.dart +++ b/dart/test/encode_test.dart @@ -46,36 +46,21 @@ void checkLocationToIntegers(String csvLine) { int latInteger = int.parse(elements[2]); int lngInteger = int.parse(elements[3]); var got = olc.locationToIntegers(latDegrees, lngDegrees); - // Due to floating point precision limitations, we may get values 1 less than expected. - expect(got[0], lessThanOrEqualTo(latInteger)); - expect(got[0] + 1, greaterThanOrEqualTo(latInteger)); - expect(got[1], lessThanOrEqualTo(lngInteger)); - expect(got[1] + 1, greaterThanOrEqualTo(lngInteger)); + expect(got[0], equals(latInteger)); + expect(got[1], equals(lngInteger)); } void main() { - // Encoding from degrees permits a small percentage of errors. - // This is due to floating point precision limitations. test('Check encode from degrees', () { - // The proportion of tests that we will accept generating a different code. - // This should not be significantly different from any other implementation. - num allowedErrRate = 0.05; - int errors = 0; - int tests = 0; csvLinesFromFile('encoding.csv').forEach((csvLine) { - tests++; var elements = csvLine.split(','); num lat = double.parse(elements[0]); num lng = double.parse(elements[1]); int len = int.parse(elements[4]); var want = elements[5]; var got = olc.encode(lat, lng, codeLength: len); - if (got != want) { - print("ENCODING DIFFERENCE: Got '$got', expected '$want'"); - errors++; - } + expect(got, equals(want)); }); - expect(errors / tests, lessThanOrEqualTo(allowedErrRate)); }); test('Check encode from integers', () { diff --git a/go/olc.go b/go/olc.go index cda9ca96..8c1404ef 100644 --- a/go/olc.go +++ b/go/olc.go @@ -229,10 +229,24 @@ func normalizeLng(value float64) float64 { return normalize(value, lngMax) } +// correctedFloor applies floor after multiplying by precision, with correction +// for floating-point errors. Due to floating-point representation, multiplying +// a value like 129.7 by 8192000 may produce 1062502399.9999999 instead of the +// exact 1062502400. A simple floor() would incorrectly return 1062502399. +// This function detects and corrects such cases. +func correctedFloor(value float64, precision int64) int64 { + n := int64(math.Floor(value * float64(precision))) + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if float64(n+1)/float64(precision) <= value { + return n + 1 + } + return n +} + // latitudeAsInteger converts a latitude in degrees into the integer representation. // It will be clipped into the degree range -90<=x<90 (actually 0-180*2.5e7-1). func latitudeAsInteger(latDegrees float64) int64 { - latVal := int64(math.Floor(latDegrees * finalLatPrecision)) + latVal := correctedFloor(latDegrees, finalLatPrecision) latVal += latMax * finalLatPrecision if latVal < 0 { latVal = 0 @@ -245,7 +259,7 @@ func latitudeAsInteger(latDegrees float64) int64 { // longitudeAsInteger converts a longitude in degrees into the integer representation. // It will be normalised into the degree range -180<=x<180 (actually 0-360*8.192e6). func longitudeAsInteger(lngDegrees float64) int64 { - lngVal := int64(math.Floor(lngDegrees * finalLngPrecision)) + lngVal := correctedFloor(lngDegrees, finalLngPrecision) lngVal += lngMax * finalLngPrecision if lngVal <= 0 { lngVal = lngVal%(2*lngMax*finalLngPrecision) + 2*lngMax*finalLngPrecision diff --git a/go/olc_test.go b/go/olc_test.go index 407fb8a0..e024d1c7 100644 --- a/go/olc_test.go +++ b/go/olc_test.go @@ -17,7 +17,6 @@ package olc import ( "bufio" "encoding/csv" - "fmt" "math" "math/rand" "os" @@ -132,18 +131,12 @@ func TestCheck(t *testing.T) { } func TestEncodeDegrees(t *testing.T) { - const allowedErrRate float64 = 0.05 - var badCodes int for i, elt := range encoding { got := Encode(elt.latDeg, elt.lngDeg, elt.length) if got != elt.code { - fmt.Printf("ENCODING DIFFERENCE %d. got %q for Encode(%v,%v,%d), wanted %q\n", i, got, elt.latDeg, elt.lngDeg, elt.length, elt.code) - badCodes++ + t.Errorf("%d. got %q for Encode(%v,%v,%d), wanted %q", i, got, elt.latDeg, elt.lngDeg, elt.length, elt.code) } } - if errRate := float64(badCodes) / float64(len(encoding)); errRate > allowedErrRate { - t.Errorf("Too many errors in encoding degrees (got %f, allowed %f)", errRate, allowedErrRate) - } } func TestEncodeIntegers(t *testing.T) { @@ -158,11 +151,11 @@ func TestEncodeIntegers(t *testing.T) { func TestConvertDegrees(t *testing.T) { for i, elt := range encoding { got := latitudeAsInteger(elt.latDeg) - if got > elt.latInt || got < elt.latInt-1 { + if got != elt.latInt { t.Errorf("%d. got %d for latitudeAsInteger(%v), wanted %d", i, got, elt.latDeg, elt.latInt) } got = longitudeAsInteger(elt.lngDeg) - if got > elt.lngInt || got < elt.lngInt-1 { + if got != elt.lngInt { t.Errorf("%d. got %d for longitudeAsInteger(%v), wanted %d", i, got, elt.lngDeg, elt.lngInt) } } diff --git a/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java b/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java index 37b36636..8c658b19 100644 --- a/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java +++ b/java/src/main/java/com/google/openlocationcode/OpenLocationCode.java @@ -656,6 +656,26 @@ public static boolean isShortCode(String code) { // Private static methods. + /** + * Apply floor after multiplying by precision, with correction for floating-point errors. + * + *

Due to floating-point representation, multiplying a value like 129.7 by 8192000 may produce + * 1062502399.9999999 instead of the exact 1062502400. A simple floor() would incorrectly return + * 1062502399. This function detects and corrects such cases. + * + * @param value The value to multiply and floor. + * @param precision The precision multiplier. + * @return The corrected floor result. + */ + private static long correctedFloor(double value, long precision) { + long n = (long) Math.floor(value * (double) precision); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if ((double) (n + 1) / (double) precision <= value) { + return n + 1; + } + return n; + } + /** * Convert latitude and longitude in degrees into the integer values needed for reliable encoding. * (To avoid floating point precision errors.) @@ -665,8 +685,8 @@ public static boolean isShortCode(String code) { * @return A list of [latitude, longitude] in clipped, normalised integer values. */ static long[] degreesToIntegers(double latitude, double longitude) { - long lat = (long) Math.floor(latitude * LAT_INTEGER_MULTIPLIER); - long lng = (long) Math.floor(longitude * LNG_INTEGER_MULTIPLIER); + long lat = correctedFloor(latitude, LAT_INTEGER_MULTIPLIER); + long lng = correctedFloor(longitude, LNG_INTEGER_MULTIPLIER); // Clip and normalise values. lat += LATITUDE_MAX * LAT_INTEGER_MULTIPLIER; diff --git a/java/src/test/java/com/google/openlocationcode/EncodingTest.java b/java/src/test/java/com/google/openlocationcode/EncodingTest.java index 3bc062eb..978de803 100644 --- a/java/src/test/java/com/google/openlocationcode/EncodingTest.java +++ b/java/src/test/java/com/google/openlocationcode/EncodingTest.java @@ -61,30 +61,21 @@ public void setUp() throws Exception { @Test public void testEncodeFromDegrees() { - double allowedErrorRate = 0.05; - int failedEncodings = 0; for (TestData testData : testDataList) { String got = OpenLocationCode.encode( testData.latitudeDegrees, testData.longitudeDegrees, testData.length); - if (!testData.code.equals(got)) { - failedEncodings++; - System.out.printf( - "ENCODING DIFFERENCE: encode(%f,%f,%d) got %s, want %s\n", - testData.latitudeDegrees, - testData.longitudeDegrees, - testData.length, - got, - testData.code); - } + Assert.assertEquals( + String.format( + "encode(%f,%f,%d) want %s, got %s", + testData.latitudeDegrees, + testData.longitudeDegrees, + testData.length, + testData.code, + got), + testData.code, + got); } - double gotRate = (double) failedEncodings / (double) testDataList.size(); - Assert.assertTrue( - String.format( - "Too many encoding errors (actual rate %f, allowed rate %f), see ENCODING DIFFERENCE" - + " lines", - gotRate, allowedErrorRate), - gotRate <= allowedErrorRate); } @Test @@ -92,22 +83,24 @@ public void testDegreesToIntegers() { for (TestData testData : testDataList) { long[] got = OpenLocationCode.degreesToIntegers(testData.latitudeDegrees, testData.longitudeDegrees); - Assert.assertTrue( + Assert.assertEquals( String.format( - "degreesToIntegers(%f, %f) returned latitude %d, expected %d", + "degreesToIntegers(%f, %f) latitude: want %d, got %d", testData.latitudeDegrees, testData.longitudeDegrees, - got[0], - testData.latitudeInteger), - got[0] == testData.latitudeInteger || got[0] == testData.latitudeInteger - 1); - Assert.assertTrue( + testData.latitudeInteger, + got[0]), + testData.latitudeInteger, + got[0]); + Assert.assertEquals( String.format( - "degreesToIntegers(%f, %f) returned longitude %d, expected %d", + "degreesToIntegers(%f, %f) longitude: want %d, got %d", testData.latitudeDegrees, testData.longitudeDegrees, - got[1], - testData.longitudeInteger), - got[1] == testData.longitudeInteger || got[1] == testData.longitudeInteger - 1); + testData.longitudeInteger, + got[1]), + testData.longitudeInteger, + got[1]); } } diff --git a/js/closure/openlocationcode.js b/js/closure/openlocationcode.js index eea7bcc6..debd4a6f 100644 --- a/js/closure/openlocationcode.js +++ b/js/closure/openlocationcode.js @@ -195,7 +195,7 @@ var MIN_TRIMMABLE_CODE_LEN = 6; * Returns the characters used to produce the codes. * @return {string} the OLC alphabet. */ -exports.getAlphabet = function() { +exports.getAlphabet = function () { return CODE_ALPHABET; }; @@ -263,8 +263,8 @@ function isValid(code) { // Strip the separator and any padding characters. code = code - .replace(new RegExp('\\' + SEPARATOR + '+'), '') - .replace(new RegExp(PADDING_CHARACTER + '+'), ''); + .replace(new RegExp('\\' + SEPARATOR + '+'), '') + .replace(new RegExp(PADDING_CHARACTER + '+'), ''); // Check the code contains only valid characters. for (var i = 0, len = code.length; i < len; i++) { var character = code.charAt(i).toUpperCase(); @@ -384,14 +384,27 @@ exports.encode = encode; * @return {Array} A tuple of the latitude integer and longitude integer. */ function _locationToIntegers(latitude, longitude) { - var latVal = Math.floor(latitude * FINAL_LAT_PRECISION); + // Apply corrected floor to handle floating-point errors. + // Due to floating-point representation, multiplying a value like 129.7 by + // 8192000 may produce 1062502399.9999999 instead of the exact 1062502400. + // A simple floor() would incorrectly return 1062502399. + var correctedFloor = function (value, precision) { + var n = Math.floor(value * precision); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if ((n + 1) / precision <= value) { + return n + 1; + } + return n; + }; + + var latVal = correctedFloor(latitude, FINAL_LAT_PRECISION); latVal += LATITUDE_MAX * FINAL_LAT_PRECISION; if (latVal < 0) { latVal = 0; } else if (latVal >= 2 * LATITUDE_MAX * FINAL_LAT_PRECISION) { latVal = 2 * LATITUDE_MAX * FINAL_LAT_PRECISION - 1; } - var lngVal = Math.floor(longitude * FINAL_LNG_PRECISION); + var lngVal = correctedFloor(longitude, FINAL_LNG_PRECISION); lngVal += LONGITUDE_MAX * FINAL_LNG_PRECISION; if (lngVal < 0) { lngVal = @@ -489,9 +502,9 @@ exports._encodeIntegers = _encodeIntegers; function decode(code) { if (!isFull(code)) { throw new Error( - 'IllegalArgumentException: ' + - 'Passed Plus Code is not a valid full code: ' + - code + 'IllegalArgumentException: ' + + 'Passed Plus Code is not a valid full code: ' + + code ); } // Strip the '+' and '0' characters from the code and convert to upper case. @@ -543,11 +556,11 @@ function decode(code) { var lat = normalLat / PAIR_PRECISION + gridLat / FINAL_LAT_PRECISION; var lng = normalLng / PAIR_PRECISION + gridLng / FINAL_LNG_PRECISION; return new CodeArea( - lat, - lng, - lat + latPrecision, - lng + lngPrecision, - Math.min(code.length, MAX_CODE_LEN) + lat, + lng, + lat + latPrecision, + lng + lngPrecision, + Math.min(code.length, MAX_CODE_LEN) ); } exports.decode = decode; @@ -580,7 +593,7 @@ function recoverNearest(shortCode, referenceLatitude, referenceLongitude) { return shortCode.toUpperCase(); } else { throw new Error( - 'ValueError: Passed short code is not valid: ' + shortCode + 'ValueError: Passed short code is not valid: ' + shortCode ); } } @@ -599,8 +612,8 @@ function recoverNearest(shortCode, referenceLatitude, referenceLongitude) { // Use the reference location to pad the supplied short code and decode it. var /** @type {!CodeArea} */ codeArea = decode( - encode(referenceLatitude, referenceLongitude).substr(0, paddingLength) + - shortCode + encode(referenceLatitude, referenceLongitude).substr(0, paddingLength) + + shortCode ); // How many degrees latitude is the code from the reference? If it is more // than half the resolution, we need to move it north or south but keep it @@ -629,9 +642,9 @@ function recoverNearest(shortCode, referenceLatitude, referenceLongitude) { } return encode( - codeArea.latitudeCenter, - codeArea.longitudeCenter, - codeArea.codeLength + codeArea.latitudeCenter, + codeArea.longitudeCenter, + codeArea.codeLength ); } exports.recoverNearest = recoverNearest; @@ -671,15 +684,15 @@ function shorten(code, latitude, longitude) { var codeArea = decode(code); if (codeArea.codeLength < MIN_TRIMMABLE_CODE_LEN) { throw new Error( - 'ValueError: Code length must be at least ' + MIN_TRIMMABLE_CODE_LEN); + 'ValueError: Code length must be at least ' + MIN_TRIMMABLE_CODE_LEN); } // Ensure that latitude and longitude are valid. latitude = clipLatitude(latitude); longitude = normalizeLongitude(longitude); // How close are the latitude and longitude to the code center. var range = Math.max( - Math.abs(codeArea.latitudeCenter - latitude), - Math.abs(codeArea.longitudeCenter - longitude) + Math.abs(codeArea.latitudeCenter - latitude), + Math.abs(codeArea.longitudeCenter - longitude) ); for (var i = PAIR_RESOLUTIONS.length - 2; i >= 1; i--) { // Check if we're close enough to shorten. The range must be less than 1/2 @@ -735,11 +748,11 @@ function normalizeLongitude(longitude) { @constructor */ function CodeArea( - latitudeLo, - longitudeLo, - latitudeHi, - longitudeHi, - codeLength + latitudeLo, + longitudeLo, + latitudeHi, + longitudeHi, + codeLength ) { /** @type {number} */ this.latitudeLo = latitudeLo; /** @type {number} */ this.longitudeLo = longitudeLo; @@ -747,12 +760,12 @@ function CodeArea( /** @type {number} */ this.longitudeHi = longitudeHi; /** @type {number} */ this.codeLength = codeLength; /** @type {number} */ this.latitudeCenter = Math.min( - latitudeLo + (latitudeHi - latitudeLo) / 2, - LATITUDE_MAX - ); + latitudeLo + (latitudeHi - latitudeLo) / 2, + LATITUDE_MAX +); /** @type {number} */ this.longitudeCenter = Math.min( - longitudeLo + (longitudeHi - longitudeLo) / 2, - LONGITUDE_MAX - ); + longitudeLo + (longitudeHi - longitudeLo) / 2, + LONGITUDE_MAX +); } exports.CodeArea = CodeArea; diff --git a/js/closure/openlocationcode_test.js b/js/closure/openlocationcode_test.js index ca6e1a1a..c312c295 100644 --- a/js/closure/openlocationcode_test.js +++ b/js/closure/openlocationcode_test.js @@ -28,19 +28,19 @@ const testSuite = goog.require('goog.testing.testSuite'); goog.require('goog.testing.asserts'); const /** @const {string} */ DECODING_TEST_FILE = - '/filez/_main/test_data/decoding.csv'; + '/filez/_main/test_data/decoding.csv'; const /** @const {string} */ ENCODING_TEST_FILE = - '/filez/_main/test_data/encoding.csv'; + '/filez/_main/test_data/encoding.csv'; const /** @const {string} */ SHORT_CODE_TEST_FILE = - '/filez/_main/test_data/shortCodeTests.csv'; + '/filez/_main/test_data/shortCodeTests.csv'; const /** @const {string} */ VALIDITY_TEST_FILE = - '/filez/_main/test_data/validityTests.csv'; + '/filez/_main/test_data/validityTests.csv'; // Initialise the async test framework. const /** @const {!AsyncTestCase} */ asyncTestCase = AsyncTestCase.createAndInstall(); testSuite({ - testDecode: function() { + testDecode: function () { const xhrIo_ = new XhrIo(); xhrIo_.listenOnce(EventType.COMPLETE, () => { const lines = xhrIo_.getResponseText().match(/^[^#].+/gm); @@ -67,13 +67,9 @@ testSuite({ asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.send(DECODING_TEST_FILE, 'GET'); }, - testEncodeDegrees: function() { + testEncodeDegrees: function () { const xhrIo_ = new XhrIo(); xhrIo_.listenOnce(EventType.COMPLETE, () => { - // Allow a 5% error rate encoding from degree coordinates (because of floating - // point precision). - const allowedErrorRate = 0.05; - var errors = 0; const lines = xhrIo_.getResponseText().match(/^[^#].+/gm); for (var i = 0; i < lines.length; i++) { const fields = lines[i].split(','); @@ -83,25 +79,18 @@ testSuite({ const code = fields[5]; const got = OpenLocationCode.encode(latDegrees, lngDegrees, length); - // Did we get the same code? - if (code != got) { - console.warn( - 'ENCODING DIFFERENCE: Expected code ' + code +', got ' + got - ); - errors++; - } + assertEquals( + 'encode(' + latDegrees + ', ' + lngDegrees + ', ' + length + ')', + code, + got + ); asyncTestCase.continueTesting(); } - console.info('testEncodeDegrees error rate is ' + (errors / lines.length)); - assertTrue( - 'testEncodeDegrees: too many errors ' + errors / lines.length, - (errors / lines.length) < allowedErrorRate - ); }); asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.send(ENCODING_TEST_FILE, 'GET'); }, - testLocationToIntegers: function() { + testLocationToIntegers: function () { const xhrIo_ = new XhrIo(); xhrIo_.listenOnce(EventType.COMPLETE, () => { const lines = xhrIo_.getResponseText().match(/^[^#].+/gm); @@ -113,18 +102,18 @@ testSuite({ const lngIntegers = parseInt(fields[3], 10); const got = OpenLocationCode._locationToIntegers( - latDegrees, - lngDegrees + latDegrees, + lngDegrees ); - // Due to floating point precision limitations, we may get values 1 less - // than expected. - assertTrue( - 'testLocationToIntegers: expected latitude ' + latIntegers + ', got ' + got[0], - got[0] == latIntegers || got[0] == latIntegers - 1 + assertEquals( + 'locationToIntegers latitude for ' + latDegrees, + latIntegers, + got[0] ); - assertTrue( - 'testLocationToIntegers: expected longitude ' + lngIntegers + ', got ' + got[1], - got[1] == lngIntegers || got[1] == lngIntegers - 1 + assertEquals( + 'locationToIntegers longitude for ' + lngDegrees, + lngIntegers, + got[1] ); asyncTestCase.continueTesting(); } @@ -132,7 +121,7 @@ testSuite({ asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.send(ENCODING_TEST_FILE, 'GET'); }, - testEncodeIntegers: function() { + testEncodeIntegers: function () { const xhrIo_ = new XhrIo(); xhrIo_.listenOnce(EventType.COMPLETE, () => { const lines = xhrIo_.getResponseText().match(/^[^#].+/gm); @@ -144,14 +133,14 @@ testSuite({ const code = fields[5]; const got = OpenLocationCode._encodeIntegers( - latIntegers, - lngIntegers, - length + latIntegers, + lngIntegers, + length ); // Did we get the same code? assertEquals( - 'testEncodeIntegers: expected code ' + code + ', got ' + got, - code, got + 'testEncodeIntegers: expected code ' + code + ', got ' + got, + code, got ); asyncTestCase.continueTesting(); } @@ -159,7 +148,7 @@ testSuite({ asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.send(ENCODING_TEST_FILE, 'GET'); }, - testShortCodes: function() { + testShortCodes: function () { const xhrIo_ = new XhrIo(); asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.listenOnce(EventType.COMPLETE, () => { @@ -186,12 +175,12 @@ testSuite({ }); xhrIo_.send(SHORT_CODE_TEST_FILE, 'GET'); }, - testRecoveryNearPoles: function() { + testRecoveryNearPoles: function () { assertEquals('2CXXXXXX+XX', OpenLocationCode.recoverNearest('XXXXXX+XX', -81.0, 0.0)); assertEquals('CFX22222+22', OpenLocationCode.recoverNearest('2222+22', 89.6, 0.0)); assertEquals('CFX22222+22', OpenLocationCode.recoverNearest('2222+22', 89.6, 0.0)); }, - testValidity: function() { + testValidity: function () { const xhrIo_ = new XhrIo(); xhrIo_.listenOnce(EventType.COMPLETE, () => { const lines = xhrIo_.getResponseText().match(/^[^#].+/gm); @@ -212,7 +201,7 @@ testSuite({ asyncTestCase.waitForAsync('Waiting for xhr to respond'); xhrIo_.send(VALIDITY_TEST_FILE, 'GET'); }, - testBenchmarks: function() { + testBenchmarks: function () { var input = []; for (var i = 0; i < 100000; i++) { var lat = Math.random() * 180 - 90; @@ -232,9 +221,9 @@ testSuite({ } var durationMillis = Date.now() - startMillis; console.info( - 'Encoding: ' + input.length + ', total ' + durationMillis * 1000 + - ' usecs, average duration ' + - ((durationMillis * 1000) / input.length) + ' usecs'); + 'Encoding: ' + input.length + ', total ' + durationMillis * 1000 + + ' usecs, average duration ' + + ((durationMillis * 1000) / input.length) + ' usecs'); startMillis = Date.now(); for (var i = 0; i < input.length; i++) { @@ -242,8 +231,8 @@ testSuite({ } durationMillis = Date.now() - startMillis; console.info( - 'Decoding: ' + input.length + ', total ' + durationMillis * 1000 + - ' usecs, average duration ' + - ((durationMillis * 1000) / input.length) + ' usecs'); + 'Decoding: ' + input.length + ', total ' + durationMillis * 1000 + + ' usecs, average duration ' + + ((durationMillis * 1000) / input.length) + ' usecs'); }, }); diff --git a/js/src/openlocationcode.js b/js/src/openlocationcode.js index 97eab85d..02bb605f 100644 --- a/js/src/openlocationcode.js +++ b/js/src/openlocationcode.js @@ -56,11 +56,11 @@ * var code = OpenLocationCode.recoverNearest('9G8F+6X', 47.4, 8.6); * var code = OpenLocationCode.recoverNearest('8F+6X', 47.4, 8.6); */ -(function(root, factory) { +(function (root, factory) { /* global define, module */ if (typeof define === 'function' && define.amd) { // AMD. Register as an anonymous module. - define(function() { + define(function () { return (root.returnExportsGlobal = factory()); }); } else if (typeof module === 'object' && module.exports) { @@ -72,7 +72,7 @@ // Browser globals root.OpenLocationCode = factory(); } -}(this, function() { +}(this, function () { var OpenLocationCode = {}; /** @@ -121,7 +121,7 @@ // First place value of the pairs (if the last pair value is 1). var PAIR_FIRST_PLACE_VALUE_ = Math.pow( - ENCODING_BASE_, (PAIR_CODE_LENGTH_ / 2 - 1)); + ENCODING_BASE_, (PAIR_CODE_LENGTH_ / 2 - 1)); // Inverse of the precision of the pair section of the code. var PAIR_PRECISION_ = Math.pow(ENCODING_BASE_, 3); @@ -142,21 +142,21 @@ // First place value of the latitude grid (if the last place is 1). var GRID_LAT_FIRST_PLACE_VALUE_ = Math.pow( - GRID_ROWS_, (GRID_CODE_LENGTH_ - 1)); + GRID_ROWS_, (GRID_CODE_LENGTH_ - 1)); // First place value of the longitude grid (if the last place is 1). var GRID_LNG_FIRST_PLACE_VALUE_ = Math.pow( - GRID_COLUMNS_, (GRID_CODE_LENGTH_ - 1)); + GRID_COLUMNS_, (GRID_CODE_LENGTH_ - 1)); // Multiply latitude by this much to make it a multiple of the finest // precision. var FINAL_LAT_PRECISION_ = PAIR_PRECISION_ * - Math.pow(GRID_ROWS_, (MAX_DIGIT_COUNT_ - PAIR_CODE_LENGTH_)); + Math.pow(GRID_ROWS_, (MAX_DIGIT_COUNT_ - PAIR_CODE_LENGTH_)); // Multiply longitude by this much to make it a multiple of the finest // precision. var FINAL_LNG_PRECISION_ = PAIR_PRECISION_ * - Math.pow(GRID_COLUMNS_, (MAX_DIGIT_COUNT_ - PAIR_CODE_LENGTH_)); + Math.pow(GRID_COLUMNS_, (MAX_DIGIT_COUNT_ - PAIR_CODE_LENGTH_)); // Minimum length of a code that can be shortened. var MIN_TRIMMABLE_CODE_LEN_ = 6; @@ -164,7 +164,7 @@ /** @return {string} Returns the OLC alphabet. */ - OpenLocationCode.getAlphabet = function() { + OpenLocationCode.getAlphabet = function () { return CODE_ALPHABET_; }; @@ -178,7 +178,7 @@ * @param {string} code The string to check. * @return {boolean} True if the string is a valid code. */ - var isValid = OpenLocationCode.isValid = function(code) { + var isValid = OpenLocationCode.isValid = function (code) { if (!code || typeof code !== 'string') { return false; } @@ -195,7 +195,7 @@ } // Is it in an illegal position? if (code.indexOf(SEPARATOR_) > SEPARATOR_POSITION_ || - code.indexOf(SEPARATOR_) % 2 == 1) { + code.indexOf(SEPARATOR_) % 2 == 1) { return false; } // We can have an even number of padding characters before the separator, @@ -212,7 +212,7 @@ // There can only be one group and it must have even length. var padMatch = code.match(new RegExp('(' + PADDING_CHARACTER_ + '+)', 'g')); if (padMatch.length > 1 || padMatch[0].length % 2 == 1 || - padMatch[0].length > SEPARATOR_POSITION_ - 2) { + padMatch[0].length > SEPARATOR_POSITION_ - 2) { return false; } // If the code is long enough to end with a separator, make sure it does. @@ -228,7 +228,7 @@ // Strip the separator and any padding characters. code = code.replace(new RegExp('\\' + SEPARATOR_ + '+'), '') - .replace(new RegExp(PADDING_CHARACTER_ + '+'), ''); + .replace(new RegExp(PADDING_CHARACTER_ + '+'), ''); // Check the code contains only valid characters. for (var i = 0, len = code.length; i < len; i++) { var character = code.charAt(i).toUpperCase(); @@ -246,14 +246,14 @@ * @return {boolean} True if the string can be produced by removing four or * more characters from the start of a valid code. */ - var isShort = OpenLocationCode.isShort = function(code) { + var isShort = OpenLocationCode.isShort = function (code) { // Check it's valid. if (!isValid(code)) { return false; } // If there are less characters than expected before the SEPARATOR. if (code.indexOf(SEPARATOR_) >= 0 && - code.indexOf(SEPARATOR_) < SEPARATOR_POSITION_) { + code.indexOf(SEPARATOR_) < SEPARATOR_POSITION_) { return true; } return false; @@ -266,7 +266,7 @@ * @return {boolean} True if the code represents a valid latitude and * longitude combination. */ - var isFull = OpenLocationCode.isFull = function(code) { + var isFull = OpenLocationCode.isFull = function (code) { if (!isValid(code)) { return false; } @@ -277,7 +277,7 @@ // Work out what the first latitude character indicates for latitude. var firstLatValue = CODE_ALPHABET_.indexOf( - code.charAt(0).toUpperCase()) * ENCODING_BASE_; + code.charAt(0).toUpperCase()) * ENCODING_BASE_; if (firstLatValue >= LATITUDE_MAX_ * 2) { // The code would decode to a latitude of >= 90 degrees. return false; @@ -285,7 +285,7 @@ if (code.length > 1) { // Work out what the first longitude character indicates for longitude. var firstLngValue = CODE_ALPHABET_.indexOf( - code.charAt(1).toUpperCase()) * ENCODING_BASE_; + code.charAt(1).toUpperCase()) * ENCODING_BASE_; if (firstLngValue >= LONGITUDE_MAX_ * 2) { // The code would decode to a longitude of >= 180 degrees. return false; @@ -308,8 +308,8 @@ * @return {string} The code. * @throws {Exception} if any of the input values are not numbers. */ - var encode = OpenLocationCode.encode = function(latitude, - longitude, codeLength) { + var encode = OpenLocationCode.encode = function (latitude, + longitude, codeLength) { latitude = Number(latitude); longitude = Number(longitude); @@ -333,15 +333,28 @@ * @param {number} longitude * @return {Array} A tuple of the latitude integer and longitude integer. */ - var locationToIntegers = OpenLocationCode.locationToIntegers = function(latitude, longitude) { - var latVal = Math.floor(latitude * FINAL_LAT_PRECISION_); + var locationToIntegers = OpenLocationCode.locationToIntegers = function (latitude, longitude) { + // Apply corrected floor to handle floating-point errors. + // Due to floating-point representation, multiplying a value like 129.7 by + // 8192000 may produce 1062502399.9999999 instead of the exact 1062502400. + // A simple floor() would incorrectly return 1062502399. + var correctedFloor = function (value, precision) { + var n = Math.floor(value * precision); + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if ((n + 1) / precision <= value) { + return n + 1; + } + return n; + }; + + var latVal = correctedFloor(latitude, FINAL_LAT_PRECISION_); latVal += LATITUDE_MAX_ * FINAL_LAT_PRECISION_; if (latVal < 0) { latVal = 0; } else if (latVal >= 2 * LATITUDE_MAX_ * FINAL_LAT_PRECISION_) { latVal = 2 * LATITUDE_MAX_ * FINAL_LAT_PRECISION_ - 1; } - var lngVal = Math.floor(longitude * FINAL_LNG_PRECISION_); + var lngVal = correctedFloor(longitude, FINAL_LNG_PRECISION_); lngVal += LONGITUDE_MAX_ * FINAL_LNG_PRECISION_; if (lngVal < 0) { lngVal = @@ -366,7 +379,7 @@ * specified. * @throws {Exception} if any of the input values are not numbers. */ - var encodeIntegers = OpenLocationCode.encodeIntegers = function(latInt, lngInt, codeLength) { + var encodeIntegers = OpenLocationCode.encodeIntegers = function (latInt, lngInt, codeLength) { if (typeof codeLength == 'undefined') { codeLength = OpenLocationCode.CODE_PRECISION_NORMAL; } else { @@ -376,7 +389,7 @@ throw new Error('ValueError: Parameters are not numbers'); } if (codeLength < MIN_DIGIT_COUNT_ || - (codeLength < PAIR_CODE_LENGTH_ && codeLength % 2 == 1)) { + (codeLength < PAIR_CODE_LENGTH_ && codeLength % 2 == 1)) { throw new Error('IllegalArgumentException: Invalid Open Location Code length'); } // Javascript strings are immutable and it doesn't have a native @@ -420,7 +433,7 @@ } // Pad and return the code. return code.slice(0, codeLength).join('') + - Array(SEPARATOR_POSITION_ - codeLength + 1).join(PADDING_CHARACTER_) + SEPARATOR_; + Array(SEPARATOR_POSITION_ - codeLength + 1).join(PADDING_CHARACTER_) + SEPARATOR_; }; /** @@ -434,13 +447,13 @@ * area of the code. * @throws {Exception} If the code is not valid. */ - var decode = OpenLocationCode.decode = function(code) { + var decode = OpenLocationCode.decode = function (code) { // This calculates the values for the pair and grid section separately, using // integer arithmetic. Only at the final step are they converted to floating // point and combined. if (!isFull(code)) { throw new Error('IllegalArgumentException: ' + - 'Passed Plus Code is not a valid full code: ' + code); + 'Passed Plus Code is not a valid full code: ' + code); } // Strip the '+' and '0' characters from the code and convert to upper case. code = code.replace('+', '').replace(/0/g, '').toLocaleUpperCase('en-US'); @@ -492,11 +505,11 @@ var lat = normalLat / PAIR_PRECISION_ + gridLat / FINAL_LAT_PRECISION_; var lng = normalLng / PAIR_PRECISION_ + gridLng / FINAL_LNG_PRECISION_; return new CodeArea( - lat, - lng, - lat + latPrecision, - lng + lngPrecision, - Math.min(code.length, MAX_DIGIT_COUNT_)); + lat, + lng, + lat + latPrecision, + lng + lngPrecision, + Math.min(code.length, MAX_DIGIT_COUNT_)); }; /** @@ -514,14 +527,14 @@ * @throws {Exception} if the short code is not valid, or the reference * position values are not numbers. */ - OpenLocationCode.recoverNearest = function( - shortCode, referenceLatitude, referenceLongitude) { + OpenLocationCode.recoverNearest = function ( + shortCode, referenceLatitude, referenceLongitude) { if (!isShort(shortCode)) { if (isFull(shortCode)) { return shortCode.toUpperCase(); } else { throw new Error( - 'ValueError: Passed short code is not valid: ' + shortCode); + 'ValueError: Passed short code is not valid: ' + shortCode); } } referenceLatitude = Number(referenceLatitude); @@ -544,18 +557,18 @@ // Use the reference location to pad the supplied short code and decode it. var codeArea = decode( - encode(referenceLatitude, referenceLongitude).substr(0, paddingLength) - + shortCode); + encode(referenceLatitude, referenceLongitude).substr(0, paddingLength) + + shortCode); // How many degrees latitude is the code from the reference? If it is more // than half the resolution, we need to move it north or south but keep it // within -90 to 90 degrees. if (referenceLatitude + halfResolution < codeArea.latitudeCenter && - codeArea.latitudeCenter - resolution >= -LATITUDE_MAX_) { + codeArea.latitudeCenter - resolution >= -LATITUDE_MAX_) { // If the proposed code is more than half a cell north of the reference location, // it's too far, and the best match will be one cell south. codeArea.latitudeCenter -= resolution; } else if (referenceLatitude - halfResolution > codeArea.latitudeCenter && - codeArea.latitudeCenter + resolution <= LATITUDE_MAX_) { + codeArea.latitudeCenter + resolution <= LATITUDE_MAX_) { // If the proposed code is more than half a cell south of the reference location, // it's too far, and the best match will be one cell north. codeArea.latitudeCenter += resolution; @@ -569,7 +582,7 @@ } return encode( - codeArea.latitudeCenter, codeArea.longitudeCenter, codeArea.codeLength); + codeArea.latitudeCenter, codeArea.longitudeCenter, codeArea.codeLength); }; /** @@ -588,8 +601,8 @@ * @throws {Exception} if the passed code is not a valid full code or the * reference location values are not numbers. */ - OpenLocationCode.shorten = function( - code, latitude, longitude) { + OpenLocationCode.shorten = function ( + code, latitude, longitude) { if (!isFull(code)) { throw new Error('ValueError: Passed code is not valid and full: ' + code); } @@ -600,8 +613,8 @@ var codeArea = decode(code); if (codeArea.codeLength < MIN_TRIMMABLE_CODE_LEN_) { throw new Error( - 'ValueError: Code length must be at least ' + - MIN_TRIMMABLE_CODE_LEN_); + 'ValueError: Code length must be at least ' + + MIN_TRIMMABLE_CODE_LEN_); } // Ensure that latitude and longitude are valid. latitude = Number(latitude); @@ -613,8 +626,8 @@ longitude = normalizeLongitude(longitude); // How close are the latitude and longitude to the code center. var range = Math.max( - Math.abs(codeArea.latitudeCenter - latitude), - Math.abs(codeArea.longitudeCenter - longitude)); + Math.abs(codeArea.latitudeCenter - latitude), + Math.abs(codeArea.longitudeCenter - longitude)); for (var i = PAIR_RESOLUTIONS_.length - 2; i >= 1; i--) { // Check if we're close enough to shorten. The range must be less than 1/2 // the resolution to shorten at all, and we want to allow some safety, so @@ -633,7 +646,7 @@ * @param {number} latitude * @return {number} The latitude value clipped to be in the range. */ - var clipLatitude = function(latitude) { + var clipLatitude = function (latitude) { return Math.min(90, Math.max(-90, latitude)); }; @@ -643,7 +656,7 @@ * @param {number} longitude * @return {number} Normalized into the range -180 to 180. */ - var normalizeLongitude = function(longitude) { + var normalizeLongitude = function (longitude) { while (longitude < -180) { longitude = longitude + 360; } @@ -667,14 +680,14 @@ * * @constructor */ - var CodeArea = OpenLocationCode.CodeArea = function( - latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength) { + var CodeArea = OpenLocationCode.CodeArea = function ( + latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength) { return new OpenLocationCode.CodeArea.fn.Init( - latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength); + latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength); }; CodeArea.fn = CodeArea.prototype = { - Init: function( - latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength) { + Init: function ( + latitudeLo, longitudeLo, latitudeHi, longitudeHi, codeLength) { /** * The latitude of the SW corner. * @type {number} @@ -705,13 +718,13 @@ * @type {number} */ this.latitudeCenter = Math.min( - latitudeLo + (latitudeHi - latitudeLo) / 2, LATITUDE_MAX_); + latitudeLo + (latitudeHi - latitudeLo) / 2, LATITUDE_MAX_); /** * The longitude of the center in degrees. * @type {number} */ this.longitudeCenter = Math.min( - longitudeLo + (longitudeHi - longitudeLo) / 2, LONGITUDE_MAX_); + longitudeLo + (longitudeHi - longitudeLo) / 2, LONGITUDE_MAX_); }, }; CodeArea.fn.Init.prototype = CodeArea.fn; diff --git a/js/test/jasmine-tests.js b/js/test/jasmine-tests.js index 1c679497..eb458546 100644 --- a/js/test/jasmine-tests.js +++ b/js/test/jasmine-tests.js @@ -1,29 +1,29 @@ -describe("Open Location Code", function() { +describe("Open Location Code", function () { var precision = 1e-10; jasmine.getFixtures().fixturesPath = "base/"; const encodingTests = JSON.parse(jasmine.getFixtures().read("encoding.json")); - it("has encoding tests", function() { + it("has encoding tests", function () { expect(encodingTests.length).toBeGreaterThan(1); }); const decodingTests = JSON.parse(jasmine.getFixtures().read("decoding.json")); - it("has decoding tests", function() { + it("has decoding tests", function () { expect(decodingTests.length).toBeGreaterThan(1); }); const validityTests = JSON.parse(jasmine.getFixtures().read("validityTests.json")); - it("has validity tests", function() { + it("has validity tests", function () { expect(validityTests.length).toBeGreaterThan(1); }); const shortenTests = JSON.parse(jasmine.getFixtures().read("shortCodeTests.json")); - it("has shorten tests", function() { + it("has shorten tests", function () { expect(shortenTests.length).toBeGreaterThan(1); }); - describe("locationToIntegers Tests", function() { + describe("locationToIntegers Tests", function () { for (let i = 0; i < encodingTests.length; i++) { const test = encodingTests[i]; const lat = test[0]; @@ -31,103 +31,93 @@ describe("Open Location Code", function() { const wantLat = test[2]; const wantLng = test[3]; const got = OpenLocationCode.locationToIntegers(lat, lng); - // Due to floating point precision limitations, we may get values 1 less - // than expected. - it("converting latitude " + lat + " to integer, want " + wantLat + ", got " + got[0], function() { - expect(got[0] == wantLat || got[0] == wantLat - 1).toBe(true); + it("converting latitude " + lat + " to integer, want " + wantLat + ", got " + got[0], function () { + expect(got[0]).toBe(wantLat); }); - it("converting longitude " + lng + " to integer, want " + wantLng + ", got " + got[1], function() { - expect(got[1] == wantLng || got[1] == wantLng - 1).toBe(true); + it("converting longitude " + lng + " to integer, want " + wantLng + ", got " + got[1], function () { + expect(got[1]).toBe(wantLng); }); } }); - describe("encode (degrees) Tests", function() { - // Allow a 5% error rate encoding from degree coordinates (because of floating - // point precision). - const allowedErrorRate = 0.05; - let errors = 0; + describe("encode (degrees) Tests", function () { for (let i = 0; i < encodingTests.length; i++) { const test = encodingTests[i]; const lat = test[0]; const lng = test[1]; const codeLength = test[4]; const want = test[5]; - const got = OpenLocationCode.encode(lat, lng, codeLength); - if (got !== want) { - console.log('ENCODING DIFFERENCE: Expected code ' + want +', got ' + got); - errors ++; - } + it("Encoding degrees " + lat + "," + lng + " with length " + codeLength, function () { + const got = OpenLocationCode.encode(lat, lng, codeLength); + expect(got).toBe(want); + }); } - it("Encoding degrees error rate too high", function() { - expect(errors / encodingTests.length).toBeLessThan(allowedErrorRate); - }); }); - describe("encodeIntegers Tests", function() { + describe("encodeIntegers Tests", function () { for (let i = 0; i < encodingTests.length; i++) { const test = encodingTests[i]; const lat = test[2]; const lng = test[3]; const codeLength = test[4]; const want = test[5]; - it("Encoding integers " + lat + "," + lng + " with length " + codeLength, function() { + it("Encoding integers " + lat + "," + lng + " with length " + codeLength, function () { const got = OpenLocationCode.encodeIntegers(lat, lng, codeLength); expect(got).toBe(want); }); } }); - describe("Decoding Tests", function() { + describe("Decoding Tests", function () { for (let i = 0; i < decodingTests.length; i++) { const test = decodingTests[i]; const area = OpenLocationCode.decode(test[0]); - it("Decoding code " + test[0] + ", checking codelength", function() { + it("Decoding code " + test[0] + ", checking codelength", function () { expect(area.codeLength).toBe(test[1]); }) - it("Decoding code " + test[0] + ", checking latitudeLo", function() { + it("Decoding code " + test[0] + ", checking latitudeLo", function () { expect(area.latitudeLo).toBeCloseTo(test[2], precision, test[0]); }) - it("Decoding code " + test[0] + ", checking longitudeLo", function() { + it("Decoding code " + test[0] + ", checking longitudeLo", function () { expect(area.longitudeLo).toBeCloseTo(test[3], precision, test[0]); }) - it("Decoding code " + test[0] + ", checking latitudeHi", function() { + it("Decoding code " + test[0] + ", checking latitudeHi", function () { expect(area.latitudeHi).toBeCloseTo(test[4], precision, test[0]); }) - it("Decoding code " + test[0] + ", checking longitudeHi", function() { + it("Decoding code " + test[0] + ", checking longitudeHi", function () { expect(area.longitudeHi).toBeCloseTo(test[5], precision, test[0]); }) } }); - describe("Validity Tests", function() { + describe("Validity Tests", function () { for (let i = 0; i < validityTests.length; i++) { const test = validityTests[i]; - it("isValid(" + test[0] + ")", function() { + it("isValid(" + test[0] + ")", function () { expect(OpenLocationCode.isValid(test[0])).toBe(test[1] === "true", test[0]); }); - it("isShort(" + test[0] + ")", function() { + it("isShort(" + test[0] + ")", function () { expect(OpenLocationCode.isShort(test[0])).toBe(test[2] === "true", test[0]); }); - it("isFull(" + test[0] + ")", function() { + it("isFull(" + test[0] + ")", function () { expect(OpenLocationCode.isFull(test[0])).toBe(test[3] === "true", test[0]); }); } }); - describe("Short Code Tests", function() { + describe("Short Code Tests", function () { for (let i = 0; i < shortenTests.length; i++) { const test = shortenTests[i]; if (test[4] == "B" || test[4] == "S") { // Shorten the full length code. - it("shorten(" + test[0] + ", " + test[1] + ", " + test[2], function() { + it("shorten(" + test[0] + ", " + test[1] + ", " + test[2], function () { const got = OpenLocationCode.shorten(test[0], test[1], test[2]); expect(got).toBe(test[3], test[0]); }); } if (test[4] == "B" || test[4] == "R") { // Now try expanding the shortened code. - it("recoverNearest(" + test[3] + ", " + test[1] + ", " + test[2], function() { + it("recoverNearest(" + test[3] + ", " + test[1] + ", " + test[2], function () { const got = OpenLocationCode.recoverNearest(test[3], test[1], test[2]); expect(got).toBe(test[0]); }); @@ -135,7 +125,7 @@ describe("Open Location Code", function() { } }); - it("Encoding benchmark", function() { + it("Encoding benchmark", function () { var input = []; for (var i = 0; i < 1000000; i++) { var lat = Math.random() * 180 - 90; @@ -155,11 +145,11 @@ describe("Open Location Code", function() { } var duration_millis = Date.now() - start; console.info( - "Encoding: " + input.length + ", average duration " + - (1000 * duration_millis / input.length) + " usecs"); + "Encoding: " + input.length + ", average duration " + + (1000 * duration_millis / input.length) + " usecs"); }); - it("Decoding benchmark", function() { + it("Decoding benchmark", function () { var input = []; for (var i = 0; i < 1000000; i++) { var lat = Math.random() * 180 - 90; @@ -179,7 +169,7 @@ describe("Open Location Code", function() { } var duration_millis = Date.now() - start; console.info( - "Decoding: " + input.length + ", average duration " + - (1000 * duration_millis / input.length) + " usecs"); + "Decoding: " + input.length + ", average duration " + + (1000 * duration_millis / input.length) + " usecs"); }); }); diff --git a/python/openlocationcode/openlocationcode.py b/python/openlocationcode/openlocationcode.py index b29a9d70..c6276dbd 100644 --- a/python/openlocationcode/openlocationcode.py +++ b/python/openlocationcode/openlocationcode.py @@ -227,6 +227,23 @@ def isFull(code): return True +def _correctedFloor(value, precision): + """ + Apply floor after multiplying by precision, with correction for + floating-point errors. + + Due to floating-point representation, multiplying a value like 129.7 by + 8192000 may produce 1062502399.9999999 instead of the exact 1062502400. + A simple floor() would incorrectly return 1062502399. This function + detects and corrects such cases. + """ + n = int(math.floor(value * precision)) + # Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if (n + 1) / precision <= value: + return n + 1 + return n + + def locationToIntegers(latitude, longitude): """ Convert location in degrees into the integer representations. @@ -240,14 +257,14 @@ def locationToIntegers(latitude, longitude): Return: A tuple of the [latitude, longitude] values as integers. """ - latVal = int(math.floor(latitude * FINAL_LAT_PRECISION_)) + latVal = _correctedFloor(latitude, FINAL_LAT_PRECISION_) latVal += LATITUDE_MAX_ * FINAL_LAT_PRECISION_ if latVal < 0: latVal = 0 elif latVal >= 2 * LATITUDE_MAX_ * FINAL_LAT_PRECISION_: latVal = 2 * LATITUDE_MAX_ * FINAL_LAT_PRECISION_ - 1 - lngVal = int(math.floor(longitude * FINAL_LNG_PRECISION_)) + lngVal = _correctedFloor(longitude, FINAL_LNG_PRECISION_) lngVal += LONGITUDE_MAX_ * FINAL_LNG_PRECISION_ if lngVal < 0: # Python's % operator differs from other languages in that it returns diff --git a/python/openlocationcode_test.py b/python/openlocationcode_test.py index 9798c3ec..eb354a4a 100644 --- a/python/openlocationcode_test.py +++ b/python/openlocationcode_test.py @@ -115,29 +115,22 @@ def setUp(self): def test_converting_degrees(self): for td in self.testdata: got = olc.locationToIntegers(td['lat'], td['lng']) - # Due to floating point precision limitations, we may get values 1 less than expected. - self.assertTrue( - td['latInt'] - 1 <= got[0] <= td['latInt'], + self.assertEqual( + td['latInt'], got[0], f'Latitude conversion {td["lat"]}: want {td["latInt"]} got {got[0]}' ) - self.assertTrue( - td['lngInt'] - 1 <= got[1] <= td['lngInt'], + self.assertEqual( + td['lngInt'], got[1], f'Longitude conversion {td["lng"]}: want {td["lngInt"]} got {got[1]}' ) def test_encoding_degrees(self): - # Allow a small proportion of errors due to floating point. - allowedErrorRate = 0.05 - errors = 0 for td in self.testdata: got = olc.encode(td['lat'], td['lng'], td['length']) - if got != td['code']: - print( - f'olc.encode({td["lat"]}, {td["lng"]}, {td["length"]}) want {td["code"]}, got {got}' - ) - errors += 1 - self.assertLessEqual(errors / len(self.testdata), allowedErrorRate, - "olc.encode error rate too high") + self.assertEqual( + td['code'], got, + f'olc.encode({td["lat"]}, {td["lng"]}, {td["length"]}) want {td["code"]}, got {got}' + ) def test_encoding_integers(self): for td in self.testdata: diff --git a/ruby/lib/plus_codes/open_location_code.rb b/ruby/lib/plus_codes/open_location_code.rb index b693da00..f51af415 100644 --- a/ruby/lib/plus_codes/open_location_code.rb +++ b/ruby/lib/plus_codes/open_location_code.rb @@ -35,6 +35,21 @@ def full?(code) valid?(code) && !short?(code) end + # Apply floor after multiplying by precision, with correction for + # floating-point errors. Due to floating-point representation, multiplying + # a value like 129.7 by 8192000 may produce 1062502399.9999999 instead of + # the exact 1062502400. A simple floor() would incorrectly return + # 1062502399. This function detects and corrects such cases. + def corrected_floor(value, precision) + n = (value * precision).floor + # Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if (n + 1).to_f / precision <= value + n + 1 + else + n + end + end + # Convert a latitude and longitude in degrees to integer values. # # This function is exposed for testing and should not be called directly. @@ -44,14 +59,14 @@ def full?(code) # @return [Array] with the latitude and longitude integer # values. def location_to_integers(latitude, longitude) - lat_val = (latitude * PAIR_CODE_PRECISION * LAT_GRID_PRECISION).floor + lat_val = corrected_floor(latitude, PAIR_CODE_PRECISION * LAT_GRID_PRECISION) lat_val += 90 * PAIR_CODE_PRECISION * LAT_GRID_PRECISION if lat_val.negative? lat_val = 0 elsif lat_val >= 2 * 90 * PAIR_CODE_PRECISION * LAT_GRID_PRECISION lat_val = 2 * 90 * PAIR_CODE_PRECISION * LAT_GRID_PRECISION - 1 end - lng_val = (longitude * PAIR_CODE_PRECISION * LNG_GRID_PRECISION).floor + lng_val = corrected_floor(longitude, PAIR_CODE_PRECISION * LNG_GRID_PRECISION) lng_val += 180 * PAIR_CODE_PRECISION * LNG_GRID_PRECISION if lng_val.negative? # Ruby's % operator differs from other languages in that it returns diff --git a/ruby/test/plus_codes_test.rb b/ruby/test/plus_codes_test.rb index db6b939a..ed5fcaf4 100644 --- a/ruby/test/plus_codes_test.rb +++ b/ruby/test/plus_codes_test.rb @@ -77,10 +77,8 @@ def test_location_to_integers lng_integer = cols[3].to_i got_lat, got_lng = @olc.location_to_integers(lat_degrees, lng_degrees) - # Due to floating point precision limitations, we may get values 1 less - # than expected. - assert_include([lat_integer - 1, lat_integer], got_lat) - assert_include([lng_integer - 1, lng_integer], got_lng) + assert_equal(lat_integer, got_lat) + assert_equal(lng_integer, got_lng) end end diff --git a/rust/src/interface.rs b/rust/src/interface.rs index 1343fe5b..0b8df798 100644 --- a/rust/src/interface.rs +++ b/rust/src/interface.rs @@ -96,13 +96,30 @@ pub fn is_full(code: &str) -> bool { is_valid(code) && !is_short(code) } +/// Apply floor after multiplying by precision, with correction for +/// floating-point errors. +/// +/// Due to floating-point representation, multiplying a value like 129.7 by +/// 8192000 may produce 1062502399.9999999 instead of the exact 1062502400. +/// A simple floor() would incorrectly return 1062502399. This function +/// detects and corrects such cases. +fn corrected_floor(value: f64, precision: i64) -> i64 { + let n = (value * precision as f64).floor() as i64; + // Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + if (n + 1) as f64 / precision as f64 <= value { + n + 1 + } else { + n + } +} + /// Convert a latitude and longitude in degrees to integer values. /// /// This function is only exposed for testing and should not be called directly. pub fn point_to_integers(pt: Point) -> (i64, i64) { let (lng, lat) = pt.x_y(); - let mut lat_val = (lat * LAT_INTEGER_MULTIPLIER as f64).floor() as i64; + let mut lat_val = corrected_floor(lat, LAT_INTEGER_MULTIPLIER); lat_val += LATITUDE_MAX as i64 * LAT_INTEGER_MULTIPLIER; if lat_val < 0 { lat_val = 0 @@ -110,7 +127,7 @@ pub fn point_to_integers(pt: Point) -> (i64, i64) { lat_val = 2 * LATITUDE_MAX as i64 * LAT_INTEGER_MULTIPLIER - 1; } - let mut lng_val = (lng * LNG_INTEGER_MULTIPLIER as f64).floor() as i64; + let mut lng_val = corrected_floor(lng, LNG_INTEGER_MULTIPLIER); lng_val += LONGITUDE_MAX as i64 * LNG_INTEGER_MULTIPLIER; if lng_val < 0 { lng_val = lng_val % (2 * LONGITUDE_MAX as i64 * LNG_INTEGER_MULTIPLIER) diff --git a/rust/tests/all_test.rs b/rust/tests/all_test.rs index 6e35a251..a94b0455 100644 --- a/rust/tests/all_test.rs +++ b/rust/tests/all_test.rs @@ -62,9 +62,6 @@ fn decode_test() { #[test] fn encode_test() { let mut tested = 0; - let mut errors = 0; - // Allow a small proportion of errors due to floating point. - let allowed_error_rate = 0.05; for line in CSVReader::new("encoding.csv") { if line.chars().count() == 0 { continue; @@ -76,21 +73,14 @@ fn encode_test() { let code = cols[5]; let got = encode(Point::new(lng, lat), len); - if got != code { - errors += 1; - println!( - "encode(Point::new({}, {}), {}) want {}, got {}", - lng, lat, len, code, got - ); - } + assert_eq!( + got, code, + "encode(Point::new({}, {}), {}) want {}, got {}", + lng, lat, len, code, got + ); tested += 1; } - assert!( - errors as f32 / tested as f32 <= allowed_error_rate, - "too many encoding errors ({})", - errors - ); assert!(tested > 0); } @@ -108,15 +98,15 @@ fn point_to_integers_test() { let lng_int = cols[3].parse::().unwrap(); let (got_lat, got_lng) = point_to_integers(Point::new(lng_deg, lat_deg)); - assert!( - got_lat >= lat_int - 1 && got_lat <= lat_int, + assert_eq!( + got_lat, lat_int, "converting lat={}, want={}, got={}", lat_deg, lat_int, got_lat ); - assert!( - got_lng >= lng_int - 1 && got_lng <= lng_int, + assert_eq!( + got_lng, lng_int, "converting lng={}, want={}, got={}", lng_deg, lng_int, diff --git a/visualbasic/OLCTests.bas b/visualbasic/OLCTests.bas index db7da450..a859be5a 100644 --- a/visualbasic/OLCTests.bas +++ b/visualbasic/OLCTests.bas @@ -311,7 +311,6 @@ Private Function loadEncodingTestCSV() AS Variant End Function ' Check the degrees to integer conversions. -' Due to floating point precision limitations, we may get values 1 less than expected. Sub TEST_IntegerConversion() Dim encodingTests As Variant Dim i As Integer @@ -327,14 +326,14 @@ Sub TEST_IntegerConversion() degrees = tc(0) want_integer = tc(2) got_integer = latitudeToInteger(degrees) - If got_integer < want_integer - 1 Or got_integer > want_integer Then + If got_integer <> want_integer Then MsgBox ("Encoding test " + CStr(i) + ": latitudeToInteger(" + CStr(degrees) + "): got " + CStr(got_integer) + ", want " + CStr(want_integer)) Exit Sub End If degrees = tc(1) want_integer = tc(3) got_integer = longitudeToInteger(degrees) - If got_integer < want_integer - 1 Or got_integer > want_integer Then + If got_integer <> want_integer Then MsgBox ("Encoding test " + CStr(i) + ": longitudeToInteger(" + CStr(degrees) + "): got " + CStr(got_integer) + ", want " + CStr(want_integer)) Exit Sub End If diff --git a/visualbasic/OpenLocationCode.bas b/visualbasic/OpenLocationCode.bas index b9b7dc48..042b9dbb 100644 --- a/visualbasic/OpenLocationCode.bas +++ b/visualbasic/OpenLocationCode.bas @@ -380,6 +380,22 @@ Private Function normalizeLongitude(ByVal longitude As Double) As Double normalizeLongitude = lng End Function +' Apply floor after multiplying by precision, with correction for floating-point +' errors. Due to floating-point representation, multiplying a value like 129.7 +' by 8192000 may produce 1062502399.9999999 instead of the exact 1062502400. +' A simple Int() would incorrectly return 1062502399. This function detects and +' corrects such cases. +Private Function correctedFloor(ByVal value As Double, ByVal precision As Double) AS Double + Dim n As Double + n = Int(value * precision) + ' Check if (n + 1) / precision <= value. If so, n + 1 is the correct floor. + If (n + 1) / precision <= value Then + correctedFloor = n + 1 + Else + correctedFloor = n + End If +End Function + ' Convert a latitude in degrees to the integer representation. ' (We return a Double, because VB as used in LibreOffice only uses 32-bit Longs.) Private Function latitudeToInteger(ByVal latitude As Double) AS Double @@ -388,7 +404,7 @@ Private Function latitudeToInteger(ByVal latitude As Double) AS Double ' Convert latitude into a positive integer clipped into the range 0-(just ' under 180*2.5e7). Latitude 90 needs to be adjusted to be just less, so the ' returned code can also be decoded. - lat = Int(latitude * FINAL_LAT_PRECISION_) + lat = correctedFloor(latitude, FINAL_LAT_PRECISION_) lat = lat + LATITUDE_MAX_ * FINAL_LAT_PRECISION_ If lat < 0 Then lat = 0 @@ -403,7 +419,7 @@ End Function Private Function longitudeToInteger(ByVal longitude As Double) AS Double Dim lng As Double ' Convert longitude into a positive integer and normalise it into the range 0-360*8.192e6. - lng = Int(longitude * FINAL_LNG_PRECISION_) + lng = correctedFloor(longitude, FINAL_LNG_PRECISION_) lng = lng + LONGITUDE_MAX_ * FINAL_LNG_PRECISION_ If lng < 0 Then lng = doubleMod(lng, (2 * LONGITUDE_MAX_ * FINAL_LNG_PRECISION_)) diff --git a/visualbasic/update_tests.sh b/visualbasic/update_tests.sh index 15c91e28..74bf0cf7 100755 --- a/visualbasic/update_tests.sh +++ b/visualbasic/update_tests.sh @@ -38,7 +38,6 @@ function addEncodingTests() { cat <>"$VBA_TEST" ' Check the degrees to integer conversions. -' Due to floating point precision limitations, we may get values 1 less than expected. Sub TEST_IntegerConversion() Dim encodingTests As Variant Dim i As Integer @@ -49,19 +48,19 @@ Sub TEST_IntegerConversion() encodingTests = loadEncodingTestCSV() - For i = 0 To ${TEST_CASE_MAX_INDEX} + For i = 0 To \${TEST_CASE_MAX_INDEX} tc = encodingTests(i) degrees = tc(0) want_integer = tc(2) got_integer = latitudeToInteger(degrees) - If got_integer < want_integer - 1 Or got_integer > want_integer Then + If got_integer <> want_integer Then MsgBox ("Encoding test " + CStr(i) + ": latitudeToInteger(" + CStr(degrees) + "): got " + CStr(got_integer) + ", want " + CStr(want_integer)) Exit Sub End If degrees = tc(1) want_integer = tc(3) got_integer = longitudeToInteger(degrees) - If got_integer < want_integer - 1 Or got_integer > want_integer Then + If got_integer <> want_integer Then MsgBox ("Encoding test " + CStr(i) + ": longitudeToInteger(" + CStr(degrees) + "): got " + CStr(got_integer) + ", want " + CStr(want_integer)) Exit Sub End If