Schedule actions at a given time

Adding functions enabling an effect to run only at a given point in time.
The functions exec and await call each other to establish when an effect must run.
The exec function calls foldM to execute the effect and retrieve its result. At this point, it gives control to await
The await function awaits when schedule signal when it's ready to run. Schedule will always receive. Schedules for this always receive unit as input because they don't need effect to run to decide when to run
When schedule is ready to run, await return control to exec, restarting the process

Two points on this PR:

i've not added unit tests because i never wrote specs before. I would really appreciate some help to write the tests.
when building microsite, i got a failure related to jekyll. Was it my fault?

@alexandredantas I think this is a useful feature to have.
However, I think we can do it in a slightly different way that has a cleaner design: adding a method into Scheduler that allows scheduling something at a particular instant.
Then, potentially, every step could return either a delay (relative time) or an instant (absolute time). I have to think this through, but I think this has all the structure required of Schedule and the existing combinators.
Do you think that would work?

@jdegoes make total sense. convert the point at time to a delay and effects would sleep until reaching it. i'll rework on that. thanks for the tip!
ps: can you help pointing out which properties i should test? :)

@alexandredantas The main thing we need to ensure is that invariants around && and || are respected. For example: semantics of && are to choose the larger of the two delays; and ||, to choose the smaller of the two delays.
Things will be easier if we introduce a type:
sealed trait Delay
object Delay {
  final case class Relative(duration: Duration)
  final case class Absolute(time: Instant) // Cannot use Instant for Scala.js???
}

Now our problem is comparing them to see which one comes first requires an effect, because we have to use the Clock, so our comparison methods will change:
sealed trait Delay {
  def < (that: Delay): ZIO[Clock, Nothing, Boolean] = ???
  def <= (that: Delay): ZIO[Clock, Nothing, Boolean] = ???
  def > (that: Delay): ZIO[Clock, Nothing, Boolean] = ???
  def >= (that: Delay): ZIO[Clock, Nothing, Boolean] = ???
  def === (that: Delay): ZIO[Clock, Nothing, Boolean] = ???
}

Direct comparison on Duration happens for && and || methods. It's not desirable to make them do the comparison (instead we should push that all the way to ZIO#retry / ZIO#repeat). So we have to change that by tweaking combineWith, using some new data structure, which can describe comparisons without actually making them:
sealed trait DelayComparison { self =>
  def choose: ZIO[Clock, Nothing, Delay] =
    self match {
      ??? // Uses effectful comparison methods on `Delay`
    }
}
object DelayComparison {
  final case class Choose(value: Delay) extends DelayComparison
  final case class Min(l: DelayComparison, r: DelayComparison) extends DelayComparison
  final case class Max(l: DelayComparison, r: DelayComparison) extends DelayComparison
}

Now combineWith will not take a function (Duration, Duration) => Duration, but rather, a (Delay, Delay) => DelayComparison. So now Clock dependency (pulled in by comparing Delay) will not contaminate the Schedule methods, but rather be pushed into ZIO#retry / ZIO#repeat, which will call the DelayComparison#choose method.
At this point all existing schedules will be updated to use Delay.Relative (with helper: Delay.relative), since that's what they use now. And some new schedules can use Delay.Absolute, e.g. Schedule.at(...).

@jdegoes Wow!! Such a very nice solution!! I'll come up with something following your suggestion

@jdegoes can you tell me if I'm on the right direction? When I changed Decision.combineWith to (Delay, Delay) => DelayComparison, it was required to touch every other schedule implementation.
Besides, combineWith take Direction.delay as arguments and I have changed it to be DelayComparison, not allowing to apply to the function :(
What did I do wrong?

@jdegoes After banging my head against wall sometimes, I've came up with this implementation. Adjusted tests accordingly and at least build is passing :smile:
If things looks good, I want to place a schedule capable of considering only a point in time. However, scalajs doesn't support LocalDateTime. Is there something else I could use to perform date calculations?

@alexandredantas After looking at some of the contortions you had to go through, I can see now what I should have seen all alone: Delay and DelayComparison need to be fused into a single structure, called Delay.
sealed trait Delay {
  def run: ZIO[Clock, Nothing, Duration, Instant]
}
object Delay {
  val none: Delay = Relative(Duration.zero)
  // Etc.

  final case class Relative(duration: Duration) extends Delay
  final case class Absolute(instant: Instant) extends Delay // Scala.js compat?
  final case class Min(left: Delay, right: Delay) extends Delay
  final case class Max(left: Delay, right: Delay) extends Delay
|

Now functions can be (Delay, Delay) => Delay and composition will be vastly superior. Should clean up a lot of code, too!

@jdegoes :smile: Sorry for noobiness. I should have tried that first instead implementing them separately. I'll clean up things and do as you suggested. After all, do things look good? Do you see any major mistake?

@alexandredantas I think a lot of the gnarlier code will be clean after this change. The overall direction of changes seems correct to me. There may be some edge case lodging that needs more careful review when the bigger changes have been made, but you're definitely on the right track! 😄

@alexandredantas Let me know if you need a hand. 😄

@jdegoes Tks!! :smile: I wasn't able to work on this in past days but I'll get back soon

@jdegoes Finished fusing DelayComparison and Delay. Code is a lot cleaner now. It allowed to compute delay directly instead returning a tuple as result of schedule effect, decoupling implementations to know about relative and absolute semantics: just need to sleep for the given duration. This design actually is a lot better, preventing clock drifts and making cpu time optimized.
Now I'm solving some issues with dotty inside schedule, ok?

@jdegoes Instant, LocalDate and LocalTime are suported in scalajs through this library: https://github.com/scala-js/scala-js-java-time
So I don't think is a big deal using these classes for now. What do you think?

@jdegoes I'm still on this PR. Just got some issues on my job but definitely will fix your requested changes

@alexandredantas No worries, let me know if you need any help!

@jdegoes Applied your changes the best I could. I didn't write a test for schedule yet because never wrote a property based test before. Can you give me some tips on how to do that?

@alexandredantas This is great work! Looking forward to having this capability in core.

@AL333Z This is looking great to me!
My only outstanding concern is if there is a way to preserve backward compatibility (right now, users will have to do xxx.relative wherever they were using xxx; with a type class, we may be able to eliminate that). Give me a bit and I'll come back with an idea here.

users will have to do xxx.relative wherever they were using xxx

also 100.ms.absolute will never make sense, will it? it would be super nice to have a panoply of combinators for future points in time: tomorrow.at(6.am), monday.in(2.week), etc. 😃

users will have to do xxx.relative wherever they were using xxx

also 100.ms.absolute will never make sense, will it? it would be super nice to have a panoply of combinators for future points in time: tomorrow.at(6.am), monday.in(2.week), etc.

@regiskuckaertz you're right: duration.absolute doesn't make sense.  I'm intending to release some of them in this PR but not a full-feature commbination. What do you think?

some smart construction as you proposed, allowing rich combinations.

I think YASSS! After re-reading, note that all my examples are relative too 😭 I still believe they'd be useful (I've been playing with a zio-cron library over schedules), the difficulty is to come up with a vocabulary that makes sense and is composable. But for absolute points in time we might as well ... parse strings, as in: absolute"2021-01-25".

some smart construction as you proposed, allowing rich combinations.

I think YASSS! After re-reading, note that all my examples are relative too  I still believe they'd be useful (I've been playing with a zio-cron library over schedules), the difficulty is to come up with a vocabulary that makes sense and is composable. But for absolute points in time we might as well ... parse strings, as in: absolute"2021-01-25".

I think your examples are very good actually. I see them as absolute because they point to a future moment. tomorrow.at(6.pm) for example can be seen as an absolute point in future running only once.
Every absolute point in time can be seen as relative but the structure itself is abstracting delay calculation from programmers :smiley:
And another upside of introducing it: I was trying to find an way to always create Absolute using milliseconds representation. Using duration is confusing and using long can be anything. Introducing some expression structure allows creating absolute points in time hiding complexity from users.
@jdegoes do you think this is the DSL you said that could be hiding on everyDay schedule?

@jdegoes do you think this is the DSL you said that could be hiding on everyDay schedule?

I think the core of the DSL involves adding a apply method to Schedule, which takes an inner Schedule, with the following semantic:
The outer schedule runs the inner schedule until completion, and then once completed, it takes a step itself, and on the next step, it resets the inner schedule (using its starting state) and runs the inner schedule again until completion, and so forth, until the outer schedule is done.
Then if we have an every method, I think we can do things like:
every(Monday) {
  every(Hour) {
    every(Minute)
  }
|

Then if we have an after method, we can combine them:
every(Monday) {
  after(6.pm) {
    every(10.minutes)
  }
}

The difference between every and after would be every keeps doing it on the specified relative schedule, whereas after would do it once after the specified elapsed interval.
Maybe others are necessary too, I am not sure, and maybe .apply is not a good name.

@jdegoes do you think this is the DSL you said that could be hiding on everyDay schedule?

I think the core of the DSL involves adding a apply method to Schedule, which takes an inner Schedule, with the following semantic:
The outer schedule runs the inner schedule until completion, and then once completed, it takes a step itself, and on the next step, it resets the inner schedule (using its starting state) and runs the inner schedule again until completion, and so forth, until the outer schedule is done.
Then if we have an every method, I think we can do things like:
every(Monday) {
  every(Hour) {
    every(Minute)
  }
|

Then if we have an after method, we can combine them:
every(Monday) {
  after(6.pm) {
    every(10.minutes)
  }
}

The difference between every and after would be every keeps doing it on the specified relative schedule, whereas after would do it once after the specified elapsed interval.
Maybe others are necessary too, I am not sure, and maybe .apply is not a good name.

But if inner schedule runs until completion, some combinations may runs over outer constraints, for example:
every(monday){
every(minute){
...
}
}
Inner schedule will start running monday but will continue to tuesday, wednesday, etc. Maybe outer schedules should take a step itself, delaying execution if required, then passing control to inner schedule. What do you think?

I'm placing a very initial draft of schedule expressions to be used as smart constructors for points in time. Please, @regiskuckaertz  and @jdegoes give some tips if i'm going on right direction. My main idea was constraining Delay.absolute constructor to use always an expression, avoiding misuse. Don't know if it's the right way to go

Inner schedule will start running monday but will continue to tuesday, wednesday, etc.

Probably because minute is not specific enough and there needs to be minuteOfTheDay, minuteOfTheHour. Then it is clear the schedule will eventually stop.

Probably because minute is not specific enough and there needs to be minuteOfTheDay, minuteOfTheHour. Then it is clear the schedule will eventually stop.

... on second thought, the output of every { every { ... } } should probably be that of all underlying schedules &&-ed. Then the problem disappears as Monday will stop as soon as it's Tuesday.

Probably because minute is not specific enough and there needs to be minuteOfTheDay, minuteOfTheHour. Then it is clear the schedule will eventually stop.

... on second thought, the output of every { every { ... } } should probably be that of all underlying schedules &&-ed. Then the problem disappears as Monday will stop as soon as it's Tuesday.

I was thinking exactly about that. Nested every can be represented as schedule composition, using &&, || and other operators. This way we don't need new structures and can build complex scheduling logic. I think some new constructors like every and after can help to hide delay calculation from programmers

I'm placing a very initial draft of schedule expressions to be used as smart constructors for points in time. Please, @regiskuckaertz and @jdegoes give some tips if i'm going on right direction. My main idea was constraining Delay.absolute constructor to use always an expression, avoiding misuse. Don't know if it's the right way to go

Personally, I'd defer "schedule expressions" for a followup PR, because it would be nice to get this one merged in, and then take a little more time to see if / what we need for schedule expressions. We need a list of use cases to solve, e.g.:

create a schedule that repeats the fourth Monday every month, and the second Tuesday, and on the second Tuesday, it repeats every 3 hours, at 15 minutes, 30 minutes, and 50 minutes past the hour mark.

This can motivate the combinators and inform the design.

I think what we miss right now (this is just a guess) is a combinator for: AFTER this schedule steps, THEN run this next schedule until it's done, then continue with the original schedule.
I can't see a way to do that from existing combinators. There's some nesting, it's like a nested for loop:
for (each step in top schedule) {
  for (each step in lower schedule) {
    ...
  }
}

I'm placing a very initial draft of schedule expressions to be used as smart constructors for points in time. Please, @regiskuckaertz and @jdegoes give some tips if i'm going on right direction. My main idea was constraining Delay.absolute constructor to use always an expression, avoiding misuse. Don't know if it's the right way to go

Personally, I'd defer "schedule expressions" for a followup PR, because it would be nice to get this one merged in, and then take a little more time to see if / what we need for schedule expressions. We need a list of use cases to solve, e.g.:

create a schedule that repeats the fourth Monday every month, and the second Tuesday, and on the second Tuesday, it repeats every 3 hours, at 15 minutes, 30 minutes, and 50 minutes past the hour mark.

This can motivate the combinators and inform the design.

I'd like to introduce at least EveryDay expression. It will help build Absolute delay without relying on Duration or a Long value. Does it make sense?

I'd like to introduce at least EveryDay expression. It will help build Absolute delay without relying on Duration or a Long value. Does it make sense?

I feel that EveryDay is a combination of three schedules:

A daily repetition
Appended to or nested within that, a single (non-repeating) delay of hour hours
Appended to or nested with that, a single (non-repeating) delay of minute minutes

We don't have a way to fuse durations in this way, or to do the nesting (although nesting is not required here, I don't think, because the latter two don't repeat in this specific example).
Actually that's not quite true, we have >>>, which may be able to do simple things:
(day(1) >>> hour(3) >>> minute(3)).forever

I think we're missing schedule1.repeat(schedule2) (which would allow implementation of .forever as schedule1.repeat(Schedule.forever), and maybe that's all.
We're close but I'd hate to ship EveryDay if we can break it down into little pieces...

I'd like to introduce at least EveryDay expression. It will help build Absolute delay without relying on Duration or a Long value. Does it make sense?

I feel that EveryDay is a combination of three schedules:

A daily repetition
Appended to or nested within that, a single (non-repeating) delay of hour hours
Appended to or nested with that, a single (non-repeating) delay of minute minutes

We don't have a way to fuse durations in this way, or to do the nesting (although nesting is not required here, I don't think, because the latter two don't repeat in this specific example).
Actually that's not quite true, we have >>>, which may be able to do simple things:
(day(1) >>> hour(3) >>> minute(3)).forever

I think we're missing schedule1.repeat(schedule2) (which would allow implementation of .forever as schedule1.repeat(Schedule.forever), and maybe that's all.
We're close but I'd hate to ship EveryDay if we can break it down into little pieces...

Ok. So for now I'll continue to build Absolute using a long representing epoch in milliseconds.

About the composition of EveryDay, I totally agree. I just don't know how to implement it for now 😃
Maybe I should leave it for another PR. What do you think?

Yes, let's do that! Excited to get this one in for the next RC.

While you are wrapping this up, I thought of something for retry:
Retry will invoke the schedule in case of a failure. It may make sense to invoke the schedule right away, and add the initial delay with whatever is returned by the schedule.
In addition, we should make sure that the initial delays compose using && and || and composeWith, appropriately. Just like the "step" delays are combined, the initial delays should be combined in the same way.
Finally, we should make the stream repetition semantics match the ordinary one.
Let me know if this makes sense or if you disagree with any of this.

I agree. Actually, I think this will be the base for that schedule dsl we've discussed. every and at could be operators like && and ||, allowing combining initial delays and evaluating if schedules should start.
I'll push this change soon

@jdegoes "Retry will invoke the schedule in case of a failure. It may make sense to invoke the schedule right away, and add the initial delay with whatever is returned by the schedule."
About this: schedule will return return delay for next step. Does it make sense to add initial delay to step delay?

About this: schedule will return return delay for next step. Does it make sense to add initial delay to step delay?

It seems like we either add them, or throw away the initial delay. It doesn't make sense (in the case of retry) to invoke the action, get a failure, wait the initial delay, pass the error to the schedule, and then get the next delay, and wait that. It will introduce a spurious wait between the time of the failure, and the time when the schedule knows about the failure.
Does that make sense? What do you think?

Initial delay is being considered before executing effect for the first time (before foldM). After that, I think delay logic responsability should be passed to update only, as it is currently. I don't know if make sense adding initial delay here as we already have update calculating the next delay. Maybe we could pass initial delay to update so it could be taken in. What do you think?

Initial delay is being considered before executing effect for the first time (before foldM). After that, I think delay logic responsability should be passed to update only, as it is currently.

I think the advantage of doing it this way is that it's consistent with repeat.
In practice, I don't think user will want to wait before executing an effect, if they only want to retry it if it fails. They may be surprised to change code like makeRequest into makeRequest.retry(schedule), and then find out the request doesn't happen right away, because of an initial delay.
However, there's another way to solve that: just making sure only the date/time combinators use the initial delay. It can be 0 for all others. In fact, before we sleep, we should check, and if it's 0, we should skip the sleep (that will result in less latency than the current approach).
Do you think that could work?

Initial delay is being considered before executing effect for the first time (before foldM). After that, I think delay logic responsability should be passed to update only, as it is currently.

I think the advantage of doing it this way is that it's consistent with repeat.
In practice, I don't think user will want to wait before executing an effect, if they only want to retry it if it fails. They may be surprised to change code like makeRequest into makeRequest.retry(schedule), and then find out the request doesn't happen right away, because of an initial delay.
However, there's another way to solve that: just making sure only the date/time combinators use the initial delay. It can be 0 for all others. In fact, before we sleep, we should check, and if it's 0, we should skip the sleep (that will result in less latency than the current approach).
Do you think that could work?

About retry: i think you're right.
I didn't get the part of date/time combinators using initial delay. Do you mean only considering initial delay on Schedule.update?

@alexandredantas Do you want to have a Google Hangout session? I think it would help us to wrap up the final issues.

@alexandredantas Do you want to have a Google Hangout session? I think it would help us to wrap up the final issues.

Sounds great!! I'd just finished merging my code with organization changes.

@jdegoes when do you want to schedule the hangout session?

@jdegoes any update? :)

@alexandredantas How about now? 😄
Did you get everything passing???

@jdegoes sorry for not getting the message early. what about friday? anytime :)
About the tests, yes, all passing!! I've merged with master and looks like something was fixed

This work is being continued in #1798. @alexandredantas Thank you for all your help on this and if you have some time to contribute, your assistance reviewing #1798 and ensuring we satisfy all requirements of the original ticket would be greatly appreciated. 🙏


Also remove --cidfile logic for run_tests.py

CC @nicolasnoble

LGTM


CV/AICity2020-Anomaly-Detection/mask_code/mask_track.py tries to open a non-existent path

Hey @liyingying0113, this is trying to open a non-existent path. I haven't seen any code to make that path within the CV/AICity2020-Anomaly-Detection
https://github.com/PaddlePaddle/Research/blob/224ba67267b2875c46e94a76a162352d30457f72/CV/AICity2020-Anomaly-Detection/mask_code/mask_track.py#L18
@AoZhang does this have a code to generate those files before this module?

@godfanmiao @milkfish1988

Hi, I think that the missing directory is using deepSORT algorithm to find the vehicle's trajectory but I have no idea how to generate the .txt file. Have you found out the way?

Hi, I think that the missing directory is using deepSORT algorithm to find the vehicle's trajectory but I have no idea how to generate the .txt file. Have you found out the way?

I haven't found out the way.