4

Closed

Southern hemisphere: GetDaylightChanges.Start and .End are reversed

description

In PublicDomain.TzTimeZone, GetDaylightChanges(iDSTYear).Start and GetDaylightChanges(iDSTYear).End return reverse values (.Start returns a DST off value and .End the DST on value) for time zone locales in the southern hemisphere.
 
Examples are 'Pacific/Auckland', all Australian TZs with DST offset such as 'Australia/Adelaide', 'America/Sao_Paulo', etc.

file attachments

Closed Dec 19, 2007 at 3:02 AM by schizoidboy

comments

schizoidboy wrote Oct 17, 2007 at 2:11 AM

Hey Mario, I haven't looked at this in detail yet, but do you know if time zone conversions work properly for those zones? I believe GetDaylightChanges shares code with the conversion code?

schizoidboy wrote Oct 17, 2007 at 2:17 AM

This is what I get for the zones mentioned -- what do you expect these to be?

Pacific/Auckland
Start: 3/18/2007 3:00:00 AM
End: 9/30/2007 2:00:00 AM

Australia/Adelaide
Start: 10/7/2007 3:00:00 AM
End: 10/28/2007 3:00:00 AM

America/Sao_Paulo
Start: 2/25/2007 12:00:00 AM
End: 11/4/2007 12:00:00 AM

MARIOIGREC wrote Nov 1, 2007 at 4:17 PM

I think the problem is that the TZTimeZone code assumes that an earlier rule date in the year is the start date. And in the case of rules like these, the start date is where the offset is <> 0 (1:00 in the 'SAVE' column before the last):

Rule NAME FROM TO TYPE IN ON AT SAVE LETTER/S

Rule Brazil 2007 max - Oct Sun>=8 0:00 1:00 S
Rule Brazil 2008 max - Feb Sun>=15 0:00 0 -

Here's what I expect:

Pacific/Auckland
End: 3/18/2007 3:00:00 AM DST time (2:00:00 AM standard time)
Start: 9/30/2007 2:00:00 AM local standard time
End: 4/6/2008 3:00:00 AM DST time (the off rule changes in 2008!)
Start: 9/28/2008 2:00:00 AM local standard time

Australia/Adelaide (the Olson DST rule is 'AS', which is: on Oct lastSun 2:00s, off >=2007 Mar lastSun 2:00s)
Start: 10/28/2007 2:00:00 AM
End: 3/30/2008 3:00:00 AM (2:00:00 AM standard time)

America/Sao_Paulo is tough. http://home.tiscali.nl/~t876506/TZworld.html#nam has the off date for 2007 as the last rule. tzdata2007i.tar.gz southamerica file says:
Rule Brazil 2007 max - Oct Sun>=8 0:00 1:00 S
Rule Brazil 2008 max - Feb Sun>=15 0:00 0 -

For dates after mid-2008, the above rules with TO="max" are guesses

and are quite possibly wrong, but are more likely than no DST at all.

So, my guess is:
Start: 11/5/200612:00:00 AM (according to Rule Brazil 2006 only - Nov 5 0:00 1:00 S)
End: 2/25/2007 1:00:00 AM DST (12:00:00 standard time) (according to Rule Brazil 2007 only - Feb 25 0:00 0 -)
Start: 10/14/2007 1:00:00 AM standard time (according to the assumed rule above)
End: 2/17/2008 1:00:00 AM DST (12:00 standard time) (according to the assumed rule above)

AleksLevshin wrote Dec 11, 2007 at 7:57 AM

I have found the same problem.

schizoidboy wrote Dec 11, 2007 at 8:41 PM

This is certainly a bug. There was a post made over on the codeproject article about PublicDomain saying the following:

"This is great stuff!! However, there are two bugs that make the code completely useless in the southern hemisphere.

First of all, you're assuming the "LETTER/S" field in the database is set to "S" for standard time and "D" for daylight time. That's not true in many locations (for example, Australia). I've changed your code for FindRuleIndex so that instead of taking an "avoidModifier" parameter, it takes a bool? parameter that is null if we don't care, true to avoid the standard time and false to avoid the daylight savings time. It's basically the same as your avoidModifier parameter, but based off the TzRule.SaveTime property rather than TzRule.Modifier.

Secondly, you're assuming that daylight saving time starts in the first half of the year, and ends in the second half of the year. That's true in the northern hemisphere (U.S. and Europe, for example) but it's not true in the southern hemisphere, where daylight saving time starts at the end of the year, and finishes at the beginning of the next year. This is evident in the GetDaylightChangeRules method, where you're assuming the "from" and "to" date/time values have the same Year component. "

I'm not sure yet how to work the nullable bool change in. I'm trying to contact the author to see if he has the code changes...

Codeka wrote Dec 11, 2007 at 10:39 PM

OK, I don't actually have the original source anymore, as I already modified it to work with my code, so I'm doing most of this from memory now :-)

Anyway, I modified FindRuleIndex as follows (removed comments for conciseness):
    public int FindRuleIndex(TzDatabase.TzZone zone, DateTime point, bool? findStandardTime, bool exactComparison)
    {
        string ruleName = zone.RuleName;

        int curRuleIndex = -1;

        if (ruleName != TzDatabase.NotApplicableValue)
        {
            int rulesLength = m_info.Rules.Count;
            if (rulesLength == 0 && !string.IsNullOrEmpty(ruleName))
            {
                throw new Exception(string.Format("No rules found for rule name {0}", ruleName));
            }

            for (int j = 0; j < rulesLength; j++)
            {
                TzDatabase.TzRule r = m_info.Rules[j];

                if (r.RuleName == ruleName)
                {
                    curRuleIndex = j;
                    if ((exactComparison && point >= r.GetFromDateTime(zone.UtcOffset) && point < r.GetToDateTime(zone.UtcOffset)) ||
                        (!exactComparison && point.Year >= r.FromYear && point.Year <= r.ToYear))
                    {
                        if (findStandardTime == null ||
                            (findStandardTime != null && findStandardTime.Value && r.SaveTime == TimeSpan.Zero) ||
                            (findStandardTime != null && !findStandardTime.Value && r.SaveTime != TimeSpan.Zero))
                        {
                            break;
                        }
                    }
                }
            }

            if (curRuleIndex == -1)
            {
                throw new Exception("Could not find rule");
            }
        }
        return curRuleIndex;
    }
Then, basically when you call it, where you use to call it with 'null' as the 3rd parameter, you still pass null. Otherwise you pass 'true' for "S" or 'false' for "D".

For the other problem, I didn't really know what to do. I basically have a hard-coded list of zones that I know are in the southern hemisphere and inside GetDaylightChangeRules, where you've got the following:
    DateTime from = one.GetDateTime(year, targetZone.UtcOffset, two);
    DateTime to = two.GetDateTime(year, targetZone.UtcOffset, one);
I changed it to:
    DateTime from = one.GetDateTime(year, targetZone.UtcOffset, two);
    if (StandardName.StartsWith("Australia") /* || etc */)
        year += 1;
    DateTime to = two.GetDateTime(year, targetZone.UtcOffset, one);
I'm pretty sure that's all I needed to change, but again, I'm just going from memory here :-)

Additionally, in GetTimeZone, you would pre-hash the names of the time zones when the source file was constructed and then do the switch() on the hash of the name passed in. The problem with that is the implementation of string.ToHashCode() has changed with different releases of the .NET framework (even in service packs!) so you've got make sure you compile with exactly the same framework version as you run with. Instead, I just use a string-based switch() which is implemented by the runtime as a jumped based of a hash dictionary anyway -- the difference in speed is minimal, but you don't have to worry about compiling against the same version of the framework as you run against.

Hope that helps!!

Codeka wrote Dec 11, 2007 at 10:40 PM

Sorry the code doesn't come out very well, but you can see what I'm getting at, hopefully :-)

bculberson wrote Dec 12, 2007 at 6:44 PM

time zone conversions to australia zones are definately not working right now.

australia is in DST, but the zone does not know that so it is always an hour off on those conversions.

schizoidboy wrote Dec 13, 2007 at 6:07 AM

This bug looks to be affecting a number of people, thanks for the update Codeka, I will look into it. I am pretty busy right now, but I'll try my best to get this fixed soon.

Codeka, as far as the GetHashCode() issue that is a really great point I didn't think of. By string based switch you mean:

switch (tzName) {
case "America/New_York":
 ....
}

etc.
?

I'm not sure what the ETA is on this bug, hopefully a few days and at most a week....

Also, it would help if someone can put together some regressions tests for me (the existing regression tests are in the source code in the PublicDomainTests project under TzTimeZoneTests.cs)

Thanks,
Kevin

Codeka wrote Dec 13, 2007 at 11:02 PM

Yeah, that's what I mean. See here, for example: http://blogs.msdn.com/abhinaba/archive/2006/12/13/why-can-we-only-use-constants-in-a-switch-case-statement.aspx

Anyway, for people who need a fix right now hopefully the code I posted is enough to get them going :-)

AleksLevshin wrote Dec 14, 2007 at 3:44 PM

Codeka, thanks for your code, but it does not solve my problems. I need to get Start/End daylight settings for current year only. But I think there is a problem in swapping Start/End date too.

schizoidboy wrote Dec 14, 2007 at 9:36 PM

bculberson has been helping out and has provided the following patch, although some of the regression tests are failing. The code is a bit different from Codeka's (specifically the adding of 1 year). Not sure when we'll have the fix, but I'll continue working with bculberson. The patch file is attached in case anyone wants to help out. By the way, I'm using svnbridge for this project instead of the default of TFS, so anyone should be able to just download the source code zip and open the solution and build without the annoyances of TFS Version Control binding alerts.

This patch also includes the proposed changes for the switch-based time zone lookup.

schizoidboy wrote Dec 15, 2007 at 5:37 AM

bculberson appears to have fixed this, we're just running some regression tests and should have the new build with the fix up soon...

Thanks

AleksLevshin wrote Dec 15, 2007 at 8:59 AM

Guys, thanks for your help. I have just patched the code and tested it. It seems that a problem still occurs. I wrote my own work around, but it looks too heavy and still returns bad End daylight datetime for some timezones (America/Canacum, America/Chihuahua, America/Cuiaba, America/Ensenada, Mexico_city is going crazy too ;).

schizoidboy wrote Dec 15, 2007 at 7:16 PM

Hey AleksLevshin, the patch that i had uploaded earlier was not working. Here's another patch from bculberson, although it hasn't been finalized. Also, AleksLevshin, what are your use cases so that we can put them into the regression tests?

Thanks

AleksLevshin wrote Dec 17, 2007 at 7:59 AM

Yeah, I have tested new patch and it works fine. But some time zones abbreviations are still incorrect. I found out the following mismatchings: America/Santiago, America/Asuncion, America/Havana and also America/Sao_Paulo time zones have wrong rollover datetime. I compared data against www.timeanddate.com information for 2008 year. I suggest that these problems can be existed in olson db' records.

AleksLevshin wrote Dec 17, 2007 at 2:52 PM

There are incorrect end daylight datetime for following time zones: Australia/ACT (10/07/2007), Australia/Adelaide (10/07/2007), Australia/Broken_Hill (10/07/2007), Australia/Canberra (10/07/2007), Australia/LHI (10/07/2007), Australia/Lord_Howe (10/07/2007), Australia/Melbourne (10/07/2007), Australia/NSW (10/07/2007), Australia/Perth (no rollover time), Australia/South (10/07/2007), Australia/Sydney (10/07/2007), Australia/Victoria (10/07/2007), Australia/Yancowinna (10/07/2007). An error with correct detection of companion rule is still active. So, I added the following changes, in general it is Codeka's method with small modifications:

TzTimeZone.cs
code begins
// first change
private PublicDomain.TzDatabase.TzRule GetCompanionRule(TzDatabase.TzZone zone, PublicDomain.TzDatabase.TzRule rule, DateTime point)
    {
        TzDatabase.TzRule result = null; 
        bool? avoidModifier = null;
        if (rule.SaveTime == TimeSpan.Zero && (rule.Modifier == "-" || rule.Modifier == "D")) {
            // we need to find DST end
            avoidModifier = false;
        } else if (rule.SaveTime != TimeSpan.Zero && (rule.Modifier == "-" || rule.Modifier == "S")) {
            // we need to find DST start
            avoidModifier = true;
        }
        int companionIndex = FindRuleIndex(zone, point, avoidModifier, false);
        if (companionIndex != -1)
        {
            result = m_info.Rules[companionIndex];
        }
        return result;
    }

// second change
/// <summary>
    /// Finds the rule.
    /// </summary>
    /// <param name="zone">The zone.</param>
    /// <param name="point">The point.</param>
    /// <param name="avoidModifier">The avoid modifier.</param>
    /// <param name="exactComparison">if set to <c>true</c> [exact comparison].</param>
    /// <returns></returns>
    public int FindRuleIndex(TzDatabase.TzZone zone, DateTime point, bool? avoidModifier, bool exactComparison)
    {
        // Now, we have the zone, we can start to figure out the amount
        // of time to add to UTC to get standard time

        // Get the standard time from the UTC Offset for the zone

        // Use the rules to see if we need to apply daylight savings time
        // "If this field is - then standard time
        // always applies in the time zone."
        string ruleName = zone.RuleName;

        int curRuleIndex = -1;

        if (ruleName != TzDatabase.NotApplicableValue)
        {
            // Iterate over the rules to find the
            // applicable rule
            int rulesLength = m_info.Rules.Count;
            if (rulesLength == 0 && !string.IsNullOrEmpty(ruleName))
            {
                throw new BaseException("No rules found for rule name {0}", ruleName);
            }

            for (int j = 0; j < rulesLength; j++)
            {
                PublicDomain.TzDatabase.TzRule r = m_info.Rules[j];

                if (r.RuleName == ruleName)
                {
                    curRuleIndex = j;
                    if ((exactComparison && point >= r.GetFromDateTime(zone.UtcOffset) && point < r.GetToDateTime(zone.UtcOffset)) ||
                        (!exactComparison && point.Year >= r.FromYear && point.Year <= r.ToYear))
                    {
                        if (avoidModifier == null || 
                            (avoidModifier == true && (r.SaveTime == TimeSpan.Zero && (r.Modifier == "-" || r.Modifier == "S"))) ||
                            (avoidModifier == false && (r.SaveTime != TimeSpan.Zero && (r.Modifier == "-" || r.Modifier == "D")))) 
                        {                                    
                            break;
                        }
                    }
                }
            }

            if (curRuleIndex == -1)
            {
                throw new Exception("Could not find rule");
            }
        }
        return curRuleIndex;
    }
<<< code ends

schizoidboy wrote Dec 19, 2007 at 2:06 AM

A few comments. First, bculberson has used the TzZoneInformation class during database parsing which parses the zone.tab file to get whether or not a zone is in the northern hemisphere, and save that off into the TzZoneInfo which can then be used instead of adding a year in finding the companion rule. This fixes the crux of the problem, and I will be uploading this fix into release 0.2.35.0 tonight; however, I'm having some issues with Australia. AleksLevshin, your solution may work, although I'm not convinced it is the best way to do it. I went back into the description of the LETTER/S column and it says:
 LETTER/S
         Gives the "variable part" (for example, the "S" or
         "D" in "EST" or "EDT") of time zone abbreviations to
         be used when this rule is in effect.  If this field
         is -, the variable part is null.
I'm not sure if I'm using this correctly in the use of the "avoidModifier," since really this is just about the variable part of the abbreviation, it does not hinge on any functional value of the zone. In fact, this is why Australia is breaking, because their abbreviation is always AS, and so LETTER/S is always '-' for all the zones. I'm not sure if this will break other zones as well, so I think what I will do is also have an avoidIndex, which finds the next best zone to use other than the one we already have. Any thoughts on this?

Right now all the regression tests are basically working. Once I upload the new release, please test it out, and if there are still problems, please give me the exact zones and dates that are failing and their expected values so that I can try to back-track and see if I need to redesign anything.

Thanks for everyone's help...

schizoidboy wrote Dec 19, 2007 at 2:48 AM

Hey all, well there is some bug in svnbridge committing the changes because TzTimeZone.cs is so huge :-). I will nevertheless create the release and upload the binaries. I am also attaching the patch that I am trying to commit (pending resolution of this bug http://www.codeplex.com/SvnBridge/WorkItem/View.aspx?WorkItemId=9077) in case anybody wants to see the code...

I believe all of the issues are fixed now, so please try it out and let me know if there are further issues. Thanks!

AleksLevshin wrote Dec 19, 2007 at 9:13 AM

Hi, schizoidboy. I applied your latest patch, but unfortunately it does not fix some time zones that have not rollover in 2008 year. For example, Africa/Accra, Asia/Chongqing, Pacific/Efate, Pacific/Rarotonga. The PublicDomain returns equal start/end datetime values for them.

schizoidboy wrote Dec 19, 2007 at 2:51 PM

If you can, please open a new issue and I'll work on it from there, this issue is getting to be too big.

Also, I'm starting to question what is the proper way to fix this, whether by using the latitude or the nullable boolean, but I'm not sure the LETTER/S column or find the "companion rule" is the way to go. Maybe I'll check out the actual C source code for olson...

Thanks

AleksLevshin wrote Dec 20, 2007 at 7:23 AM

Hey. I saw the new ticket was opened. I think it is not a big issue, the start/end values should be compared after GetDateTime(). I placed if (Start == End) condition after GetDaylightChanges() finishes its work in my code, but for me it will be better to put the checking inside your code ;) For example like this,
code begins
GetDaylightChangeRule()
......
if (one != null && two != null)
{
 DateTime from = one.GetDateTime(time.Year, targetZone.UtcOffset, two);
 DateTime to = two.GetDateTime(time.Year, targetZone.UtcOffset, one);

 if (from == to) {
     return null; 
 }
 ......
code ends
Or it can be used nullable type for from/to objects.

AlanBirenbaum wrote Jan 14, 2009 at 11:34 PM

Seems that you also need to modify the else if in the TzTimeZone PrepOffset routine as follows:
    private void PrepOffset(ref DateTime time, ref TimeSpan result, PublicDomain.TzDatabase.TzZone zone)
    {
        // First, figure out which zone we're in
        DateTime point = new DateTime(time.Year, 1, 1);

        PublicDomain.TzDatabase.TzRule one;
        PublicDomain.TzDatabase.TzRule two;
        DaylightTime daylightTime;

        GetDaylightChangeRules(ref point, zone, DateTimeKind.Local, out one, out two, out daylightTime);

        if (daylightTime != null && daylightTime.Start < daylightTime.End && time >= daylightTime.Start && time < daylightTime.End)
        {
            result += one.SaveTime;
        }
        //**orig** else if (daylightTime != null && daylightTime.Start > daylightTime.End && (time <= daylightTime.Start || time > daylightTime.End))
        //                                                                                                          !  check that it's NOT standart time
        else if (daylightTime != null && daylightTime.Start > daylightTime.End && !(time >= daylightTime.End && time < daylightTime.Start))
        {
            result += one.SaveTime;
        }
    }