Strange behavior with 24-hour work days #138

jimryan · 2019-01-11T03:32:34Z

I'm using biz on a project that doesn't really have a concept of shifts or working hours (just work days), and for the most part, schedules in full days. As a result, we work mostly with midnights on the start of a given work day. I'm running into a strange issue when trying to find the work day before a given midnight, when the returned time should be midnight on a Monday. Check out these examples:

Biz.configure do |config|
  config.hours = {
    mon: {'00:00' => '24:00'},
    tue: {'00:00' => '24:00'},
    wed: {'00:00' => '24:00'},
    thu: {'00:00' => '24:00'},
    fri: {'00:00' => '24:00'}
  }
end

monday = Time.utc(2019, 1, 14)

Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC
Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC
Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC
Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19
Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct

tuesday = Time.utc(2019, 1, 15)

Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC
Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct
Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC

All is well up until the returned time should be midnight on a Monday, at which point biz returns midnight on the prior Saturday. When we want 5 work days before midnight on Monday 1/14/19, we get midnight on Saturday 1/5/19 (a non work day) instead of midnight on Monday 1/7/19. Similarly, when we want 0 work days before midnight on Monday, 1/14/19 (i.e. itself), we get midnight on Saturday, 1/12/19. This happens when starting with any day, not just Mondays, which is why I included some examples using a Tuesday date.

Is this a bug or am I missing something? The obvious workaround is to avoid midnights, but that feels hacky to me. Any insight would be much appreciated. Thanks!

The text was updated successfully, but these errors were encountered:

jimryan · 2019-01-11T17:31:29Z

Just a quick update: To remove potential issues with full-day work days from the equation, I configured my work days as 9AM - 5PM, and am seeing the same issue. I recognize that "0 days before" is kind of an edge case, and may or may not be supported, but I do think there's a bug here when ">0 days before" doesn't return the expected result.

I think the issue lies specifically with Times that start exactly on the beginning of a work day.

craiglittle · 2019-01-11T18:48:37Z

Thanks for the report, @jimryan!

You're right that day calculations can be a bit ambiguous and squirrelly. It's a feature I was reluctant to support for those reasons, but I have an inkling of what's going on here. Let me dig into it a bit and think about how best to proceed.

jimryan · 2019-01-11T19:09:16Z

Thanks, @craiglittle! Let me know how I can help.

jimryan · 2019-01-12T05:00:33Z

@craiglittle While you're looking into this, I've stumbled upon some more strange behavior. Given the same above setup:

friday = Time.utc(2019, 1, 11)
Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected
Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct

Before, a lack of handling of time segment endpoints in calculations consistent with how they are handled elsewhere in the gem (that is, *including* the start time and *excluding* the end time) was causing weird behavior and inconsistencies across both `#for` and `#until` period generation methods, which filtered down to high-level calculations. When looking forward in time, if consumes, or lands at the end of a period, one extra "moment" period should be generated to compensate for the fact that the end time of a time segment is not considered part of that segment. Under similar circumstances when looking backward in time, since the start time of a time segment is considered inclusive to that time segment, calculations that end on or at that moment should not proceed to generate the next moment period. A nice side effect of making this behavior consistent at the lowest level of the engine is that we can remove special-case handling that was added to the high-level calculation object in support of zero-scalar calculations. We can simply use the period and timeline objects normally and get the correct behavior. Woohoo! Running the benchmark suite, we don't see a meaningful delta in performance characteristics with these changes. Before: ``` Biz.configure do |config| config.hours = { mon: {'00:00' => '24:00'}, tue: {'00:00' => '24:00'}, wed: {'00:00' => '24:00'}, thu: {'00:00' => '24:00'}, fri: {'00:00' => '24:00'} } end monday = Time.utc(2019, 1, 14) Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19 Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct tuesday = Time.utc(2019, 1, 15) Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC friday = Time.utc(2019, 1, 11) Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct ``` After: ``` >> Biz.configure do |config| ?> config.hours = { ?> mon: {'00:00' => '24:00'}, ?> tue: {'00:00' => '24:00'}, ?> wed: {'00:00' => '24:00'}, ?> thu: {'00:00' => '24:00'}, ?> fri: {'00:00' => '24:00'} >> } >> end >> >> monday = Time.utc(2019, 1, 14) => 2019-01-14 00:00:00 UTC >> >> Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC => 2019-01-11 00:00:00 UTC >> Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC => 2019-01-10 00:00:00 UTC >> Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC => 2019-01-08 00:00:00 UTC >> Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19 => 2019-01-07 00:00:00 UTC >> Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 => 2019-01-14 00:00:00 UTC >> Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct => 2019-01-14 00:00:01 UTC >> >> tuesday = Time.utc(2019, 1, 15) => 2019-01-15 00:00:00 UTC >> >> Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC => 2019-01-15 00:00:00 UTC >> Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 => 2019-01-14 00:00:00 UTC >> Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct => 2019-01-14 00:00:01 UTC >> Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC => 2019-01-08 00:00:00 UTC >> friday = Time.utc(2019, 1, 11) => 2019-01-11 00:00:00 UTC >> Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected => 2019-01-14 00:00:00 UTC >> Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct => 2019-01-14 00:00:00 UTC ``` Fixes #138.

craiglittle · 2019-01-13T03:29:18Z

@jimryan A fix is on the way! #139

Long story short, I needed to modify the underlying engine to ensure it was handling period endpoints consistently across the board.

With that said, given your use case (not interested in business hours, just days), you might want to check out biz's sister gem clavius; it's built expressly for that purpose.

Here's an example:

Clavius.configure do |c|
  c.weekdays = %i[mon tue wed thu fri]
end

>> monday = Date.new(2019, 1, 14)
=> #<Date: 2019-01-14 ((2458498j,0s,0n),+0s,2299161j)>
>> Clavius.days(1).before(monday)
=> #<Date: 2019-01-11 ((2458495j,0s,0n),+0s,2299161j)>
>> Clavius.days(2).before(monday)
=> #<Date: 2019-01-10 ((2458494j,0s,0n),+0s,2299161j)>
>> Clavius.days(4).before(monday)
=> #<Date: 2019-01-08 ((2458492j,0s,0n),+0s,2299161j)>
>> Clavius.days(5).before(monday)
=> #<Date: 2019-01-07 ((2458491j,0s,0n),+0s,2299161j)>
>> Clavius.days(0).before(monday)
=> #<Date: 2019-01-14 ((2458498j,0s,0n),+0s,2299161j)>

If you prefer to stick with biz, this functionality is accessible from there via the dates method on a schedule (or the Biz constant):

>> Biz.dates.days(1).after(monday)
=> #<Date: 2019-01-15 ((2458499j,0s,0n),+0s,2299161j)>

Hope that's helpful! I expect to get the fix merged and released by early next week.

Before, a lack of handling of time segment endpoints in calculations consistent with how they are handled elsewhere in the gem (that is, *including* the start time and *excluding* the end time) was causing weird behavior and inconsistencies across both `#for` and `#until` period generation methods, which filtered down to high-level calculations. When looking forward in time, if consumes, or lands at the end of a period, one extra "moment" period should be generated to compensate for the fact that the end time of a time segment is not considered part of that segment. Under similar circumstances when looking backward in time, since the start time of a time segment is considered inclusive to that time segment, calculations that end on or at that moment should not proceed to generate the next moment period. A nice side effect of making this behavior consistent at the lowest level of the engine is that we can remove special-case handling that was added to the high-level calculation object in support of zero-scalar calculations. We can simply use the period and timeline objects normally and get the correct behavior. Woohoo! Running the benchmark suite, we don't see a meaningful delta in performance characteristics with these changes. Before: ``` Biz.configure do |config| config.hours = { mon: {'00:00' => '24:00'}, tue: {'00:00' => '24:00'}, wed: {'00:00' => '24:00'}, thu: {'00:00' => '24:00'}, fri: {'00:00' => '24:00'} } end monday = Time.utc(2019, 1, 14) Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19 Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct tuesday = Time.utc(2019, 1, 15) Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC friday = Time.utc(2019, 1, 11) Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct ``` After: ``` >> Biz.configure do |config| ?> config.hours = { ?> mon: {'00:00' => '24:00'}, ?> tue: {'00:00' => '24:00'}, ?> wed: {'00:00' => '24:00'}, ?> thu: {'00:00' => '24:00'}, ?> fri: {'00:00' => '24:00'} >> } >> end >> >> monday = Time.utc(2019, 1, 14) => 2019-01-14 00:00:00 UTC >> >> Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC => 2019-01-11 00:00:00 UTC >> Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC => 2019-01-10 00:00:00 UTC >> Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC => 2019-01-08 00:00:00 UTC >> Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19 => 2019-01-07 00:00:00 UTC >> Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 => 2019-01-14 00:00:00 UTC >> Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct => 2019-01-14 00:00:01 UTC >> >> tuesday = Time.utc(2019, 1, 15) => 2019-01-15 00:00:00 UTC >> >> Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC => 2019-01-15 00:00:00 UTC >> Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19 => 2019-01-14 00:00:00 UTC >> Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct => 2019-01-14 00:00:01 UTC >> Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC => 2019-01-08 00:00:00 UTC >> friday = Time.utc(2019, 1, 11) => 2019-01-11 00:00:00 UTC >> Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected => 2019-01-14 00:00:00 UTC >> Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct => 2019-01-14 00:00:00 UTC ``` Fixes #138.

craiglittle · 2019-01-14T23:43:51Z

@jimryan This fix is released in v1.8.2. ✨

jimryan · 2019-01-15T18:22:46Z

Thank you so much for the ultra-fast fix, @craiglittle! And I wasn't aware of Clavius or the #dates method, so I'll check both out, as I agree they're a better fit since we don't care about time. Thanks again!

craiglittle mentioned this issue Jan 13, 2019

Treat endpoints consistently in calculations #139

Merged

craiglittle closed this as completed in #139 Jan 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strange behavior with 24-hour work days #138

Strange behavior with 24-hour work days #138

jimryan commented Jan 11, 2019

jimryan commented Jan 11, 2019 •

edited

craiglittle commented Jan 11, 2019

jimryan commented Jan 11, 2019

jimryan commented Jan 12, 2019 •

edited

craiglittle commented Jan 13, 2019

craiglittle commented Jan 14, 2019

jimryan commented Jan 15, 2019

Strange behavior with 24-hour work days #138

Strange behavior with 24-hour work days #138

Comments

jimryan commented Jan 11, 2019

jimryan commented Jan 11, 2019 • edited

craiglittle commented Jan 11, 2019

jimryan commented Jan 11, 2019

jimryan commented Jan 12, 2019 • edited

craiglittle commented Jan 13, 2019

craiglittle commented Jan 14, 2019

jimryan commented Jan 15, 2019

jimryan commented Jan 11, 2019 •

edited

jimryan commented Jan 12, 2019 •

edited