The Inbox: Chronology-Core-cmm.14.mcz
Locked
 
|
Chris Muller uploaded a new version of Chronology-Core to project The Inbox:
http://source.squeak.org/inbox/Chronology-Core-cmm.14.mcz ==================== Summary ==================== Name: Chronology-Core-cmm.14 Author: cmm Time: 17 October 2018, 10:07:14.485014 pm UUID: 5d93900c-4a4e-4a4e-80f3-800dbdb07d0b Ancestors: Chronology-Core-tcj.12 - A fix and optimization of Timespan>>#=. Both elements being compared must have the same timezone (or same state of #noTimezone) in order to take advantage of the optimized #hasEqualTicks: comparison. Otherwise (if different timezones), a full comparison of their starts (via #=) is needed. - There was a mention of this optimization put into the class comment. This level of detail may be a bit tedious for users to read at that level, so Brents original comment was restored. =============== Diff against Chronology-Core-tcj.12 =============== Item was changed: Magnitude subclass: #Timespan instanceVariableNames: 'start duration' classVariableNames: '' poolDictionaries: '' category: 'Chronology-Core'! + !Timespan commentStamp: 'cmm 10/17/2018 22:00' prior: 0! + I represent a duration starting on a specific DateAndTime.! - !Timespan commentStamp: 'bf 2/18/2016 14:43' prior: 0! - I represent a duration starting on a specific DateAndTime. - - If my start has an offset identical to my #defaultOffset then comparisons ignore timezone offset.! Item was changed: ----- Method: Timespan>>= (in category 'ansi protocol') ----- = comparand + ^ self class = comparand class + and: [(((self noTimezone and: [comparand noTimezone]) or: [self start offset = comparand start offset]) + ifTrue: [ self start hasEqualTicks: comparand start ] + ifFalse: [ self start = comparand start ]) + and: [ self duration = comparand duration ] ] - ^ self class = comparand class - and: [((self noTimezone or: [ comparand noTimezone ]) - ifTrue: [ self start hasEqualTicks: comparand start ] - ifFalse: [ self start = comparand start ]) - and: [ self duration = comparand duration ] ] .! |
|
|
I like this. Although I notice that the comparison makes sure that the classes are the same - so a Timespan starting at the exact same time as a Date and with 24 hour duration is not the same as a Date with the exact same values. This would allow further speedups if Date, Week, Month, and Year if we wanted - just drop the comparison of duration in those classes. (The general case is still needed to tell Timespan apart from Schedule.) If you did that, you probably also want to re-implement hash as well - simplifying it the same way. -cbc On Wed, Oct 17, 2018 at 8:07 PM <[hidden email]> wrote: Chris Muller uploaded a new version of Chronology-Core to project The Inbox: |
Re: The Inbox: Chronology-Core-cmm.14.mcz
|
|
I'm not sure this is right. The current implementation comes from the "trunk thinks its tomorrow" discussion starting on 17 Feb 2016. As a result we introduced the "noTimezone" notion: a date without timezone compares equal to the same date in any time zone. At some point this made all the tests green. - Bert - On Wed, Oct 17, 2018 at 8:53 PM Chris Cunningham <[hidden email]> wrote:
|
|
|
On Thu, Oct 18, 2018 at 10:27 AM Bert Freudenberg <[hidden email]> wrote:
True. But we were missing some tests (we should have a test the check everywhere we defined #= to validate that equal objects also have equal #hash). As a consequence of that change, the following also happens: dates := { Date today. DateAndTime now asDate }. dates first = dates second. "=true" dates asSet size. "=2" Also, using dates as keys in Dictionaries might not work correctly, either. We could fix these issues by altering #hash so that equal dates hash equally - say, hashing ticks and maybe duration: hash | ticks | ticks := start ticks. ^((ticks second // 86400 + ticks first) hashMultiply bitXor: ticks second \\ 86400 bitXor: ticks third) + duration hash but explicitly ignoring offset. This would "work" such that any date with the same starting time (ignoring offset) would have the same hash. The Set example above would resolve down to 1, but dates with different offsets would still be present in the Set (since that relies on #=, which would be different in this case). However, if we used Dates as a key in Dictionary, then the different Dates with offsets will work fine, but the Date without offset will map to the first date key in the chain for the same hash key, but not the others. This invokes weird behaviour like: |dict| dict := Dictionary new: 3. dict at: (DateAndTime now offset: 3 hours) asDate put: '3 hours'; at: (DateAndTime now offset: -6 hours) asDate put: '-6 hours'. dict at: Date today. "=> '3 hours' " dict removeKey: (DateAndTime now offset: 3 hours) asDate. dict at: Date today. "=> '-6 hours' " Not ideal. (Of course, this is how Dates behave for us today as well. Even weirder if you put the Date today in as a key first - unpredictable results ensue.) That's why I like this change - I can't think of a better one. Except maybe having 2 Date classes like Richard is suggesting. -cbc
|
Re: The Inbox: Chronology-Core-cmm.14.mcz
|
|
In reply to this post by Bert Freudenberg
> I'm not sure this is right.
I assure you, it is right. This is how Chronology has always worked from the very beginning. To jog your memory, have a look at versions of Timespan class>>#defaultOffset, and see the change you made to the comment. If that doesn't help, more background below... ___________ Background: We had a discussion similar to the "Elephant in the room" thread going on Pharo-Users way back in 2011. As a consequence, I introduced #makeUTC in order to have "globalized Dates", ones that were sure to compare equal with other Dates regardless of which timezone they were created. All it did was set the 'start's offset to 0:00:00 for all instances I created, so that it would eliminate the timezone from being a factor when comparing dates (while vastly improving hashing performance). To implement that, a #defaultOffset was introduced and used when creating instances of Date, Month and Year. The old #starting: constructor is still there for the original (TZ-specific) Dates, but the default offset to use became defined by #defaultOffset, which was 0:00:00. The #noTimezone was borne from you wanting to make the fact that "we don't want no stinkin' timezone in our Date" more explicit. Instead of a defaultOffset of 0:00:00, you changed it to nil and then put nil checks in all the appropriate places. That way, there is no doubt that those are globalized Dates | Months | Years. We did not do anything else. The original Timespan behavior, which DOES have valid TZ-specific use-cases, is still available via #starting: and always was. There is no need to remove it since we have what we need in TZ-independent Dates via #defaultOffset. > The current implementation comes from the "trunk thinks its tomorrow" discussion starting on 17 Feb 2016. As a result we introduced the "noTimezone" notion: a date without timezone compares equal to the same date in any time zone. At some point this made all the tests green. Actually, that thread is about the bug that got introduced when the new utcMicroseconds thingy went in. It is totally unrelated to this... - Chris > > - Bert - > > On Wed, Oct 17, 2018 at 8:53 PM Chris Cunningham <[hidden email]> wrote: >> >> I like this. >> >> Although I notice that the comparison makes sure that the classes are the same - so a Timespan starting at the exact same time as a Date and with 24 hour duration is not the same as a Date with the exact same values. >> >> This would allow further speedups if Date, Week, Month, and Year if we wanted - just drop the comparison of duration in those classes. (The general case is still needed to tell Timespan apart from Schedule.) >> >> If you did that, you probably also want to re-implement hash as well - simplifying it the same way. >> >> -cbc >> >> On Wed, Oct 17, 2018 at 8:07 PM <[hidden email]> wrote: >>> >>> Chris Muller uploaded a new version of Chronology-Core to project The Inbox: >>> http://source.squeak.org/inbox/Chronology-Core-cmm.14.mcz >>> >>> ==================== Summary ==================== >>> >>> Name: Chronology-Core-cmm.14 >>> Author: cmm >>> Time: 17 October 2018, 10:07:14.485014 pm >>> UUID: 5d93900c-4a4e-4a4e-80f3-800dbdb07d0b >>> Ancestors: Chronology-Core-tcj.12 >>> >>> - A fix and optimization of Timespan>>#=. Both elements being compared must have the same timezone (or same state of #noTimezone) in order to take advantage of the optimized #hasEqualTicks: comparison. Otherwise (if different timezones), a full comparison of their starts (via #=) is needed. >>> - There was a mention of this optimization put into the class comment. This level of detail may be a bit tedious for users to read at that level, so Brents original comment was restored. >>> >>> =============== Diff against Chronology-Core-tcj.12 =============== >>> >>> Item was changed: >>> Magnitude subclass: #Timespan >>> instanceVariableNames: 'start duration' >>> classVariableNames: '' >>> poolDictionaries: '' >>> category: 'Chronology-Core'! >>> >>> + !Timespan commentStamp: 'cmm 10/17/2018 22:00' prior: 0! >>> + I represent a duration starting on a specific DateAndTime.! >>> - !Timespan commentStamp: 'bf 2/18/2016 14:43' prior: 0! >>> - I represent a duration starting on a specific DateAndTime. >>> - >>> - If my start has an offset identical to my #defaultOffset then comparisons ignore timezone offset.! >>> >>> Item was changed: >>> ----- Method: Timespan>>= (in category 'ansi protocol') ----- >>> = comparand >>> + ^ self class = comparand class >>> + and: [(((self noTimezone and: [comparand noTimezone]) or: [self start offset = comparand start offset]) >>> + ifTrue: [ self start hasEqualTicks: comparand start ] >>> + ifFalse: [ self start = comparand start ]) >>> + and: [ self duration = comparand duration ] ] >>> - ^ self class = comparand class >>> - and: [((self noTimezone or: [ comparand noTimezone ]) >>> - ifTrue: [ self start hasEqualTicks: comparand start ] >>> - ifFalse: [ self start = comparand start ]) >>> - and: [ self duration = comparand duration ] ] >>> .! >>> >>> >> > |
Re: The Inbox: Chronology-Core-cmm.14.mcz
|
|
In reply to this post by cbc
Hi Chris,
> True. But we were missing some tests (we should have a test the check everywhere we defined #= to validate that equal objects also have equal #hash). > As a consequence of that change, the following also happens: > dates := { Date today. DateAndTime now asDate }. > dates first = dates second. "=true" > dates asSet size. "=2" These are the correct answers. Have a look at the comment of Timespan class>>#defaultOffset for an explanation of this. Best, Chris |
Re: The Inbox: Chronology-Core-cmm.14.mcz
|
|
> > True. But we were missing some tests (we should have a test the check everywhere we defined #= to validate that equal objects also have equal #hash).
> > As a consequence of that change, the following also happens: > > dates := { Date today. DateAndTime now asDate }. > > dates first = dates second. "=true" > > dates asSet size. "=2" > > These are the correct answers. Have a look at the comment of Timespan > class>>#defaultOffset for an explanation of this. Sorry, ignore that. You were talking about equality vs. hashing. |
Re: The Inbox: Chronology-Core-cmm.14.mcz
|
|
In reply to this post by Bert Freudenberg
Hey Bert,
I just noticed what you added to the very end of that comment in Timespan class>>#defaultOffset, where you added: "It can only compare equally to Dates of its time-zone or Dates without timezone." I didn't know (or remember) that's what you were trying to do until just now! It seems like a cool idea, but it seems impossible due to the #hash vs. #= problem. Hmmm... I think apps will have to send #beCanonical if they want canonical Dates from DateAndTime's... - Chris On Thu, Oct 18, 2018 at 12:27 PM Bert Freudenberg <[hidden email]> wrote: > > I'm not sure this is right. > > The current implementation comes from the "trunk thinks its tomorrow" discussion starting on 17 Feb 2016. As a result we introduced the "noTimezone" notion: a date without timezone compares equal to the same date in any time zone. At some point this made all the tests green. > > - Bert - > > On Wed, Oct 17, 2018 at 8:53 PM Chris Cunningham <[hidden email]> wrote: >> >> I like this. >> >> Although I notice that the comparison makes sure that the classes are the same - so a Timespan starting at the exact same time as a Date and with 24 hour duration is not the same as a Date with the exact same values. >> >> This would allow further speedups if Date, Week, Month, and Year if we wanted - just drop the comparison of duration in those classes. (The general case is still needed to tell Timespan apart from Schedule.) >> >> If you did that, you probably also want to re-implement hash as well - simplifying it the same way. >> >> -cbc >> >> On Wed, Oct 17, 2018 at 8:07 PM <[hidden email]> wrote: >>> >>> Chris Muller uploaded a new version of Chronology-Core to project The Inbox: >>> http://source.squeak.org/inbox/Chronology-Core-cmm.14.mcz >>> >>> ==================== Summary ==================== >>> >>> Name: Chronology-Core-cmm.14 >>> Author: cmm >>> Time: 17 October 2018, 10:07:14.485014 pm >>> UUID: 5d93900c-4a4e-4a4e-80f3-800dbdb07d0b >>> Ancestors: Chronology-Core-tcj.12 >>> >>> - A fix and optimization of Timespan>>#=. Both elements being compared must have the same timezone (or same state of #noTimezone) in order to take advantage of the optimized #hasEqualTicks: comparison. Otherwise (if different timezones), a full comparison of their starts (via #=) is needed. >>> - There was a mention of this optimization put into the class comment. This level of detail may be a bit tedious for users to read at that level, so Brents original comment was restored. >>> >>> =============== Diff against Chronology-Core-tcj.12 =============== >>> >>> Item was changed: >>> Magnitude subclass: #Timespan >>> instanceVariableNames: 'start duration' >>> classVariableNames: '' >>> poolDictionaries: '' >>> category: 'Chronology-Core'! >>> >>> + !Timespan commentStamp: 'cmm 10/17/2018 22:00' prior: 0! >>> + I represent a duration starting on a specific DateAndTime.! >>> - !Timespan commentStamp: 'bf 2/18/2016 14:43' prior: 0! >>> - I represent a duration starting on a specific DateAndTime. >>> - >>> - If my start has an offset identical to my #defaultOffset then comparisons ignore timezone offset.! >>> >>> Item was changed: >>> ----- Method: Timespan>>= (in category 'ansi protocol') ----- >>> = comparand >>> + ^ self class = comparand class >>> + and: [(((self noTimezone and: [comparand noTimezone]) or: [self start offset = comparand start offset]) >>> + ifTrue: [ self start hasEqualTicks: comparand start ] >>> + ifFalse: [ self start = comparand start ]) >>> + and: [ self duration = comparand duration ] ] >>> - ^ self class = comparand class >>> - and: [((self noTimezone or: [ comparand noTimezone ]) >>> - ifTrue: [ self start hasEqualTicks: comparand start ] >>> - ifFalse: [ self start = comparand start ]) >>> - and: [ self duration = comparand duration ] ] >>> .! >>> >>> >> > |
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |