Resolve "Define time spans periodically for ResourceUnavailable"

[jbdyn]

This PR presents an additional ResourceConstraint basing class ResourcePeriodicallyUnavailable, which enables the user to give a list of time intervals in which a resource is unavailable and to repeat that pattern indefinitely. The periodicity, the offset as well as the range in which the pattern is repeated can be controlled by setting the respective parameters. Using modulo calculation, this adds only one solver assertion for one interval, regardless of the number of repetitions.

Fixes #141

[jbdyn]

What do you think about my implementation? :slightly_smiling_face:

I still need to add tests, though.

[codecov-commenter]

Codecov Report

Attention: 71 lines in your changes are missing coverage. Please review.

Comparison is base (247f8a5) 98.88% compared to head (69bca56) 97.75%. Report is 5 commits behind head on master.

:exclamation: Current head 69bca56 differs from pull request most recent head 7381eff. Consider uploading reports for the commit 7381eff to get more accurate results

Files	Patch %	Lines
processscheduler/resource_constraint.py	36.03%	71 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #142 +/- ## ========================================== - Coverage 98.88% 97.75% -1.14% ========================================== Files 42 43 +1 Lines 5478 5742 +264 ========================================== + Hits 5417 5613 +196 - Misses 61 129 +68 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[tpaviot]

@jbdyn that's great, thank you for this contribution! I'm amazed you went so fast, you actually have a very good understanding of the core concepts, congrats

I have two questions/comments:

• I'm not sure whether a new class should be created (as you did, ResourcePeriodicallyUnavailable), or add parameters to the existing ResourceUnvailable. For, instance, a periodic flag could be added to this class, by default set to False. This would perhaps avoid code duplication. That's something to balance: more classes simple classes with risks of code duplication, less classes with more parameters. As a user, your opinion ?

• in the test suite, the horizon is set for each case. What if the horizon is not set in the SchedulingProblem, did you test it still works?

If this class fits your needs, I would be in favor of merging as is and update the doc, we'll see later if a kind of refatoring is needed.

[jbdyn]

Thank you, I am very happy that you like it!

I definitely see your point. Both classes have quite similar logic implemented.
Personally, despite that, I would like to have an extra ResourcePeriodicallyUnavailable class. Cons for me are

• clearly separated code (one class, one functionality)
• clearly separated documentation entries (as a user, spotting a big class name would be faster than spotting a switch parameter)
• As similar as the class bodies are, casting them into one is not possible as far as I see (modulo vs. no modulo operation), so one would have the two logic blocks either way. And then the arguments above apply again.
• one parameter less

Sure, I can add tests without a horizon set. Any particular case you want to see covered?

I appreciate that. Cool! :sunglasses: Before merging though, I would also like to add an InterruptibleTask class since I definitely need this for finishing my visualizer. I start right away.

[jbdyn]

As it turned out, the InterruptibleTask was not the right way and instead I need new ResourceInterrupted and ResourcePeriodicallyInterrupted constraints. The ResourceUnavailable and ResourcePeriodicallyUnavailable constraints are defined such that they exclude any overlap of a task with given time intervals, thus leading to an unsatisfied solver.
In addition, I need to select a task which allows variation in its duration, i.e. VariableDurationTask.

For now, it only kind of works when only a single time interval is given in list_of_time_intervals. I still need to account for the sum of overlapped intervals, but wanted to share some usable code.

[tpaviot]

Yes, the ResourceUnavailable prevent an overlapping and is not compliant with an "interruption" or a "break". Actually I thinkg the "break" or "interruption" if softer than an unavailability. If the resource is "unavailable", it's like an empy set, a void, the resource is not there, it cannot do anything even having a break. In case the (human) resource needs a break, or a rotating machine needs some time to get cooled, the semantics of the "break" is more suitable. But there is also a connection with the task itself: the task must be "breakable". Indeed there are some tasks that cannot be interrupted when they are started. So, I see two different cases:

1. a "ResourceInterrupted" is assigned a "BreakbleTask": everything is ok, the tasks starts, the resource takes its break and goes back to work to finish the task
2. a "ResourceInterrupted" is assigned a "NonBreakableTask" that will be scheduled after, or before, the break, but there cannot be any overlapping

I'm not sure that your first draft handles the two cases, I have the feeling it considers all tasks as "breakable" by default.

[jbdyn]

@tpaviot I rewrote ResourceInterrupted and ResourcePeriodicallyInterrupted.

You were right: Before, I treated all tasks as breakable. Now the code clearly separates between those two types of tasks. In addition, the sum of all overlapped time intervals is respected, so the classes are basically finished.

I also marked the parts which still concern me with a # TODO.

What do you think?

[tpaviot]

Thank you, this is a great contribution, related to #117 and #124

I suggest to merge the PR as is, I will perhaps commit a few cosmestic changes

[jbdyn]

Thank you, @tpaviot!

I appreciate your positive feedback. :relaxed: