mourner / suncalc

A tiny JavaScript library for calculating sun/moon positions and phases.
BSD 2-Clause "Simplified" License
3.08k stars 412 forks source link

Incorrect moon rise/set times when Dec 31st #61

Closed squarefeet closed 8 years ago

squarefeet commented 8 years ago

Hey,

I've just grabbed this library to do some day/night cycle stuff, and noticed an error when using getMoonTimes(). Code to reproduce is below (I've manually set the date and time since it might not be 31st December when you see this):

var suncalc = require( 'suncalc' );

var moonDate = new Date();
moonDate.setHours( 14 );
moonDate.setDate( 31 );
moonDate.setMonth( 11 );
moonDate.setYear( 2015 );

var moontimes = suncalc.getMoonTimes(
    moonDate,
    51.51, 
    -0.13
);

console.log( moontimes );

// Outputs:
//    { 
//      rise: Thu Dec 31 2015 10:59:33 GMT+0000 (GMT),
//      set: Thu Dec 31 2015 10:09:59 GMT+0000 (GMT) 
//    }

I think the rise time is out by 12 hours, and the rise and set times should exchange places?

Thanks for the lib, btw!

EDIT: Corrected moonDate.setDate to set 31st, not 30th! Apologies.

jspeaker commented 8 years ago

I'm not able to repro the behavior you've described. Also, I do not recognize "rise" and "set" as properties of the object that is returned by the getMoonTimes method. Here is the output I get in chrome browser tools:

SunCalc.getTimes(new Date(2015, 11, 30, 14), 51.51, -0.13);: Object dawn: Tue Dec 29 2015 23:27:24 GMT-0800 (Pacific Standard Time) dusk: Wed Dec 30 2015 08:40:35 GMT-0800 (Pacific Standard Time) goldenHour: Wed Dec 30 2015 06:59:19 GMT-0800 (Pacific Standard Time) goldenHourEnd: Wed Dec 30 2015 01:08:40 GMT-0800 (Pacific Standard Time) nadir: Tue Dec 29 2015 16:04:00 GMT-0800 (Pacific Standard Time) nauticalDawn: Tue Dec 29 2015 22:44:22 GMT-0800 (Pacific Standard Time) nauticalDusk: Wed Dec 30 2015 09:23:37 GMT-0800 (Pacific Standard Time) night: Wed Dec 30 2015 10:04:20 GMT-0800 (Pacific Standard Time) nightEnd: Tue Dec 29 2015 22:03:40 GMT-0800 (Pacific Standard Time) solarNoon: Wed Dec 30 2015 04:04:00 GMT-0800 (Pacific Standard Time) sunrise: Wed Dec 30 2015 00:07:31 GMT-0800 (Pacific Standard Time) sunriseEnd: Wed Dec 30 2015 00:11:53 GMT-0800 (Pacific Standard Time) sunset: Wed Dec 30 2015 08:00:28 GMT-0800 (Pacific Standard Time) sunsetStart: Wed Dec 30 2015 07:56:06 GMT-0800 (Pacific Standard Time)

squarefeet commented 8 years ago

Oh dear... I made a pretty bad mistake in my post: It should read 31st Dec, not 30th! I've updated it accordingly. It'll explain why you're not seeing the same issue as me.

I'm running this in NodeJS v0.12.0, not in the browser. Trying in the browser, I get the following output that matches the issue I'm seeing in Node:

rise: Thu Dec 31 2015 10:59:33 GMT+0000 (GMT)
set: Thu Dec 31 2015 10:09:59 GMT+0000 (GMT)

I'm not sure what you mean by not recognising rise and set properties – your example is calling getTimes(), not getMoonTimes(). The getTimes() method doesn't have rise and set properties, whereas the getMoonTimes() one does.

mourner commented 8 years ago

Could it be explained by just the timezone differences? What happens if you pass true as the 4th argument (inUTC)?

jspeaker commented 8 years ago

Wow. Indeed. (facepalm) I'll be trying this again, but for the moment I'm gonna bow out, embarrassed.

jspeaker commented 8 years ago

I've been running a number of experiments in my browser tools. And, oddly, the problem seems to be related to the precision of the longitude decimal. The problem rears its ugly head when decimal is reduced to two digits on the longitude. Following are a couple of examples:

Repro: SunCalc.getMoonTimes(new Date(2015, 11, 31), 51.51, -0.13): Object rise: Thu Dec 31 2015 02:59:33 GMT-0800 (Pacific Standard Time) set: Thu Dec 31 2015 02:09:59 GMT-0800 (Pacific Standard Time)

No Repro: SunCalc.getMoonTimes(new Date(2015, 11, 31), 51.51, -0.131): Object rise: Thu Dec 31 2015 15:02:04 GMT-0800 (Pacific Standard Time) set: Thu Dec 31 2015 03:00:39 GMT-0800 (Pacific Standard Time)

Also, it has nothing to do with the 4th argument, which I did not find has any effect at all:

SunCalc.getMoonTimes(new Date(2015, 11, 31), 51.51, -0.131, true): Object rise: Thu Dec 31 2015 15:02:04 GMT-0800 (Pacific Standard Time) set: Thu Dec 31 2015 03:00:39 GMT-0800 (Pacific Standard Time)

SunCalc.getMoonTimes(new Date(2015, 11, 31), 51.51, -0.131, false): Object rise: Thu Dec 31 2015 15:02:04 GMT-0800 (Pacific Standard Time) set: Thu Dec 31 2015 03:00:39 GMT-0800 (Pacific Standard Time)

Vladimir, hoping this helps you narrow it down. I'd dig, but I really need to get to work now. Cheers

jspeaker commented 8 years ago

By the way, if you want a quick and dirty way to play with SunCalc in your browser, you can visit https://fishing.smarttripmap.com/ - the sunrise/sunset and moonrise/moonset times are based on the selected location (just drop a marker), and the date in the Conditions pane, which you can change.

fstm-crop

squarefeet commented 8 years ago

@mourner I tried that, and many other things to see if I could find the cause, but didn't have any luck. Looks like @jspeaker got there instead - huge thanks for finding the cause!

I'll spend a bit of time digging into the code to see if there's a way to mitigate this issue when there isn't "enough" precision in the lat/longs, although there's no particular reason I used 2 decimal places other than laziness, but it would be interesting to find out why it's doing what it's doing.

Thanks again, @jspeaker :) Also, that fishing site you linked to is really rather cool – if you made it, nice work!

jspeaker commented 8 years ago

@squarefeet, glad that helped!

Yes, Smart Trip Map has been a labor of love for over 6 years now. Thank you, I appreciate it!

mourner commented 8 years ago

Can you check whether it's better now after merging https://github.com/mourner/suncalc/pull/64? It might have fixed this issue.