Refactor and fix size_commodity property

This should be the last step of implementing pint units and adding
support for time intervals different from hourly.

syfop/network.py

@@ -219,11 +219,6 @@ def _set_output_connections(self, nodes):
                 ]
 
                 if len(node.output_commodities) == 0:
-                    if node.size_commodity is None:
-                        raise ValueError(
-                            f"node '{node.name}' has no output nodes defined, so "
-                            "size_commmodity must be set"
-                        )
                     # here we have no ouputs of a Node, so there is only the leaf output flow
                     node.output_commodities = [node.size_commodity]
 

syfop/node.py

@@ -102,13 +102,6 @@ def __init__(
         )
         self.input_flows = None
 
-    def _get_size_commodity(self):
-        # TODO refactor this into a property
-
-        # there can be only one output commodity because we don't have convertion_factors here
-        assert len(set(self.output_commodities)) == 1
-        return self.output_commodities[0]
-
     def create_variables(self, model, time_coords):
         super().create_variables(model, time_coords)
         self.input_flows = {"": self.size * self.input_profile}
@@ -198,7 +191,7 @@ def __init__(
         size_commodity : str
             Which commodity is used to define the size of the Node. This parameter is only
             required, if there is more than one output commodity or if there are no output nodes
-            connected.
+            connected, otherwise it is defined automatically.
         input_proportions : dict
             Proportions of the input flows. The keys are the names of the input commodities and the
             values are a quantity of the type of the input commodity, all multiples of these values
@@ -233,32 +226,20 @@ def __init__(
         # output_proportions are checked in Network.__init__, when we know the output commodities
         self._check_proportions_valid(input_proportions, self.input_commodities, "input")
 
-        self.size_commodity = size_commodity
+        self._size_commodity = size_commodity
         self.input_proportions = input_proportions
         self.output_proportions = output_proportions
 
         self.input_flow_costs = input_flow_costs
 
-    def _get_size_commodity(self):
-        # TODO refactor this into a property
-        if self.size_commodity is None:
-            if len(set(self.output_commodities)) > 1:
-                raise ValueError(
-                    "size_commodity not provided, but required for multiple "
-                    "different output commodities"
-                )
-            return self.output_commodities[0]
-        else:
-            return self.size_commodity
-
     def create_constraints(self, model):
         super().create_constraints(model)
 
         # constraint: output_flows are limited by the size of technology in each timestamp
         # Note: this is not needed for NodeScalableInput and NodeScalableOutput because there the
         # input_profile and output_profile are checked to be between 0 and 1.
         if self.size is not None:
-            output_flows = self._get_output_flows(self._get_size_commodity())
+            output_flows = self._get_output_flows(self.size_commodity)
             lhs = sum(output_flows) - self.size
 
             # FIXME this is probably probably missing for NodeScalableInput

syfop/node_base.py

@@ -26,6 +26,7 @@ def __init__(
         # this needs to be filled later
         self.outputs = None
         self.output_flows = None
+        self._size_commodity = None
 
         # overwritten in some subclasses
         self.input_commodities = None
@@ -61,6 +62,26 @@ def _check_proportions_valid(self, proportions, commodities, input_or_output):
                 f"{set(commodities)}"
             )
 
+    @property
+    def size_commodity(self):
+        """Which commodity is used to define the size of the Node."""
+        # self._size_commodity can be not None only for type Node. All other types should have
+        # exactly one output commodity set - either directly in __init__ or by Network.__init__().
+        if self._size_commodity is None:
+            if len(set(self.output_commodities)) == 0:
+                raise ValueError(
+                    f"node '{self.name}' has no output nodes defined, so "
+                    "size_commmodity must be set"
+                )
+            if len(set(self.output_commodities)) > 1:
+                raise ValueError(
+                    "size_commodity not provided, but required for multiple "
+                    "different output commodities"
+                )
+            return self.output_commodities[0]
+        else:
+            return self._size_commodity
+
     def has_costs(self):
         return not (self.costs == 0.0 or self.costs is None)
 
@@ -349,7 +370,7 @@ def create_constraints(self, model):
 
     def storage_cost_magnitude(self, currency_unit):
         assert hasattr(self, "storage") and self.storage is not None, "node has no storage"
-        storage_unit = default_units[self._get_size_commodity()]
+        storage_unit = default_units[self.size_commodity]
         return self.storage.costs.to(currency_unit / (storage_unit * ureg.h)).magnitude
 
 
@@ -360,7 +381,7 @@ class NodeScalableBase(NodeBase):
     def costs_magnitude(self, currency_unit):
         """Returns the costs scaled to ``currency_unit`` / ``size_unit``, where ``size_unit`` is
         the unit of the output commodity."""
-        size_unit = default_units[self._get_size_commodity()]
+        size_unit = default_units[self.size_commodity]
         costs_mag = self.costs.to(currency_unit / size_unit).magnitude
         return costs_mag
 

tests/test_network.py

@@ -106,7 +106,7 @@ def test_simple_co2_storage(storage_type):
         co2_flow = 2 * co2_flow
         co2_flow[1::2] = np.array(0.0) * ureg.t / ureg.h
         co2_storage = Storage(
-            costs=1000 * ureg.EUR / (ureg.t / ureg.h),  # price not relevant, see comment above
+            costs=1000 * ureg.EUR / ureg.t,  # price not relevant, see comment above
             max_charging_speed=1.0,
             storage_loss=0.0,
             charging_loss=0.0,