Add unit tests for Port and Node base classes

CHANGELOG.md

@@ -10,7 +10,11 @@
 
 - Prevent warnings for unconnected (dangling) ports on EVs.
 - NewSolar node renamed ScaledSolar.
-- Decompose the large model modules into smaller more manageable modules. 
+- Decomposes the large model modules into smaller more manageable modules. 
+- Trying to add a port to a model with import/export contraints set to `FlowConstraint.Series` or `FlowConstraint.InRange` now raises a `NotImplementedError`.
+- When setting active periods on a Port, the `set_active_periods_from_array` method now accepts a list of booleans in addition to a list of 0's and 1's.
+- The `get_port` method on Node now returns None rather than raising a ValueError, if no such port with that name can be found.
+- `verify_node` now called when adding nodes to models and `verify_node` modified to now verify not only check the node but also verify (via call to `verify_port`) any ports attached to the node. 
 
 
 ## Releases

src/echo/models/base/node.py

@@ -46,22 +46,25 @@ def add_ports_from_list(self, names: Iterable[str], port_type: type[Port], **kwa
         for name in names:
             self.add_port(name, port_type(**kwargs))
 
-    def get_port(self, port_name: str) -> Port:
+    def get_port(self, port_name: str) -> Port | None:
         """Returns the Port object with the name port_name.
 
         Args:
             port_name: The name of the Port.
 
         Returns:
-            Port: The port object with the name port_name
+            Port: The port object with the name port_name, or None if the port isn't found
         """
 
-        port = self.ports.get(port_name)
+        return self.ports.get(port_name)
 
-        if port is not None:
-            return port
-        else:
-            raise ValueError(f"Port name: {port_name} does not correspond to any Port object.")
+    def num_ports(self) -> int:
+        """Returns the number of ports associated with this node.
+
+        Returns:
+            The number of ports for this node.
+        """
+        return len(self.ports)
 
     def verify_node(self) -> None:
         """Checks there is at least one port associated with this node.
@@ -70,9 +73,12 @@ def verify_node(self) -> None:
             ConfigurationError: If there are no ports present on this node.
         """
 
-        if bool(self.ports) is False:
+        if len(self.ports) < 1:
             raise ConfigurationError("A node must have at least one port.")
 
+        for port in self.ports.values():
+            port.verify_port()
+
     def add_node_to_model(self, model: EchoConcreteModel, profile: pd.DataFrame) -> None:
         """Add this node to a concrete model.
 
@@ -81,8 +87,8 @@ def add_node_to_model(self, model: EchoConcreteModel, profile: pd.DataFrame) ->
             profile: The data associated with this node.
         """
 
+        self.verify_node()
         for port in self.ports.values():
-            port.verify_port()
             port.add_port_to_model(model, profile)
 
     # @abc.abstractmethod
@@ -112,24 +118,13 @@ def add_objective(self, model: EchoConcreteModel) -> None:
         Returns:
             None
         """
-        total = 0
-
-        self.objective += total
-
-    def num_ports(self) -> int:
-        """Returns the number of ports associated with this node.
-
-        Returns:
-            The number of ports for this node.
-        """
-
-        return len(self.ports)
+        pass
 
     # @abc.abstractmethod
     def apply_node_constraints(self, model: EchoConcreteModel) -> None:
         """Apply constraints associated with this node to a concrete model.
 
-        Can be overwritten.
+        Intended to be overridden in subclasses.
 
         Define constraints as functions that returns a Constraint, EqualityExpression or InequalityExpression.
 
@@ -152,7 +147,6 @@ def tellegen_node_rule(model: EchoConcreteModel, p: int, t:int) -> EqualityExpre
         Returns:
             None
         """
-
         pass
 
     def get_port_name_to_port_dict_name_map(self) -> dict[str, str]:

src/echo/models/base/port.py

@@ -15,7 +15,7 @@
 from echo.constants import negative_variable_component, positive_variable_component
 from echo.exceptions import ConfigurationError
 from echo.models.base import BaseModel
-from echo.models.base.types import ConstraintValueType, InitialValueInput
+from echo.models.base.types import ConstraintValueType, InitialValue, InitialValueInput
 from echo.models.scenario import EchoConcreteModel
 from echo.utils import (
     TimeSeriesData,
@@ -121,7 +121,7 @@ def set_flow_constraints(
 
     def process_initial_value(
         self,
-        initial_val: InitialValueInput,
+        initial_val: InitialValueInput | str,
         expansion_periods: int = 1,
         time_periods: int | None = None,
     ) -> None:
@@ -348,8 +348,8 @@ def _determine_initial_value(
         self,
         time_periods: int,
         expansion_periods: int,
-        profile: pd.DataFrame,
-    ) -> dict[tuple[int, int], float]:
+        profile: pd.DataFrame | None,
+    ) -> InitialValue:
 
         initial_value_scaling = self.initial_value_scaling or 1
 
@@ -367,7 +367,7 @@ def _determine_initial_value(
 
         return initial_val
 
-    def add_port_to_model(self, model: EchoConcreteModel, profile: pd.DataFrame) -> None:
+    def add_port_to_model(self, model: EchoConcreteModel, profile: pd.DataFrame | None) -> None:
         """Creates pyomo vars, params, and constraints for the port."""
         initial_value = self._determine_initial_value(
             time_periods=len(model.Time),
@@ -388,9 +388,15 @@ def add_port_to_model(self, model: EchoConcreteModel, profile: pd.DataFrame) ->
         if self.import_constraint is FlowConstraint.Fixed:  # only apply import/export constraints to variables
             self._add_import_constraints_to_model(model=model)
 
+        if self.import_constraint in [FlowConstraint.Series, FlowConstraint.InRange]:
+            raise NotImplementedError("Series and InRange import flow constraints are not implemented")
+
         if self.export_constraint is FlowConstraint.Fixed:  # only apply these constraints to variables
             self._add_export_constraints_to_model(model=model)
 
+        if self.export_constraint in [FlowConstraint.Series, FlowConstraint.InRange]:
+            raise NotImplementedError("Series and InRange export flow constraints are not implemented")
+
         if self.active_periods is not None:
             self._add_active_period_constraints_to_model(model=model)
 
@@ -499,7 +505,7 @@ def set_initial_value_from_array(
         self.set_initial_value_from_timeseriesdata(time_series_data=time_series_data)
 
     def set_active_periods_from_array(
-        self, array: list[bool], expansion_periods: int = 1, time_periods: int | None = None
+        self, array: list[bool] | list[int], expansion_periods: int = 1, time_periods: int | None = None
     ) -> None:
         """Sets port active periods
 
@@ -511,8 +517,17 @@ def set_active_periods_from_array(
         if time_periods is None:
             time_periods = len(array)
 
+        # We need an array which only contains 0 or 1 representing inactive (flow fixed to 0)
+        # or active (flow can be optimised)
+        # Convert bools to ints
+        active_periods_as_ints = [int(i) for i in array]
+        set_of_active_periods = set(active_periods_as_ints)
+
+        if set_of_active_periods not in [{0}, {1}, {0, 1}]:
+            raise ValueError("Active periods must be a list of booleans or a list only containing 0's or 1's")
+
         time_series_data = TimeSeriesData(
-            value=array,
+            value=active_periods_as_ints,
             num_time_intervals=time_periods,
             num_expansion_intervals=expansion_periods,
         )

tests/unit/models/base/test_node.py

@@ -0,0 +1,64 @@
+from unittest.mock import MagicMock
+
+import pytest
+import shortuuid
+
+from echo.exceptions import ConfigurationError
+from echo.models.base.node import Node
+from echo.models.base.port import Port
+
+
+def test_node_name():
+    name = "my-node-name"
+    node = Node(node_name=name)
+    assert node.node_name == name
+
+    node = Node()
+    basename = "node_"
+    assert node.node_name.startswith(basename)
+    assert len(node.node_name) == len(shortuuid.uuid()) + len(basename)
+
+
+def test_add_port():
+    num_ports = 9
+    ports = [Port(port_name=f"port_{i}") for i in range(num_ports)]
+
+    node = Node()
+    for p in ports:
+        node.add_port(p.port_name, p)
+
+    assert len(node.ports) == len(ports)
+    assert node.num_ports() == len(ports)
+    for p in ports:
+        assert node.get_port(p.port_name) == p
+
+
+def test_add_ports_from_list():
+    num_ports = 9
+    port_names = [f"port_{i}" for i in range(num_ports)]
+
+    node = Node()
+    node.add_ports_from_list(names=port_names, port_type=Port)
+
+    assert len(node.ports) == len(port_names)
+    assert node.num_ports() == len(port_names)
+
+
+def test_verify_ports_raises_configurationerror():
+    node = Node()
+    with pytest.raises(ConfigurationError):
+        node.verify_node()
+
+
+def test_add_node_to_model(empty_model):
+    node = Node(node_name="node_1")
+    port_instance = MagicMock()
+    port_instance.verify_port.return_value = None
+    port_instance.add_port_to_model.return_value = None
+
+    node.add_port("port_1", port_instance)
+    model = empty_model()
+    node.add_node_to_model(model, profile=None)
+
+    port_instance.verify_port.assert_called_once()
+    port_instance.add_port_to_model.assert_called_once_with(model, None)