flexmeasures icon indicating copy to clipboard operation
flexmeasures copied to clipboard

Document & refactor scheduling specs for storage flexibility model

Open nhoening opened this issue 1 year ago • 1 comments

This PR adds documentation to the /api/sensor//schedules/trigger endpoint to better understand how to describe a flexibility model. Also in preparation for more flexibility modes to come, right now we only support storage.

The code also gets a small refactoring to make this clearer to developers.

nhoening avatar Oct 01 '22 16:10 nhoening

Pull Request Test Coverage Report for Build 3392429724

  • 187 of 200 (93.5%) changed or added relevant lines in 8 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.1%) to 65.236%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flexmeasures/data/models/planning/init.py 4 5 80.0%
flexmeasures/data/models/planning/battery.py 49 50 98.0%
flexmeasures/data/models/planning/utils.py 20 23 86.96%
flexmeasures/data/models/planning/charging_station.py 49 53 92.45%
flexmeasures/data/services/scheduling.py 49 53 92.45%
<!-- Total: 187 200
Files with Coverage Reduction New Missed Lines %
flexmeasures/api/v1_3/implementations.py 1 67.41%
flexmeasures/api/v3_0/sensors.py 2 69.95%
<!-- Total: 3
Totals Coverage Status
Change from base Build 3380979685: 0.1%
Covered Lines: 6599
Relevant Lines: 9498

💛 - Coveralls

coveralls avatar Oct 01 '22 16:10 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?

nhoening avatar Nov 02 '22 10:11 nhoening

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):

Flix6x avatar Nov 02 '22 11:11 Flix6x

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.

nhoening avatar Nov 02 '22 13:11 nhoening

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.

Flix6x avatar Nov 02 '22 20:11 Flix6x

Yes I found out about them, as well.

Is your idea that the parameter decides which data source to use?

nhoening avatar Nov 02 '22 20:11 nhoening

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.

Flix6x avatar Nov 02 '22 20:11 Flix6x

I'd very happily skip making that migration, but you're the better judge here.

nhoening avatar Nov 02 '22 20:11 nhoening