Move over deprecated soc-targets field (#566)

Fixes the following bugs:

-  test_trigger_and_get_schedule was not checking for soc-targets sent in the deprecated soc-targets field
-  test_trigger_and_get_schedule was not taking into account efficiency losses when checking soc-targets
-  test_trigger_and_get_schedule was not checking for soc-targets sent within the flex-model field
-  test_trigger_and_get_schedule was assuming wrong units for the soc-max and soc-targets fields, thereby creating an unsolvable scheduling problem
-  The deprecated soc-targets field was hidden in an unused **kwargs variable, and not moved over to become part of the flex-model field
-  The deprecated soc-targets field was still being deserialized by the API endpoint rather than by the scheduler
-  The SoC state was stored with the wrong units as an asset attribute


* soc-targets are now deserialized with the SOCTargetSchema by the scheduler rather than by the /schedules/trigger/ endpoint

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Test whether SOC targets are met, in the new and the old way

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Fix test

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Another small bug

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Move field setup to a single place, and add inline comments relating to SoC units

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Now that we added comments about the SoC units, it becomes obvious that we set up the constraints for an infeasible problem; let's remedy that

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Let's not silently assume that a non-kWh unit is automatically a MWh unit

Signed-off-by: F.N. Claessen <felix@seita.nl>

* Refactor and test whether persist_flex_model did what it was supposed to do

Signed-off-by: F.N. Claessen <felix@seita.nl>

* black and flake8

Signed-off-by: F.N. Claessen <felix@seita.nl>

* changelog entry

Signed-off-by: F.N. Claessen <felix@seita.nl>

Signed-off-by: F.N. Claessen <felix@seita.nl>

documentation/changelog.rst

@@ -1,3 +1,4 @@
+
 **********************
 FlexMeasures Changelog
 **********************
@@ -40,7 +41,7 @@ Infrastructure / Support
 * Remove bokeh dependency and obsolete UI views [see `PR #476 <http://www.github.com/FlexMeasures/flexmeasures/pull/476>`_]
 * Fix ``flexmeasures db-ops dump`` and ``flexmeasures db-ops restore`` not working in docker containers [see `PR #530 <http://www.github.com/FlexMeasures/flexmeasures/pull/530>`_] and incorrectly reporting a success when `pg_dump` and `pg_restore` are not installed [see `PR #526 <http://www.github.com/FlexMeasures/flexmeasures/pull/526>`_]
 * Plugins can save BeliefsSeries, too, instead of just BeliefsDataFrames [see `PR #523 <http://www.github.com/FlexMeasures/flexmeasures/pull/523>`_]
-* Improve documentation and code w.r.t. storage flexibility modelling ― prepare for handling other schedulers & merge battery and car charging schedulers [see `PR #511 <http://www.github.com/FlexMeasures/flexmeasures/pull/511>`_ and `PR #537 <http://www.github.com/FlexMeasures/flexmeasures/pull/537>`_]
+* Improve documentation and code w.r.t. storage flexibility modelling ― prepare for handling other schedulers & merge battery and car charging schedulers [see `PR #511 <http://www.github.com/FlexMeasures/flexmeasures/pull/511>`_, `PR #537 <http://www.github.com/FlexMeasures/flexmeasures/pull/537>`_ and `PR #566 <http://www.github.com/FlexMeasures/flexmeasures/pull/566>`_]
 * Revised strategy for removing unchanged beliefs when saving data: retain the oldest measurement (ex-post belief), too [see `PR #518 <http://www.github.com/FlexMeasures/flexmeasures/pull/518>`_]
 * Scheduling test for maximizing self-consumption, and improved time series db queries for fixed tariffs (and other long-term constants) [see `PR #532 <http://www.github.com/FlexMeasures/flexmeasures/pull/532>`_]
 * Clean up table formatting for ``flexmeasures show`` CLI commands [see `PR #540 <http://www.github.com/FlexMeasures/flexmeasures/pull/540>`_]

flexmeasures/api/v3_0/sensors.py

@@ -36,7 +36,6 @@
 from flexmeasures.data.schemas.sensors import SensorSchema, SensorIdField
 from flexmeasures.data.schemas.times import AwareDateTimeField
 from flexmeasures.data.schemas.units import QuantityField
-from flexmeasures.data.schemas.scheduling.storage import SOCTargetSchema
 from flexmeasures.data.schemas.scheduling import FlexContextSchema
 from flexmeasures.data.services.sensors import get_sensors
 from flexmeasures.data.services.scheduling import (
@@ -240,9 +239,7 @@ def get_data(self, response: dict):
                     ]
                 ),
             ),  # todo: allow unit to be set per field, using QuantityField("%", validate=validate.Range(min=0, max=1))
-            "targets": fields.List(
-                fields.Nested(SOCTargetSchema), data_key="soc-targets"
-            ),
+            "targets": fields.List(fields.Dict, data_key="soc-targets"),
             "prefer_charging_sooner": fields.Bool(
                 data_key="prefer-charging-sooner", required=False
             ),
@@ -401,6 +398,7 @@ def trigger_schedule(  # noqa: C901
             (soc_min, "soc-min"),
             (soc_max, "soc-max"),
             (unit, "soc-unit"),
+            (kwargs.get("targets"), "soc-targets"),
             (roundtrip_efficiency, "roundtrip-efficiency"),
             (
                 prefer_charging_sooner,

flexmeasures/api/v3_0/tests/test_sensor_schedules.py

@@ -89,9 +89,6 @@ def test_trigger_and_get_schedule(
     asset_name,
 ):
     # trigger a schedule through the /sensors/<id>/schedules/trigger [POST] api endpoint
-    message["roundtrip-efficiency"] = 0.98
-    message["soc-min"] = 0
-    message["soc-max"] = 4
     assert len(app.queues["scheduling"]) == 0
 
     sensor = Sensor.query.filter(Sensor.name == asset_name).one_or_none()
@@ -106,11 +103,12 @@ def test_trigger_and_get_schedule(
             headers={"Authorization": auth_token},
         )
         print("Server responded with:\n%s" % trigger_schedule_response.json)
-        check_deprecation(trigger_schedule_response)
+        if "flex-model" not in message:
+            check_deprecation(trigger_schedule_response)
+            assert (
+                "soc-min" in trigger_schedule_response.json["message"]
+            )  # deprecation warning
         assert trigger_schedule_response.status_code == 200
-        assert (
-            "soc-min" in trigger_schedule_response.json["message"]
-        )  # deprecation warning
         job_id = trigger_schedule_response.json["schedule"]
 
     # look for scheduling jobs in queue
@@ -152,18 +150,28 @@ def test_trigger_and_get_schedule(
         == app.config.get("FLEXMEASURES_PLANNING_HORIZON") / resolution
     )
 
-    # check targets, if applicable
-    if "targets" in message:
+    if "flex-model" not in message:
         start_soc = message["soc-at-start"] / 1000  # in MWh
+        roundtrip_efficiency = message["roundtrip-efficiency"]
+        soc_targets = message.get("soc-targets")
+    else:
+        start_soc = message["flex-model"]["soc-at-start"] / 1000  # in MWh
+        roundtrip_efficiency = message["flex-model"]["roundtrip-efficiency"]
+        soc_targets = message["flex-model"].get("soc-targets")
+
+    # check targets, if applicable
+    if soc_targets:
         soc_schedule = integrate_time_series(
             consumption_schedule,
             start_soc,
+            up_efficiency=roundtrip_efficiency**0.5,
+            down_efficiency=roundtrip_efficiency**0.5,
             decimal_precision=6,
         )
         print(consumption_schedule)
         print(soc_schedule)
-        for target in message["targets"]:
-            assert soc_schedule[target["datetime"]] == target["soc-target"] / 1000
+        for target in soc_targets:
+            assert soc_schedule[target["datetime"]] == target["value"] / 1000
 
     # try to retrieve the schedule through the /sensors/<id>/schedules/<job_id> [GET] api endpoint
     get_schedule_message = message_for_get_device_message(
@@ -204,3 +212,6 @@ def test_trigger_and_get_schedule(
         get_schedule_response_long.json["values"][0:192]
         == get_schedule_response.json["values"]
     )
+
+    # Check whether the soc-at-start was persisted as an asset attribute
+    assert sensor.generic_asset.get_attribute("soc_in_mwh") == start_soc

flexmeasures/api/v3_0/tests/utils.py

@@ -54,18 +54,24 @@ def message_for_trigger_schedule(
             "start"
         ] = "2040-01-01T00:00:00+01:00"  # We have no beliefs in our test database about 2040 prices
 
+    flex_model = {
+        "soc-at-start": 12.1,  # in kWh, according to soc-unit
+        "soc-min": 0,  # in kWh, according to soc-unit
+        "soc-max": 40,  # in kWh, according to soc-unit
+        "soc-unit": "kWh",
+        "roundtrip-efficiency": 0.98,
+    }
     if deprecated_format_pre012:
-        message["soc-at-start"] = 12.1
-        message["soc-unit"] = "kWh"
+        # unpack flex model directly as message fields
+        message = dict(**message, **flex_model)
     else:
-        message["flex-model"] = {}
-        message["flex-model"]["soc-at-start"] = 12.1
-        message["flex-model"]["soc-unit"] = "kWh"
+        message["flex-model"] = flex_model
     if with_targets:
         if realistic_targets:
-            targets = [{"value": 3500, "datetime": "2015-01-02T23:00:00+01:00"}]
+            # this target (in kWh, according to soc-unit) is well below the soc_max_in_mwh on the battery's sensor attributes
+            targets = [{"value": 25, "datetime": "2015-01-02T23:00:00+01:00"}]
         else:
-            # this target is actually higher than soc_max_in_mwh on the battery's sensor attributes
+            # this target (in kWh, according to soc-unit) is actually higher than soc_max_in_mwh on the battery's sensor attributes
             targets = [{"value": 25000, "datetime": "2015-01-02T23:00:00+01:00"}]
         if deprecated_format_pre012:
             message["soc-targets"] = targets

flexmeasures/data/models/planning/storage.py

@@ -188,14 +188,17 @@ def compute_schedule(
     def persist_flex_model(self):
         """Store new soc info as GenericAsset attributes"""
         self.sensor.generic_asset.set_attribute("soc_datetime", self.start.isoformat())
-        if self.flex_model.get("soc_unit") == "kWh":
+        soc_unit = self.flex_model.get("soc_unit")
+        if soc_unit == "kWh":
             self.sensor.generic_asset.set_attribute(
                 "soc_in_mwh", self.flex_model["soc_at_start"] / 1000
             )
-        else:
+        elif soc_unit == "MWh":
             self.sensor.generic_asset.set_attribute(
                 "soc_in_mwh", self.flex_model["soc_at_start"]
             )
+        else:
+            raise NotImplementedError(f"Unsupported SoC unit '{soc_unit}'.")
 
     def deserialize_flex_config(self):
         """

flexmeasures/data/schemas/scheduling/storage.py

@@ -56,7 +56,7 @@ def post_load_sequence(self, data: dict, **kwargs) -> dict:
             if data.get("soc_targets"):
                 for target in data["soc_targets"]:
                     target["value"] /= 1000.0
-            data["soc_unit"] == "MWh"
+            data["soc_unit"] = "MWh"
 
         # Convert round-trip efficiency to dimensionless (to the (0,1] range)
         if data.get("roundtrip_efficiency") is not None: