Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use faster maths to project WGS84 to mercator #88231

Merged
merged 6 commits into from
Jul 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/88231.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 88231
summary: Use faster maths to project WGS84 to mercator
area: Geo
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.elasticsearch.common.geo;

import org.apache.lucene.util.SloppyMath;
import org.elasticsearch.core.ESSloppyMath;
import org.elasticsearch.geometry.Rectangle;

/**
Expand All @@ -17,8 +19,8 @@ public class SphericalMercatorUtils {

public static final double MERCATOR_BOUNDS = 20037508.34;
private static final double MERCATOR_FACTOR = MERCATOR_BOUNDS / 180.0;
// latitude lower limit. Below this limit the transformation might result in -Infinity
private static final double LAT_LOWER_LIMIT = Math.nextUp(-90.0);
// tangent lower limit. Below this limit the transformation result is -Infinity
private static final double TAN_LOWER_LIMIT = Math.tan(Math.nextUp(0.0));

/**
* Transforms WGS84 longitude to a Spherical mercator longitude
Expand All @@ -31,7 +33,9 @@ public static double lonToSphericalMercator(double lon) {
* Transforms WGS84 latitude to a Spherical mercator latitude
*/
public static double latToSphericalMercator(double lat) {
double y = Math.log(Math.tan((90 + Math.max(lat, LAT_LOWER_LIMIT)) * Math.PI / 360)) / (Math.PI / 180);
final double cos = SloppyMath.cos((90 + lat) * Math.PI / 360);
final double tan = Math.max(TAN_LOWER_LIMIT, Math.sqrt(1 - cos * cos) / cos);
final double y = ESSloppyMath.log(tan) / (Math.PI / 180);
return y * MERCATOR_FACTOR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,17 @@ public void testRectangle() {
assertThat(mercatorRect.getMinY(), Matchers.equalTo(latToSphericalMercator(rect.getMinY())));
assertThat(mercatorRect.getMaxY(), Matchers.equalTo(latToSphericalMercator(rect.getMaxY())));
}

public void testLatitudeExactMath() {
// check that the maths we are using are precise to 1mm from the strict maths
for (int i = 0; i < 10000; i++) {
double lat = randomDoubleBetween(-GeoTileUtils.LATITUDE_MASK, GeoTileUtils.LATITUDE_MASK, true);
assertEquals(lat + "", strictLatToSphericalMercator(lat), latToSphericalMercator(lat), 1e-3);
}
}

private static double strictLatToSphericalMercator(double lat) {
double y = Math.log(Math.tan((90 + Math.max(lat, Math.nextUp(-90.0))) * Math.PI / 360)) / (Math.PI / 180);
return y * SphericalMercatorUtils.MERCATOR_BOUNDS / 180;
}
}
1 change: 1 addition & 0 deletions x-pack/plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task ->
task.skipTest("indices.freeze/10_basic/Test index options", "#70192 -- the freeze index API is removed from 8.0")
task.skipTest("sql/sql/Paging through results", "scrolling through search hit queries no longer produces empty last page in 8.2")
task.skipTest("service_accounts/10_basic/Test get service accounts", "new service accounts are added")
task.skipTest("spatial/70_script_doc_values/diagonal length", "precision changed in 8.4.0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we permanently skip this test? I see you changed the test result in the yaml file so do we need to skip this at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are skipping the V7 compatibility test (yamlRestTestV7CompatTransform) . It seems this mode is running the test on the v7 branch against this cluster. That currently fails because the change in precision which is fine.


task.replaceValueInMatch("_type", "_doc")
task.addAllowedWarningRegex("\\[types removal\\].*")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,4 @@ setup:
script:
source: "doc['geo_shape'].getMercatorHeight()"
- match: { hits.hits.0.fields.width.0: 389.62170283915475 }
- match: { hits.hits.0.fields.height.0: 333.37976840604097 }
- match: { hits.hits.0.fields.height.0: 333.37976841442287 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if we could assert on values with a precision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not help here.