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

On day of time changes, the sunrise is off by 1 if the time is set to midnight #10

Open
sgabler opened this issue Dec 3, 2021 · 2 comments
Labels

Comments

@sgabler
Copy link

sgabler commented Dec 3, 2021

I noticed an issue that might be a bug, or at least something that I misunderstood from the examples provided in the README.

Example: We have a location in Berlin, Germany. We now want to calculate the sunrise for the following three days

  • day before winter time change: Oct 30 2021
  • day of winter time change: Oct 31 2021
  • day after winter time change: Nov 1 2021

The way I used the library, the correct sunrise/sunset was calculated for the day before and the day after, but not for the day of the winter time change.

  • Oct 30 2021: 2021-10-30T07:58:41
  • Oct 31 2021: 2021-10-31T08:00:35 (wrong, off by one hour, should be 07:00:35)
  • Nov 01 2021: 2021-11-01T07:02:25

For each case I created a DateTime with Kind Unspecified. So for the example of Oct 31:

var date = new DateTime(2021, 10, 31, 0, 0, 0);
var solarTimes = new SolarTimes(date, 52.50, 13.35);

This calculates the wrong date, because at 00:00 the UTC offset is still 2. At 3:00 the UTC offset changes to 1. The offset is used in internal calculations, which I found out after debugging a while.

So as a workaround/fix, we are now always passing a DateTime set to noon (12:00).

var date = new DateTime(2021, 10, 31, 12, 0, 0);
var solarTimes = new SolarTimes(date, 52.50, 13.35);

Then it works very well.

In my mind that seems like a bug, because even if you are checking the sunrise/sunset of a day before 3:00 AM (e.g. via DateTime.Now), you should be able to get the correct sunrise/sunset for that day.

The above examples are a bit simplified from my real code and I didn't double check them, but the point is that some days have 2 valid UTC offsets, and if the wrong one is accidentally used, then the sunrise is off by 1.

Did I misunderstand something maybe, or might this be a bug?

@jbfraser1
Copy link

jbfraser1 commented Jan 17, 2022

I've got a repro, and I think I've got a good idea for the fix. I'll try to get tests and a PR in the next day or two.

The easiest work around in the meantime is to provide a Utc DateTime as input and convert after getting the results back. Treat the returned DateTime as Utc
(My investigation revealed another issue. If you provide a Utc DateTime, the DateTime returned should probably have Kind set to Utc as well, but currently is Unspecified.)

Repro when run on a system in United States Central Time:

using Innovative.SolarCalculator;

var date = new DateTime(2022, 3, 10, 0, 0, 0, DateTimeKind.Unspecified);
var solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise1 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 11, 0, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise2 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 12, 0, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise3 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 13, 0, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise4 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 14, 0, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise5 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 15, 0, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
var sunrise6 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

// shows a jump back of one hour, and then a jump forward of one hour. This seems to be the described problem.
// one of those jumps (forward) is correct, as part of Daylight Saving
Console.WriteLine($"Sunrise2 is ~{((sunrise2 - sunrise1).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise3 is ~{((sunrise3 - sunrise2).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise4 is ~{((sunrise4 - sunrise3).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise5 is ~{((sunrise5 - sunrise4).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise6 is ~{((sunrise6 - sunrise5).TotalMinutes) - 1440:#0.##} difference from the day before.");



date = new DateTime(2022, 3, 10, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise1 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 11, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise2 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 12, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise3 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 13, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise4 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 14, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise5 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 15, 0, 0, 0, DateTimeKind.Utc);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise6 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


// shows correct times, UTC, but math shows jump since these are Unspecified Kind, should be UTC.
Console.WriteLine($"Sunrise2 is ~{((sunrise2 - sunrise1).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise3 is ~{((sunrise3 - sunrise2).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise4 is ~{((sunrise4 - sunrise3).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise5 is ~{((sunrise5 - sunrise4).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise6 is ~{((sunrise6 - sunrise5).TotalMinutes) - 1440:#0.##} difference from the day before.");



date = new DateTime(2022, 3, 10, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise1 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 11, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise2 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


date = new DateTime(2022, 3, 12, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise3 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 13, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise4 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 14, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise5 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");

date = new DateTime(2022, 3, 15, 12, 0, 0, DateTimeKind.Unspecified);
solarTimes = new SolarTimes(date, 44.927481, -93.306843);
sunrise6 = solarTimes.Sunrise.ToUniversalTime();
Console.WriteLine($"sunrise for: {date} : {solarTimes.Sunrise} ({solarTimes.Sunrise.ToUniversalTime()})");


// Shows correct times, local to the sunrise.
Console.WriteLine($"Sunrise2 is ~{((sunrise2 - sunrise1).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise3 is ~{((sunrise3 - sunrise2).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise4 is ~{((sunrise4 - sunrise3).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise5 is ~{((sunrise5 - sunrise4).TotalMinutes) - 1440:#0.##} difference from the day before.");
Console.WriteLine($"Sunrise6 is ~{((sunrise6 - sunrise5).TotalMinutes) - 1440:#0.##} difference from the day before.");

output:

sunrise for: 3/11/2022 12:00:00 AM : 3/11/2022 6:33:23 AM (3/11/2022 12:33:23 PM)
sunrise for: 3/12/2022 12:00:00 AM : 3/12/2022 6:31:32 AM (3/12/2022 12:31:32 PM)
sunrise for: 3/13/2022 12:00:00 AM : 3/13/2022 6:29:42 AM (3/13/2022 11:29:42 AM)
sunrise for: 3/14/2022 12:00:00 AM : 3/14/2022 7:27:55 AM (3/14/2022 12:27:55 PM)
sunrise for: 3/15/2022 12:00:00 AM : 3/15/2022 7:26:04 AM (3/15/2022 12:26:04 PM)
Sunrise2 is ~-1.83 difference from the day before.
Sunrise3 is ~-1.84 difference from the day before.
Sunrise4 is ~-61.85 difference from the day before.
Sunrise5 is ~58.23 difference from the day before.
Sunrise6 is ~-1.86 difference from the day before.
sunrise for: 3/10/2022 12:00:00 AM : 3/10/2022 12:35:40 PM (3/10/2022 6:35:40 PM)
sunrise for: 3/11/2022 12:00:00 AM : 3/11/2022 12:33:50 PM (3/11/2022 6:33:50 PM)
sunrise for: 3/12/2022 12:00:00 AM : 3/12/2022 12:32:00 PM (3/12/2022 6:32:00 PM)
sunrise for: 3/13/2022 12:00:00 AM : 3/13/2022 12:30:09 PM (3/13/2022 5:30:09 PM)
sunrise for: 3/14/2022 12:00:00 AM : 3/14/2022 12:28:18 PM (3/14/2022 5:28:18 PM)
sunrise for: 3/15/2022 12:00:00 AM : 3/15/2022 12:26:27 PM (3/15/2022 5:26:27 PM)
Sunrise2 is ~-1.83 difference from the day before.
Sunrise3 is ~-1.84 difference from the day before.
Sunrise4 is ~-61.84 difference from the day before.
Sunrise5 is ~-1.85 difference from the day before.
Sunrise6 is ~-1.85 difference from the day before.
sunrise for: 3/10/2022 12:00:00 PM : 3/10/2022 6:35:13 AM (3/10/2022 12:35:13 PM)
sunrise for: 3/11/2022 12:00:00 PM : 3/11/2022 6:33:23 AM (3/11/2022 12:33:23 PM)
sunrise for: 3/12/2022 12:00:00 PM : 3/12/2022 6:31:32 AM (3/12/2022 12:31:32 PM)
sunrise for: 3/13/2022 12:00:00 PM : 3/13/2022 7:29:46 AM (3/13/2022 12:29:46 PM)
sunrise for: 3/14/2022 12:00:00 PM : 3/14/2022 7:27:55 AM (3/14/2022 12:27:55 PM)
sunrise for: 3/15/2022 12:00:00 PM : 3/15/2022 7:26:04 AM (3/15/2022 12:26:04 PM)
Sunrise2 is ~-1.83 difference from the day before.
Sunrise3 is ~-1.84 difference from the day before.
Sunrise4 is ~-1.77 difference from the day before.
Sunrise5 is ~-1.85 difference from the day before.
Sunrise6 is ~-1.86 difference from the day before.

@porrey porrey added the bug label Jan 18, 2022
@bmasotta-lcg
Copy link

bmasotta-lcg commented May 9, 2023

If you don't need full exact date precision, you can just convert the library DateTime objects to TimeOnly objects taking into account that times across two days will require additional logic for range checking (sunrise hhmm > sunset hhmm)

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

4 participants