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

Off by 1 meter? #27

Open
apiszcz opened this issue Sep 29, 2017 · 18 comments
Open

Off by 1 meter? #27

apiszcz opened this issue Sep 29, 2017 · 18 comments
Labels

Comments

@apiszcz
Copy link

apiszcz commented Sep 29, 2017

I reviewed this capability and am seeing a 1 meter difference when converting from DD to MGRS. I compared against geotrans and ESRI outputs and note a 1 meter difference.

@DanielJDufour
Copy link
Collaborator

Hi, @apiszcz . A new maintainer here! Thanks for catching this! Could you provide some specific examples?

@jbaruch76
Copy link

I have seen this behavior. Passing mgrs.forward([0, 0]) I return 31NAA6602100000 which is actually 00.0000000°, -000.0000040°.

@DanielJDufour
Copy link
Collaborator

Thank you @jbaruch76 . Increasing the precision of this library is a top priority and I'm open to any help you can provide!

You can run tests using NGA's GEOTRANS testing data by running CHECK_GEOTRANS=true npm test after running cd test; node setup.js;

Thank you!!

@jbaruch76
Copy link

Have been playing around with this a bit. Unfortunately most of the math is going above my head. I did find however that it seems to be related to the accuracy number. I removed this if statement and just set result to what is in the else statement. https://github.com/proj4js/mgrs/blob/master/mgrs.js#L264. I then started to get the results I was expecting. The hardcoded results in the test don't match what I find using this online converter. http://www.earthpoint.us/Convert.aspx When I change the tests to match what I find here, they pass. What is the point of the accuracy number?

@Klaus-Tockloth
Copy link

I have seen this behavior. Passing mgrs.forward([0, 0]) I return 31NAA6602100000 which is actually 00.0000000°, -000.0000040°.

What is your expected result?

@jbaruch76
Copy link

jbaruch76 commented Sep 26, 2019

I have seen this behavior. Passing mgrs.forward([0, 0]) I return 31NAA6602100000 which is actually 00.0000000°, -000.0000040°.

What is your expected result?

I thought it should be 0, 0, but actually, I'm not too sure now. Looking at http://www.earthpoint.us/Convert.aspx when I enter 31NAA6602100000, it comes back 00.0000000°, -000.0000040° but if I do the reverse and enter 0, 0 it returns mgrs 31NAA6602100000. So I'm not sure where the issue is at the moment.

@Klaus-Tockloth
Copy link

$ echo 31NAA6602100000 | GeoConvert -g -p 2
0.0000045 0.0000005

@jbaruch76
Copy link

$ echo 31NAA6602100000 | GeoConvert -g -p 2
0.0000045 0.0000005

Where did you get this GeoConvert tool? Is that what I should use to confirm results rather than http://www.earthpoint.us/Convert.aspx ?

@apiszcz
Copy link
Author

apiszcz commented Sep 26, 2019 via email

@DanielJDufour
Copy link
Collaborator

@jbaruch76 , I don't believe I answered your question about the accuracy number. The accuracy number tells us the size of the MGRS grid squares that we use to locate the point. In other words, a low accuracy might have accuracy within 100km whereas a higher accuracy can locate the point within 1 meter. I hope that helps.

I'm definitely open to pull requests if people want to make the code a little more human readable and fix bugs in https://github.com/proj4js/mgrs/blob/master/mgrs.js :-)

@EricKit
Copy link

EricKit commented Jul 24, 2020

@DanielJDufour I'm having a similar issue. I currently have a coordinate model that I use to convert between D.D, DM.M, DMS.S, and MGRS.

The problem I'm getting is that my base coordinates are stored in D.D (for more precision I store the decimal separate from the whole number portion).

However, the accuracy I obtain from this library gives me 1 meter off. Here is an example with taking an MGRS string, converting to decimal, and converting back to a string adds a meter:

      const [lng, lat] = mgrs.toPoint('11S PA 22');
      console.log(mgrs.forward([lng, lat]));
      //logs 11SPA2000120001 instead of 11SPA2000020000

I really appreciate this library and will gladly contribute if I can help. I understand that the result is actually a 1m x 1m box, perhaps I should be using bbox instead?

@hodbauer
Copy link

hodbauer commented Jan 21, 2021

Hello,
I've got the same issue and I think that I understand why it happened.
The code in master solve the problem in the private method LLtoUTM because it use Math.trunc for northing and easting instead of Math.round. (there is an explanation about it in Wikipedia )
But the code that published to npm is from branch build from 2014 and not include this fix.

@EricKit
Copy link

EricKit commented Jan 23, 2021

Guess the solution for now is just to pull this project and use what's in master instead of npm. Thanks.

@gudatcomputers
Copy link

@DanielJDufour Given this comment about what's on master being right and what's published on npm containing the off by 1 error, can we just publish that to npm or do you have other concerns?

@spatialillusions
Copy link

Would be appreciated if you could publish this fix to NPM.
Thanks!

@DanielJDufour
Copy link
Collaborator

Hello. I got a prerelease branch out. Could you npm install mgrs@next? And let me know if that version works for you? Once I get confirmation, I'll publish the new version. Thank you!

And have a great weekend!

@spatialillusions
Copy link

Thanks for the quick reply!

It works fine for me after updating. (https://spatialillusions.com/unitgenerator2/ temporary url)
First I run toPoint(mgrs) to normalise all input, and then forward(point) to get MGRS for all types of input, and after I get back the same as I input, or what the user expect
Input "42SUF1234" now gives back "42SUF1200034000" instead of "42SUF1200134001" or something similar.

Great work!

@DanielJDufour
Copy link
Collaborator

@spatialillusions , I believe the credit for that fix goes to the original library authors (@ahocevar and @calvinmetcalf ). I just published a new version :-)

I also promoted the prelease version to a major version, so running npm install mgrs will now install version 2.0.0.

Thanks for your interest and please do let me know if you encounter any more bugs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants