Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add ProcessScheduler #729

Merged
merged 31 commits into from Jul 31, 2023
Merged

Conversation

victorgarcia98
Copy link
Contributor

@victorgarcia98 victorgarcia98 commented Jun 15, 2023

Description

Following the code in E-mission, this PR introduces a new scheduler that can schedule flexible loads. The ProcessScheduler implements the logic to schedule three different types of loads. Optionally, the user can specify a series of time restrictions (defined as a start and duration pair) where the load cannot be schedule to, in other words, the load cannot be ON during the course of any the time restriction.

This class handles three type of loads:

  • Inflexible: scheduled as soon as possible
  • Breakable: can be split into multiple blocks that don't need to be connected.
  • Shiftable: can be scheduled at any time within the planned windows, respecting the time restrictions.

Tests

This PR includes tests for the following features:

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
…test.py

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 self-assigned this Jun 15, 2023
@victorgarcia98 victorgarcia98 changed the title Add Shiftable Load Scheduler feat: add ShiftableLoadScheduler Jun 15, 2023
@victorgarcia98 victorgarcia98 marked this pull request as ready for review June 15, 2023 11:59
@Flix6x Flix6x added this to the 0.15.0 milestone Jun 15, 2023
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!!

I have yet to review the intelligent part, but I already have some comments about the modeling and documentation.

Also, I believe this scheduler deserves a dedicated CLI command.

And I believe we should add one simple integration test in api/v3_0/tests/test_sensor_schedules.py, to see that this scheduler can be used via the API, with the job queue involved.

flexmeasures/data/schemas/scheduling/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/scheduling/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In services.scheduling.find_scheduler_class we should look up this class if the asset type is LoadType.

Then, this scheduler doesn't require custom configuration of scheduling like you did in #768:

{"custom-scheduler": {"class": "ShiftableLoadScheduler", "module": "flexmeasures.data.models.planning.shiftable_load"}

This should be one of our batteries-included schedulers which just works.

@nhoening
Copy link
Contributor

In services.scheduling.find_scheduler_class we should look up this class if the asset type is LoadType.

Just a note on this:

While we should do what I said above in this PR, the real solution should be that Asset Types can specify what scheduler class should be used for them. That should be its own PR, probably.

I might start a discussion, as I don't know what I really meant with "logic store" in the code comment, and looking up logic/algorithms might get a new representation after our work on reporters.

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do have some wishes here but this looks close to ready to me!

flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/times.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
@victorgarcia98
Copy link
Contributor Author

I might start a discussion, as I don't know what I really meant with "logic store" in the code comment, and looking up logic/algorithms might get a new representation after our work on reporters.

Good point!

In the context of DataGenerators (PR #751), the scheduler type would go in the DataSource. I think this is convenient as we can have multiple scheduler for the same asset.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving forward, thanks!

See also my inline comment about doing the feasibility check in another way.

flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/shiftable_load.py Outdated Show resolved Hide resolved
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some mentions of consumption-price-sensor which we can do without, or whoch shluld be moved.

flexmeasures/data/schemas/scheduling/shiftable_load.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/tests/test_scheduling.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/tests/test_scheduling.py Outdated Show resolved Hide resolved
flexmeasures/data/schemas/tests/test_scheduling.py Outdated Show resolved Hide resolved
… only when optimizing a INFLEXIBLE or SHIFTABLE load type.

Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 changed the title feat: add ShiftableLoadScheduler feat: add ProcessScheduler Jul 26, 2023
@nhoening
Copy link
Contributor

I might start a discussion, as I don't know what I really meant with "logic store" in the code comment, and looking up logic/algorithms might get a new representation after our work on reporters.

Good point!

In the context of DataGenerators (PR #751), the scheduler type would go in the DataSource. I think this is convenient as we can have multiple scheduler for the same asset.

Just as an addendum from our conversation today: Multiple schedulers per asset is not the standard use case. It's an edge case. I'm careful with starting modeling sketches with the edge case, as it's confusing for users/developers.

Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have one more renaming request (if you agree), sorry :)

Otherwise this is it IMO

flexmeasures/data/models/planning/process.py Outdated Show resolved Hide resolved
flexmeasures/data/models/planning/process.py Outdated Show resolved Hide resolved
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve, with one more tiny wish!

flexmeasures/data/schemas/scheduling/process.py Outdated Show resolved Hide resolved
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
Signed-off-by: Victor Garcia Reolid <victor@seita.nl>
@victorgarcia98 victorgarcia98 merged commit 1b62bd7 into main Jul 31, 2023
4 checks passed
@victorgarcia98 victorgarcia98 deleted the feature/shiftable-load-scheduler branch July 31, 2023 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants