add extra validation and use 100 for the missing values instead of th…

…e roundtrip

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

flexmeasures/data/models/planning/storage.py

@@ -101,7 +101,6 @@ def _prepare(self, skip_validation: bool = False) -> tuple:  # noqa: C901
         soc_max = self.flex_model.get("soc_max")
         soc_minima = self.flex_model.get("soc_minima")
         soc_maxima = self.flex_model.get("soc_maxima")
-        roundtrip_efficiency = self.flex_model.get("roundtrip_efficiency")
         storage_efficiency = self.flex_model.get("storage_efficiency")
         prefer_charging_sooner = self.flex_model.get("prefer_charging_sooner", True)
 
@@ -270,29 +269,37 @@ def _prepare(self, skip_validation: bool = False) -> tuple:  # noqa: C901
         charging_efficiency = get_continuous_series_sensor_or_quantity(
             quantity_or_sensor=self.flex_model.get("charging_efficiency"),
             actuator=sensor,
-            unit="%",
+            unit="dimensionless",
             query_window=(start, end),
             resolution=resolution,
             beliefs_before=belief_time,
             fallback_attribute="charging-efficiency",
-        )
+        ).fillna(1)
         discharging_efficiency = get_continuous_series_sensor_or_quantity(
             quantity_or_sensor=self.flex_model.get("discharging_efficiency"),
             actuator=sensor,
-            unit="%",
+            unit="dimensionless",
             query_window=(start, end),
             resolution=resolution,
             beliefs_before=belief_time,
             fallback_attribute="discharging-efficiency",
-        )
+        ).fillna(1)
 
-        device_constraints[0][
-            "derivative down efficiency"
-        ] = discharging_efficiency.fillna(roundtrip_efficiency**0.5)
-        device_constraints[0]["derivative up efficiency"] = charging_efficiency.fillna(
-            roundtrip_efficiency**0.5
+        roundtrip_efficiency = self.flex_model.get(
+            "roundtrip_efficiency", self.sensor.get_attribute("roundtrip_efficiency", 1)
         )
 
+        # if dis/charging efficiency pair is not defined then the roundtrip efficiency is used
+        if not (
+            "charging_efficiency" in self.flex_model
+            or self.sensor.has_attribute("charging-efficiency")
+        ):
+            charging_efficiency = roundtrip_efficiency**0.5
+            discharging_efficiency = roundtrip_efficiency**0.5
+
+        device_constraints[0]["derivative down efficiency"] = discharging_efficiency
+        device_constraints[0]["derivative up efficiency"] = charging_efficiency
+
         # Apply storage efficiency (accounts for losses over time)
         device_constraints[0]["efficiency"] = storage_efficiency
 
@@ -461,16 +468,6 @@ def deserialize_flex_config(self):
                 "storage_efficiency", 1
             )
 
-        # Check for round-trip efficiency
-        # todo: simplify to: `if self.flex_model.get("roundtrip-efficiency") is None:`
-        if (
-            "roundtrip-efficiency" not in self.flex_model
-            or self.flex_model["roundtrip-efficiency"] is None
-        ):
-            # Get default from sensor, or use 100% otherwise
-            self.flex_model["roundtrip-efficiency"] = self.sensor.get_attribute(
-                "roundtrip_efficiency", 1
-            )
         self.ensure_soc_min_max()
 
         # Now it's time to check if our flex configurations holds up to schemas

flexmeasures/data/models/planning/tests/conftest.py

@@ -223,7 +223,7 @@ def process(db, building, setup_sources) -> dict[str, Sensor]:
 def efficiency_sensors(db, add_battery_assets, setup_sources) -> dict[str, Sensor]:
     battery = add_battery_assets["Test battery"]
     sensors = {}
-    sensor_specs = [("efficiency", timedelta(minutes=15), 0.9)]
+    sensor_specs = [("efficiency", timedelta(minutes=15), 90)]
 
     for name, resolution, value in sensor_specs:
         # 1 days of test data

flexmeasures/data/models/planning/tests/test_solver.py

@@ -1450,57 +1450,66 @@ def test_battery_power_capacity_as_sensor(
     assert all(device_constraints["derivative max"].values == expected_consumption)
 
 
-@pytest.mark.parametrize(
-    "flex_model_field,device_constraint",
-    [
-        ("charging-efficiency", "derivative up efficiency"),
-        ("discharging-efficiency", "derivative down efficiency"),
-    ],
-)
+def get_efficiency_problem_device_constraints(
+    extra_flex_model, efficiency_sensors, add_battery_assets
+) -> pd.DataFrame:
+
+    _, battery = get_sensors_from_db(add_battery_assets)
+    tz = pytz.timezone("Europe/Amsterdam")
+    start = tz.localize(datetime(2015, 1, 1))
+    end = tz.localize(datetime(2015, 1, 2))
+    resolution = timedelta(minutes=15)
+    base_flex_model = {
+        "soc-max": 2,
+        "soc-min": 0,
+    }
+    base_flex_model.update(extra_flex_model)
+    scheduler: Scheduler = StorageScheduler(
+        battery, start, end, resolution, flex_model=base_flex_model
+    )
+
+    scheduler_data = scheduler._prepare()
+    return scheduler_data[5][0]
+
+
 def test_dis_charging_efficiency_as_sensor(
     db,
     add_battery_assets,
     add_inflexible_device_forecasts,
     add_market_prices,
     efficiency_sensors,
-    device_constraint,
-    flex_model_field,
 ):
     """
-    The efficiency sensor defined an efficiency of 90% for 23h of the 24h of the schedule. The last
-    hour should use the roundtrip-efficiency of 92.16% (charge and discharge efficiencies of 96%).
+    Check that the charging and discharging efficiency can be defined in the flex-model.
+    For missing values, the fallback value is 100%.
     """
 
-    _, battery = get_sensors_from_db(add_battery_assets)
     tz = pytz.timezone("Europe/Amsterdam")
-    start = tz.localize(datetime(2015, 1, 1))
+
     end = tz.localize(datetime(2015, 1, 2))
     resolution = timedelta(minutes=15)
 
-    scheduler: Scheduler = StorageScheduler(
-        battery,
-        start,
-        end,
-        resolution,
-        flex_model={
-            "soc-max": 2,
-            "soc-min": 0,
-            flex_model_field: {"sensor": efficiency_sensors["efficiency"].id},
-            "roundtrip-efficiency": 0.9216,
-        },
-    )
+    extra_flex_model = {
+        "charging-efficiency": {"sensor": efficiency_sensors["efficiency"].id},
+        "discharging-efficiency": "200%",
+    }
 
-    scheduler_data = scheduler._prepare()
-    device_constraints = scheduler_data[5][0]
+    device_constraints = get_efficiency_problem_device_constraints(
+        extra_flex_model, efficiency_sensors, add_battery_assets
+    )
 
     assert all(
-        device_constraints[: end - timedelta(hours=1) - resolution][device_constraint]
+        device_constraints[: end - timedelta(hours=1) - resolution][
+            "derivative up efficiency"
+        ]
         == 0.9
     )
     assert all(
-        device_constraints[end - timedelta(hours=1) :][device_constraint] == 0.96
+        device_constraints[end - timedelta(hours=1) :]["derivative up efficiency"] == 1
     )
 
+    assert all(device_constraints["derivative down efficiency"] == 2)
+
 
 @pytest.mark.parametrize(
     "stock_delta_sensor",

flexmeasures/data/schemas/scheduling/storage.py

@@ -11,7 +11,7 @@
     fields,
     validates,
 )
-from marshmallow.validate import OneOf
+from marshmallow.validate import OneOf, ValidationError
 
 from flexmeasures.data.models.time_series import Sensor
 from flexmeasures.data.schemas.times import AwareDateTimeField
@@ -145,6 +145,20 @@ def check_whether_targets_exceed_max_planning_horizon(self, data: dict, **kwargs
                 f"Target datetime exceeds {max_server_datetime}. Maximum scheduling horizon is {max_server_horizon}."
             )
 
+    @validates_schema
+    def check_whether_efficiency_pair_is_incomplete(self, data: dict, **kwargs):
+        """
+        Check if one of the efficiency fields is missing.
+
+        :raise: ValidationError
+        """
+        fields = ["charging_efficiency", "discharging_efficiency"]
+
+        if any(fields[b] in data and fields[1 - b] not in data for b in [0, 1]):
+            raise ValidationError(
+                "`charging-efficiency` and `discharge-efficiency` need to be provided as a pair. Only one of them was povided"
+            )
+
     @post_load
     def post_load_sequence(self, data: dict, **kwargs) -> dict:
         """Perform some checks and corrections after we loaded."""

flexmeasures/data/schemas/tests/conftest.py

@@ -6,16 +6,20 @@
 
 
 @pytest.fixture(scope="module")
-def setup_dummy_sensors(db, app):
-
+def dummy_asset(db, app):
     dummy_asset_type = GenericAssetType(name="DummyGenericAssetType")
     db.session.add(dummy_asset_type)
 
-    dummy_asset = GenericAsset(
+    _dummy_asset = GenericAsset(
         name="DummyGenericAsset", generic_asset_type=dummy_asset_type
     )
-    db.session.add(dummy_asset)
+    db.session.add(_dummy_asset)
+
+    return _dummy_asset
+
 
+@pytest.fixture(scope="module")
+def setup_dummy_sensors(db, app, dummy_asset):
     sensor1 = Sensor(
         "sensor 1",
         generic_asset=dummy_asset,
@@ -51,3 +55,17 @@ def setup_dummy_sensors(db, app):
     db.session.commit()
 
     yield sensor1, sensor2
+
+
+@pytest.fixture(scope="module")
+def setup_efficiency_sensors(db, app, dummy_asset):
+    sensor = Sensor(
+        "efficiency",
+        generic_asset=dummy_asset,
+        event_resolution=timedelta(hours=1),
+        unit="%",
+    )
+    db.session.add(sensor)
+    db.session.commit()
+
+    return sensor

flexmeasures/data/schemas/tests/test_scheduling.py

@@ -1,8 +1,11 @@
+import pytest
+
 from flexmeasures.data.schemas.scheduling.process import (
     ProcessSchedulerFlexModelSchema,
     ProcessType,
 )
-
+from flexmeasures.data.schemas.scheduling.storage import StorageFlexModelSchema
+from marshmallow.validate import ValidationError
 from datetime import datetime
 import pytz
 
@@ -73,3 +76,53 @@ def test_process_scheduler_flex_model_process_type(db, app, setup_dummy_sensors)
     )
 
     assert process_scheduler_flex_model["process_type"] == ProcessType.SHIFTABLE
+
+
+@pytest.mark.parametrize(
+    "fields, fails",
+    [
+        (
+            [
+                "charging-efficiency",
+            ],
+            True,
+        ),
+        (
+            [
+                "discharging-efficiency",
+            ],
+            True,
+        ),
+        (["discharging-efficiency", "charging-efficiency"], False),
+    ],
+)
+def test_efficiency_pair(
+    db, app, setup_dummy_sensors, setup_efficiency_sensors, fields, fails
+):
+    """
+    Check that the fields `charging-efficiency` and `discharging-efficiency` always go together.
+    If one of them is missing, the validation should raise an error.
+    """
+
+    sensor1, _ = setup_dummy_sensors
+
+    schema = StorageFlexModelSchema(
+        sensor=sensor1,
+        start=datetime(2023, 1, 1, tzinfo=pytz.UTC),
+    )
+
+    def load_schema():
+        flex_model = {
+            "storage-efficiency": 1,
+            "soc-at-start": 0,
+        }
+        for f in fields:
+            flex_model[f] = {"sensor": setup_efficiency_sensors.id}
+
+        schema.load(flex_model)
+
+    if fails:
+        with pytest.raises(ValidationError):
+            load_schema()
+    else:
+        load_schema()