Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Round latitude/longitude values #6800

Closed
tobrun opened this issue Oct 24, 2016 · 13 comments
Closed

Round latitude/longitude values #6800

tobrun opened this issue Oct 24, 2016 · 13 comments
Labels
Android Mapbox Maps SDK for Android bug

Comments

@tobrun
Copy link
Member

tobrun commented Oct 24, 2016

While writing some instrumentation tests on the Camera API. I noticed that latitude/longitude coming from core needs to be rounded. Eg. setting a camera position at latitude = 1.0 results in getting core values equal to 1.0000000000000142. I'm suggesting to round this value to 10 decimals.

@tobrun tobrun added bug Android Mapbox Maps SDK for Android labels Oct 24, 2016
@tobrun
Copy link
Member Author

tobrun commented Oct 24, 2016

Using 10 decimals results in precision of:

0.011132 mm

@tobrun
Copy link
Member Author

tobrun commented Oct 24, 2016

Same issue with bearing:
screen shot 2016-10-24 at 11 46 27 1

@tobrun
Copy link
Member Author

tobrun commented Oct 24, 2016

It seems that using 10 decimals is not ideal. For example take a LatLngBounds with corners equal to {0,0} and {2,2}. This should obviously result in a center of {1,1} but instead I'm hitting:

screen shot 2016-10-24 at 12 05 17

In this case we should round with 3 decimals, which results in a precision of 111m at equator...

update this mistake in rounding is not coming from core but comes from our own calculations when trying to determine the center of the bounds for the camera update.

@tobrun
Copy link
Member Author

tobrun commented Oct 24, 2016

Similar issue with `moveTo(x,y):

screen shot 2016-10-24 at 12 25 35

@tobrun tobrun mentioned this issue Oct 24, 2016
17 tasks
@tobrun
Copy link
Member Author

tobrun commented Oct 31, 2016

Capturing from internal discussions this is expected behaviour from doubles and we/endusers should take this in account when working with the core api. Eg. when comparing two doubles we should always take in account a delta.

Closing for now and will fix up tests #6803 following these guidelines.

@tobrun tobrun closed this as completed Oct 31, 2016
@1ec5
Copy link
Contributor

1ec5 commented Jan 8, 2017

The imprecision described here occurs when you take a float and cast it to double. In fact, in the iOS and macOS SDKs, we warn the developer if they attempt to use a float in a format string because of this potential to introduce imprecision. To my knowledge, mbgl uses doubles consistently for geographic values (but floats for non-geographic values like color alpha components and symbol offsets).

@tobrun tobrun reopened this Jan 9, 2017
@tobrun
Copy link
Member Author

tobrun commented Jan 9, 2017

I have been looking into this issue is more depth by reading up on it and debugging core. I was able to verify the following statement:

The difference you are observing, will be due to differences in the formatting code that turns the double values into digits for output. In the Java case, the code is giving the most accurate decimal representation of the actual double that it can achieve. In the C++ case, it is rounding off the least significant digits ... which gives the number you expect, but not the most accurate decimal representation.

This is the output produced by java vs c++:

Zoom

C++ =  1.216975
java =  1.216975118093277

Latitude

C++ = -21.798482
java = -21.798481588640755

Longitude

C++ = 6.316062
java = 6.316062123791681

Bearing

C++ = 54.137765
java = 54.13776528835294

I'm now thinking about rounding the values coming from core to the same decimal amount as what I'm seeing with logging.

update:
@1ec5 mentioned that logging automatically shortens the output to 6 decimals.. Need to revisit this.

@1ec5
Copy link
Contributor

1ec5 commented Jan 9, 2017

This is the output produced by java vs c++:

That’s an apples-to-oranges comparison of the default string conversion formats for floating-point numbers: by default, C/C++ APIs like sprintf and cout round to six decimal places, whereas Java apparently goes out to 15:

printf("%g", M_PI);
// => 3.14159
printf("%f", M_PI);
// => 3.141593
printf("%.15f", M_PI);
// => 3.141592653589793

What you care about is not the way these numbers get converted to strings but rather how they’re represented in memory.

@1ec5
Copy link
Contributor

1ec5 commented Jan 9, 2017

I’m suspicious of lines like this that appear to reinterpret a float as a double, guaranteeing a bit of cruft in the double-precision decimal places.

@tobrun
Copy link
Member Author

tobrun commented Jan 9, 2017

This is the output produced by java vs c++:

That’s an apples-to-oranges comparison of the default string conversion formats for floating-point numbers: by default, C/C++ APIs like sprintf and cout round to six decimal places, whereas Java apparently goes out to 15:

Thank you for 👀, was able to verify that they are the same if I increased the decimals while logging. I have now a setup to do some more testes while consecutive zooming isn't resulting in rounded zoom levels.

I’m suspicious of lines like this that appear to reinterpret a float as a double, guaranteeing a bit of cruft in the double-precision decimal places.

Yes, this is could definitely be an issue, but not the issue for which I reopened this ticket as the code shown there is only used at map startup. I have commented this code out to do additional test on this issue. Will pick this up in a follow up.

@tobrun
Copy link
Member Author

tobrun commented Jan 9, 2017

First thing I'm noticing is that a map that is initialised with zoom level 0 is set to 0.236014191900084796538195064386

Second thing I noticed is that if you zoom in by 1 using the on screen zoom control you get:
1.23601419190008487980492191127 while you would expect 1.236014191900084796538195064386.

@1ec5
Copy link
Contributor

1ec5 commented Jan 9, 2017

First thing I'm noticing is that a map that is initialised with zoom level 0 is set to 0.236014191900084796538195064386

That could be due to mbgl::TransformState constraining the map so that we don't show anything beyond ±180° latitude don't show more than 180° of longitude at the same time (avoiding "world copying").

@tobrun
Copy link
Member Author

tobrun commented Mar 7, 2017

Figured this one out, it's not related to floating point, our camera change listener was not hooked into the correct MapChange events.

@tobrun tobrun closed this as completed Apr 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android bug
Projects
None yet
Development

No branches or pull requests

2 participants