flexmeasures
flexmeasures copied to clipboard
Document & refactor scheduling specs for storage flexibility model
This PR adds documentation to the /api/sensor/
The code also gets a small refactoring to make this clearer to developers.
Pull Request Test Coverage Report for Build 3494526674
- 158 of 182 (86.81%) changed or added relevant lines in 13 files are covered.
- 4 unchanged lines in 3 files lost coverage.
- Overall coverage decreased (-0.05%) to 64.974%
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
---|---|---|---|
flexmeasures/api/common/schemas/sensor_data.py | 1 | 2 | 50.0% |
flexmeasures/cli/data_add.py | 1 | 2 | 50.0% |
flexmeasures/data/models/planning/init.py | 4 | 5 | 80.0% |
flexmeasures/data/models/planning/storage.py | 59 | 60 | 98.33% |
flexmeasures/data/services/scheduling.py | 44 | 48 | 91.67% |
flexmeasures/data/models/planning/battery.py | 0 | 5 | 0.0% |
flexmeasures/data/models/planning/charging_station.py | 0 | 5 | 0.0% |
flexmeasures/data/models/planning/utils.py | 24 | 30 | 80.0% |
<!-- | Total: | 158 | 182 |
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
flexmeasures/api/common/schemas/sensor_data.py | 1 | 66.84% |
flexmeasures/api/v1_3/implementations.py | 1 | 67.53% |
flexmeasures/api/v3_0/sensors.py | 2 | 69.95% |
<!-- | Total: | 4 |
Totals | |
---|---|
Change from base Build 3487808393: | -0.05% |
Covered Lines: | 6583 |
Relevant Lines: | 9511 |
💛 - Coveralls
One thing to consider: We might add a data migration, so existing data sources for scheduling also get model and version information. But we haven't distinguished so far between the battery scheduler and the charging-station scheduler.
With this PR, we would start doing that, but I don't believe we can automate for people how to separate scheduling data attribution in their existing data.
The way forward I see now is to
- make an existing source with name "Seita" and type "scheduling script" the battery one (model "schedule_battery", version "1")
- add another one for model = "schedule_charging_station"
- Any existing scheduling data will still be linked to the first source. There is a potential SQL Update command which changes the source for scheduling data for all those sensors which actually represent a charging station (but I don't think there is a generic_asset_type which would help us).
How would you go forward with this?
And are you sure we shouldn't go with scheduler classes instead of functions, before opening up scheduler customisation to users?
class StorageScheduler(BaseScheduler):
__author__ = "Organization"
__version__ = 1
def schedule(sensor, start, end, resolution, storage_specs):
And are you sure we shouldn't go with scheduler classes instead of functions, before opening up scheduler customisation to users?
class StorageScheduler(BaseScheduler): __author__ = "Organization" __version__ = 1 def schedule(sensor, start, end, resolution, storage_specs):
I wanted to make another PR for that before the next release, and have postponed the exact decision.
One thing to consider: We might add a data migration, so existing data sources for scheduling also get model and version information. But we haven't distinguished so far between the battery scheduler and the charging-station scheduler. ... How would you go forward with this?
Sorry, I had missed this comment before. Such a data migration would make sense imo, but it's not really mandatory so we could use an opt-in parameter for the upgrade command in the revision file (we had some previous revisions that used parameters, which could serve as an example).
The relevant GenericAssetType.name
would be "one-way_evse"
and "two-way_evse"
btw.
Yes I found out about them, as well.
Is your idea that the parameter decides which data source to use?
Is your idea that the parameter decides which data source to use?
My idea was that the parameter decides whether to actually run the migration or skip its logic (and just update the alembic-version). But perhaps I haven't thought it through (or I'm overthinking it), and the migration should just be as you proposed.
One more thought that came to mind: aren't the battery and Charge Point schedulers the exact same (storage) scheduler by now, in terms of their logic? Then we'd need only one scheduler function (later/soon: scheduler class) and won't need to split the existing data over two sources.
I'd very happily skip making that migration, but you're the better judge here.
Handing it over to you again.
- [x] Merged PR #525 to ensure our left-inclusive DatetimeIndex works with pandas>=1.4
- [x] Our schedulers are merged into
StorageScheduler
and moved it to a new module - [x] Original scheduling functions placed back with deprecation warnings (in case plugins use them, to help them transition)
Thanks! I highly doubt a plugin would use them, but good thinking.
Maybe merging this PR at this stage would be nice, and I add the parameterization update for the trigger endpoint in a separate PR.
This PR is huge, and you just added a few smaller things, as well.
I just reviewed those fixes and am okay with them. Do you need to do any reviewing here or are we done?
The changelog message could maybe be expanded to include that we refactored basic scheduling code, as well, but it's not that interesting to users.
The changelog message could maybe be expanded
It might be good to mention that we merged our two scheduler functions (for batteries and Charge Points) into a single scheduler class.
Do we still need a migration, where we amend the data source for existing schedules? (all using the StorageScheduler)?
You're right, that is still missing.