Tronald / CoordinateSharp

A library designed to ease geographic coordinate format conversions, and determine sun/moon information in C#
Other
365 stars 59 forks source link

ArgumentOutOfRangeException using Puebla, Mexico coordinates #21

Closed dfaroi closed 6 years ago

dfaroi commented 6 years ago

Hi,

I met a case that throw new ArgumentOutOfRangeException : new Coordinate(19.0991676999824, -98.2209974507276, new DateTime(2018,10,31,0,0,0))

It seems that using these coordinates (Albuquerque) and this date, the calculated Sunset time is "24".

Thanks for your help.

Tronald commented 6 years ago

Yep, that is a BIG bug nice find. And it has been there for a while. Have no idea how it's gone this long without being discovered. I will push an update immediately (v1.1.2.2). I'll close this once its pushed.

Just a heads up, your Coordinates appear to be for Southern Mexico not ABQ, NM, USA.

Out of curiosity, what are you using this library for. I see people downloading it, but I always wonder if it's for experimentation or use in an actual application.

Tronald commented 6 years ago

The Sunrises/Sunsets are set to null if a set or rise doesn't occur on the specified day. In the example you provided, the Sunset was occurring, but it occurred at the end of the day at 00:00(Z) and the program treated it as hour 24 instead of 0 since it was the end of the day. I just threw an integrity check in there that will reset a 24 int value to 0.

I just pushed a Nuget Package (v1.1.2.2). Give it a bit to index and show in the search results. Let me know if it works for you.

I really appreciate the contribution!!!

-Justin

dfaroi commented 6 years ago

@Tronald Thanks for your quick answer.

You're right, sorry Provider coordinates was for PUEBLA city of Mexico.

To answer your question, as proof of concept (experimental purpose) :

Sunset/Sunrise computation will definatly be a base element for energy saving concerns

Regards, -David

Tronald commented 6 years ago

Awesome! Glad to see that other people have use for the library.

Thank you again for letting me know about the bug!

dfaroi commented 6 years ago

Thanks,

I’ll give it a try and keep you informed

-- David Faroi

Le 2 févr. 2018 à 16:17, Justin Gielski notifications@github.com a écrit :

The Sunrises/Sunsets are set to null if a set or rise doesn't occur on the specified day. In the example you provided, the Sunset was occurring, but it occurred at the end of the day at 00:00(Z) and the program treated it as hour 24 instead of 0 since it was the end of the day. I just threw an integrity check in there that will reset a 24 int value to 0.

I just pushed a Nuget Package (v1.1.2.2). Give it a bit to index and show in the search results. Let me know if it works for you.

I really appreciate the contribution!!!

-Justin

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

dfaroi commented 6 years ago

Hi, I confirm that Nuget Package (v1.1.2.2) resolve this issue.

I found a new edge case where there is no Sunset computed (same Puebla coordinates): new Coordinate(19.0991676999824, -98.2209974507276, new DateTime(2018,12,20,0,0,0))

According to the following link : suncalc.org, there is a Sunset at 6 PM local time (or midnight at UTC time).

By changing line 49 (in 'Celestial.SunCalculations.cs ' file) from : for (int k = 0; k < 24; k++) to for (int k = 1; k < 25; k++) this issue can be resolved but I don't know if it's relevant.

Thanks for your help. David

Tronald commented 6 years ago

I just tried it out in my production environment and got the same issue.I think increasing my loop iterations will lead to adverse affects in other areas. Let me look into this.

I need to make better unit tests to catch this stuff. I'll get back to you.

Tronald commented 6 years ago

Your solution looks good. It catches those odd instances where the time may be 24.000053234, but doesn't appear to allow any calculations into the next day. I'm going to build a more thorough test and run a loop from the -65 to +65 parallels and search for any odd behaviors regarding no sets/rises.

I also found a glitch regarding dawn and dusk that I need to patch up while checking this out. I forgot to handle instances where there may be a sunrise or set but no dawn dusk.

Your contribution to this has been very helpful. I'll post when the package is pushed.

dfaroi commented 6 years ago

Justin,

Sorry I’ll be out of office for few weeks.

Thanks again for your help.

Cheers,

David

Le 5 févr. 2018 à 19:25, Justin Gielski notifications@github.com a écrit :

Assigned #21 to @dfaroi.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

Tronald commented 6 years ago

No problem, Cheers!

Tronald commented 6 years ago

Upon further testing, the extra iteration is causing issues at certain locations. I analyzed the issue further and noticed that on the specific day at the coordinate you provided, the Sun Set actually doesn't occur on that (Z) day. This is due to the event slowly shifting times over midnight and how the formula works I believe.

For example, the sun could set at 23:59:59:59999999999999 on 12/19/2018 and then not set again until 00:00 on 12/21/2018. It's slightly greater than a 24 hour hour period by milliseconds.

I'm looking at treating horizon values of .002 and less as -.002 and visa versa. this will trigger a horizon crossing event during hour 0 in the TestHour() call and allow the formula to set a Set or Rise condition during these odd circumstances.

I'm running loops down the the the Minute (degrees) level to test this now.

Tronald commented 6 years ago

Got it working. I ended having to do a 25 hour iteration with some post loop checks. Going to push an update in a bit. Thanks again!