proj4js / mgrs

Utility for converting between WGS84 lat/lng and MGRS coordinates
MIT License
105 stars 45 forks source link

Off by 1 meter? #27

Open apiszcz opened 7 years ago

apiszcz commented 7 years ago

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 commented 5 years ago

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

jbaruch76 commented 5 years ago

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

DanielJDufour commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jbaruch76 commented 5 years ago

$ 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 commented 5 years ago

With another converter i see 0.0 0.00000398

On Thu, Sep 26, 2019 at 8:41 AM jbaruch76 notifications@github.com wrote:

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?

This should be 0, 0 according to http://www.earthpoint.us/Convert.aspx

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/proj4js/mgrs/issues/27?email_source=notifications&email_token=AAK5KTNBNVBZRICJ2NFXYBDQLSUZJA5CNFSM4D5COZC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7VNOQA#issuecomment-535484224, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK5KTIZBKVOXGNMDW4CB2DQLSUZJANCNFSM4D5COZCQ .

DanielJDufour commented 5 years ago

@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 commented 4 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

gudatcomputers commented 2 years ago

@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 commented 1 year ago

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

DanielJDufour commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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.