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

Return real precision of coordinates #138

Closed
SlowMo24 opened this issue Feb 20, 2021 · 4 comments · Fixed by #139
Closed

Return real precision of coordinates #138

SlowMo24 opened this issue Feb 20, 2021 · 4 comments · Fixed by #139
Labels
bug Something isn't working enhancement New feature or request

Comments

@SlowMo24
Copy link
Contributor

OSM uses a precision of 7 decimal values (https://wiki.openstreetmap.org/wiki/DE:Genauigkeit_von_Koordinaten). The ohsome API returns many coordinates with a precision of up to 14 decimals (enough to separate neighbouring atoms).

This is harmful in many ways

@SlowMo24 SlowMo24 added bug Something isn't working enhancement New feature or request labels Feb 20, 2021
@FabiKo117
Copy link
Contributor

If you are referring to the geometries of the OSM features that are returned through either the /elements, /elementsFullHistory, or /contributions endpoint, they are not modified within the ohsome API in any way. They are taken from the OSHDB and added to the GeoJSON response.

I was using this example request and went through the coordinates. In general, most of the coordinates are correctly limited to the precision of 7 decimal values. However, there are some like 49.405108899999995 which looks to me like there is some issue within the geometry library (potentially JTS?) not being able to give that coordinate on a 7 decimal precision. We could potentially limit the precision always to a maximum of 7 decimal values and just cut off the values after for such cases, or what do you think about that @tyrasd and @rtroilo? I guess it would make sense to integrate that in the OSHDB though and not in the ohsome API.

@FabiKo117 FabiKo117 added the upstream Issue or feature request related to an upstream dependency (e.g. OSHDB) label Feb 22, 2021
@tyrasd
Copy link
Member

tyrasd commented Feb 22, 2021

which looks to me like there is some issue within […]

it's actually caused by the floating point arithmetic of the CPU:

$ jshell
jshell> double x = 494051089 * 1E-7
x ==> 49.405108899999995

$ node
> 494051089 * 1E-7
49.405108899999995

$ python3
>>> 494051089 * 1E-7
49.405108899999995

We could potentially limit the precision always to a maximum of 7 decimal values and just cut off the values after […]

Hm, I would say rounding is better than truncating. e.g. 49.405108899999995 should be returned as 49.4051089 not as 49.4051088.

I guess it would make sense to integrate that in the OSHDB though and not in the ohsome API.

see GIScience/oshdb#320 (comment)

Summarizing, IMHO it's best to round the coordinates at the output stage, so in the ohsome-API.

--

cause topological errors

@SlowMo24 at first glance I think this would not be the case here since every single coordinate pair should be returned with the same rounding errors applied, so you would never run into issues of having the same coordinate once reported as …x99999 and once as …x. Except if OSHDB/ohsome-API data is mashed up with OSM data from other sources perhaps.

@tyrasd tyrasd removed bug Something isn't working upstream Issue or feature request related to an upstream dependency (e.g. OSHDB) labels Feb 22, 2021
@SlowMo24
Copy link
Contributor Author

SlowMo24 commented Feb 22, 2021

Yes rounding would be the correct behaviour to get back the original coordinates and it might in fact be best done at the latest point possible but this would leave OSHDB-Geometries vulnerable but that is up to the developers of course.

I'm though not sure why this is not considered a bug. Of course the amount of error is very small but is it constant and affects (from what I have seen) >50% of the coordinates (or 100% of requests) and causes issues on the client.

at first glance I think this would not be the case here since every single coordinate pair should be returned with the same rounding errors

I was also surprised by this error but non-noded intersection as well as the topology were both solved by snapping the geometries to a seven decimal grid.

@FabiKo117
Copy link
Contributor

FabiKo117 commented Feb 22, 2021

Hm, I would say rounding is better than truncating.

Makes sense. I will reference this then in a PR and apply the rounding on 7 digits to every output geometry coordinate. Should be pretty easy to implement and apply.

I'm though not sure why this is not considered a bug.

I guess that is arguable and depends how you define a bug. I would say that it is not a bug within the ohsome API, but can potentially cause issues in other services that deal with the response data. So the bug label here in the ohsome API repo would refer to to things that are not working correctly inside the code of the ohsome API and cause problems there. You could maybe also extend it a bit with producing not-intended things that cause problems elsewhere (like this issue here for instance). So for me, I'm fine with or without the bug label in this case.

@tyrasd tyrasd added the bug Something isn't working label Feb 22, 2021
FabiKo117 added a commit that referenced this issue Feb 23, 2021
to better reflect the actual change

Co-authored-by: Martin Raifer <[email protected]>
FabiKo117 added a commit that referenced this issue Feb 24, 2021
round output coordinates to 7 decimal places
closes #138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants