Bug Fix: Operating Reductions Inconsistently Rest (#143)

* convert validator to 0-1 inclusive range and apply

* add replacement trigger for interrupts and fix bug in persistent failures

* save changes

* update changelog

* update replacement to subassembly id to avoid killing every process on a system

* fix bug in overextensive subassembly request purge

* remove extraneous key from toml

CHANGELOG.md

@@ -7,6 +7,10 @@
 - Updates Polars API usage to account for a series of deprecation and future warnings.
 - Changes the metrics demonstration to use the COREWIND Morro Bay in situ example, and
   adds the availability plotting to the demonstration example.
+- `RepairRequest.prior_operating_level` has been added to allow 100% reduction factor failures to correctly and consistently restore the operating level of a subassembly following a repair.
+- Replaces the `valid_reduction` attrs validator with `validate_0_1_inclusive` to reuse the logic in multiple places without duplicating checking methods.
+- Adds a `replacement` flag for interruption methods, so that a failure or replacement comment can be added as a cause for `simpy.process.interrupt`. This update allows the failure and maintenance processes to check if an interruption should cause the process to exit completely. Additionally, the forced exit ensures that processes can't persist after a replacement event when a process is recreated, which was happening in isolated cases.
+- Fixes a bug in `RepairManager.purge_subassemble_requests()` where the pending tows are cleared regardless of whether or not the focal subassembly is the cause of the tow, leading to a simulation failure.
 
 ## v0.9.3 (15 February 2024)
 

pyproject.toml

@@ -2,9 +2,6 @@
 requires = ["setuptools", "setuptools-scm"]
 build-backend = "setuptools.build_meta"
 
-[metadata]
-version = "attr: wombat.__version__"
-
 [project]
 name = "wombat"
 dynamic = ["version"]
@@ -184,7 +181,7 @@ fix = true
 # for rules included and matching to prefix.
 # TODO: "FBT", "B", "PIE, "T20", "SIM", "PTH", "PD", "I", "PL"
 ignore-init-module-imports = true
-select = ["F", "E", "W", "C4", "D", "UP", "NPY201"]
+select = ["F", "E", "W", "C4", "D", "UP"]
 
 # D205: not using summary lines and descriptions, just descriptions
 # D401: don't believe enough in imperative mode to make all the changes currently

tests/test_data_classes.py

@@ -26,10 +26,10 @@
     UnscheduledServiceEquipmentData,
     valid_hour,
     convert_to_list,
-    valid_reduction,
     annual_date_range,
     clean_string_input,
     convert_to_list_lower,
+    validate_0_1_inclusive,
     convert_ratio_to_absolute,
 )
 
@@ -151,14 +151,16 @@ class HourClass:
     assert hour.hour == 24
 
 
-def test_valid_reduction():
-    """Tests the ``valid_reduction`` validator."""
+def test_validate_0_1_inclusive():
+    """Tests the ``validate_0_1_inclusive`` validator."""
 
     @attr.s(auto_attribs=True)
     class ReductionClass:
-        """Dummy class for testing ``valid_reduction``."""
+        """Dummy class for testing ``validate_0_1_inclusive``."""
 
-        speed_reduction: int = attr.ib(converter=float, validator=valid_reduction)
+        speed_reduction: int = attr.ib(
+            converter=float, validator=validate_0_1_inclusive
+        )
 
     # Test the fringes
     with pytest.raises(ValueError):

wombat/core/data_classes.py

@@ -278,7 +278,7 @@ def valid_hour(
         raise ValueError(f"Input {attribute.name} must be between 0 and 24, inclusive.")
 
 
-def valid_reduction(
+def validate_0_1_inclusive(
     instance,
     attribute: Attribute,
     value: int | float,  # pylint: disable=W0613
@@ -292,8 +292,8 @@ def valid_reduction(
     """
     if value < 0 or value > 1:
         raise ValueError(
-            f"Input for {attribute.name}'s `speed_reduction_factor` must be between"
-            " 0 and 1, inclusive."
+            f"Input for {attribute.name} must be between 0 and 1, inclusive, not:"
+            f" {value=}."
         )
 
 
@@ -650,6 +650,9 @@ class RepairRequest(FromDictMixin):
     cable: bool = field(default=False, converter=bool, kw_only=True)
     upstream_turbines: list[str] = field(default=Factory(list), kw_only=True)
     upstream_cables: list[str] = field(default=Factory(list), kw_only=True)
+    prior_operating_level: float = field(
+        default=1, kw_only=True, validator=validate_0_1_inclusive
+    )
     request_id: str = field(init=False)
 
     def assign_id(self, request_id: str) -> None:
@@ -1068,7 +1071,7 @@ class ScheduledServiceEquipmentData(FromDictMixin, DateLimitsMixin):
     workday_end: int = field(default=-1, converter=int, validator=valid_hour)
     crew_transfer_time: float = field(converter=float, default=0.0)
     speed_reduction_factor: float = field(
-        default=0.0, converter=float, validator=valid_reduction
+        default=0.0, converter=float, validator=validate_0_1_inclusive
     )
     port_distance: float = field(default=0.0, converter=float)
     onsite: bool = field(default=False, converter=bool)
@@ -1287,7 +1290,7 @@ class UnscheduledServiceEquipmentData(FromDictMixin, DateLimitsMixin):
     workday_end: int = field(default=-1, converter=int, validator=valid_hour)
     crew_transfer_time: float = field(converter=float, default=0.0)
     speed_reduction_factor: float = field(
-        default=0.0, converter=float, validator=valid_reduction
+        default=0.0, converter=float, validator=validate_0_1_inclusive
     )
     port_distance: float = field(default=0.0, converter=float)
     onsite: bool = field(default=False, converter=bool)

wombat/core/repair_management.py

@@ -532,18 +532,23 @@ def invalidate_system(
                 self.systems_waiting_for_tow.index(system.id)
             )
 
-    def interrupt_system(self, system: System | Cable) -> None:
+    def interrupt_system(
+        self, system: System | Cable, replacement: str | None = None
+    ) -> None:
         """Sets the turbine status to be in servicing, and interrupts all the processes
         to turn off operations.
 
         Parameters
         ----------
         system_id : str
             The system to disable repairs.
+        replacement: str | None, optional
+            If a subassebly `id` is provided, this indicates the interruption is caused
+            by its replacement event. Defaults to None.
         """
         if system.servicing.triggered and system.id in self.invalid_systems:
             system.servicing = self.env.event()
-            system.interrupt_all_subassembly_processes()
+            system.interrupt_all_subassembly_processes(replacement=replacement)
         else:
             raise RuntimeError(
                 f"{self.env.simulation_time} {system.id} already being serviced"
@@ -671,19 +676,25 @@ def purge_subassembly_requests(
         if not self.items:
             return None
 
-        requests = [
+        # First check the system matches because we'll need these separated later to
+        # ensure we don't incorrectly remove towing requests from other subassemblies
+        system_requests = [
             request
             for request in self.items
-            if (
-                request.system_id == system_id
-                and request.subassembly_id == subassembly_id
-                and request.request_id not in exclude
-            )
+            if request.system_id == system_id and request.request_id not in exclude
         ]
-        if requests == []:
+        if system_requests == []:
             return None
 
-        for request in requests:
+        subassembly_requests = [
+            request
+            for request in system_requests
+            if request.subassembly_id == subassembly_id
+        ]
+        if subassembly_requests == []:
+            return None
+
+        for request in subassembly_requests:
             which = "repair" if isinstance(request.details, Failure) else "maintenance"
             self.env.log_action(
                 system_id=request.system_id,
@@ -702,14 +713,18 @@ def purge_subassembly_requests(
             _ = self.get(lambda x: x is request)  # pylint: disable=W0640
         sid = request.system_id
 
-        # Ensure that if it was reset, and a tow was waiting, that it gets cleared
+        # Ensure that if it was reset, and a tow was waiting, that it gets cleared,
+        # unless a separate subassembly required the tow
         if sid in self.systems_waiting_for_tow:
-            if sid not in self.systems_in_tow:
+            other_subassembly_match = [
+                r for r in system_requests if "TOW" in r.details.service_equipment
+            ]
+            if sid not in self.systems_in_tow and other_subassembly_match == []:
                 _ = self.systems_waiting_for_tow.pop(
                     self.systems_waiting_for_tow.index(sid)
                 )
 
-        return requests
+        return subassembly_requests
 
     @property
     def request_map(self) -> dict[str, int]:

wombat/core/service_equipment.py

@@ -489,8 +489,12 @@ def register_repair_with_subassembly(
             )
             subassembly.recreate_processes()
         elif operation_reduction == 1:
-            subassembly.operating_level = starting_operating_level
-            subassembly.broken.succeed()
+            subassembly.operating_level = repair.prior_operating_level
+            try:
+                subassembly.broken.succeed()
+            except RuntimeError as e:
+                print(subassembly.system.id, repair.details.description)
+                raise e
         elif operation_reduction == 0:
             subassembly.operating_level = starting_operating_level
         else:
@@ -1467,7 +1471,10 @@ def in_situ_repair(
             raise RuntimeError(f"{self.settings.name} is lost!")
 
         if initial:
-            self.manager.interrupt_system(system)
+            replacement = (
+                request.subassembly_id if request.details.replacement else None
+            )
+            self.manager.interrupt_system(system, replacement=replacement)
         yield self.env.process(
             self.crew_transfer(system, subassembly, request, to_system=True)
         )
@@ -1885,7 +1892,8 @@ def run_tow_to_port(self, request: RepairRequest) -> Generator[Process, None, No
         )
 
         # Turn off the turbine
-        self.manager.interrupt_system(system)
+        replacement = request.subassembly_id if request.details.replacement else None
+        self.manager.interrupt_system(system, replacement=replacement)
 
         # Unmoor the turbine and tow it back to port
         yield self.env.process(self.mooring_connection(system, request, which="unmoor"))

wombat/windfarm/system/cable.py

@@ -169,25 +169,41 @@ def recreate_processes(self) -> None:
         """
         self.processes = dict(self._create_processes())
 
-    def interrupt_processes(self) -> None:
+    def interrupt_processes(self, replacement: str | None = None) -> None:
         """Interrupts all of the running processes within the subassembly except for the
         process associated with failure that triggers the catastrophic failure.
 
         Parameters
         ----------
         subassembly : Subassembly
             The subassembly that should have all processes interrupted.
+        replacement: bool, optional
+            If a subassebly `id` is provided, this indicates the interruption is caused
+            by its replacement event. Defaults to None.
         """
+        cause = "failure"
+        if self.id == replacement:
+            cause = "replacement"
+
         for _, process in self.processes.items():
             try:
-                process.interrupt()
+                process.interrupt(cause=cause)
             except RuntimeError:
                 # This error occurs for the process halting all other processes.
                 pass
 
-    def interrupt_all_subassembly_processes(self) -> None:
-        """Thin wrapper for ``interrupt_processes`` for consistent usage with system."""
-        self.interrupt_processes()
+    def interrupt_all_subassembly_processes(
+        self, replacement: str | None = None
+    ) -> None:
+        """Thin wrapper for ``interrupt_processes`` for consistent usage with system.
+
+        Parameters
+        ----------
+        replacement: bool, optional
+            If a subassebly `id` is provided, this indicates the interruption is caused
+            by its replacement event. Defaults to None.
+        """
+        self.interrupt_processes(replacement=replacement)
 
     def stop_all_upstream_processes(self, failure: Failure | Maintenance) -> None:
         """Stops all upstream turbines and cables from producing power by creating a
@@ -266,6 +282,7 @@ def trigger_request(self, action: Maintenance | Failure):
             The maintenance or failure event that triggers a ``RepairRequest``.
         """
         which = "maintenance" if isinstance(action, Maintenance) else "repair"
+        current_ol = self.operating_level
         self.operating_level *= 1 - action.operation_reduction
 
         # Automatically submit a repair request
@@ -280,6 +297,7 @@ def trigger_request(self, action: Maintenance | Failure):
             cable=True,
             upstream_turbines=self.upstream_nodes,
             upstream_cables=self.upstream_cables,
+            prior_operating_level=current_ol,
         )
         repair_request = self.system.repair_manager.register_request(repair_request)
         self.env.log_action(
@@ -341,14 +359,10 @@ def run_single_maintenance(self, maintenance: Maintenance) -> Generator:
                         yield self.env.timeout(hours_to_next)
                         hours_to_next = 0
                         self.trigger_request(maintenance)
-                    except simpy.Interrupt:
-                        if not self.broken.triggered:
-                            # The subassembly had to restart the maintenance cycle
-                            hours_to_next = 0
-                        else:
-                            # A different process failed, so subtract the elapsed time
-                            # only if it had started to be processed
-                            hours_to_next -= 0 if start == -1 else self.env.now - start
+                    except simpy.Interrupt as i:
+                        if i.cause == "replacement":
+                            return
+                        hours_to_next -= 0 if start == -1 else self.env.now - start
 
     def run_single_failure(self, failure: Failure) -> Generator:
         """Runs a process to trigger one type of failure repair request throughout the
@@ -385,11 +399,7 @@ def run_single_failure(self, failure: Failure) -> Generator:
                         yield self.env.timeout(hours_to_next)
                         hours_to_next = 0
                         self.trigger_request(failure)
-                    except simpy.Interrupt:
-                        if not self.broken.triggered:
-                            # Restart after fixing
-                            hours_to_next = 0
-                        else:
-                            # A different process failed, so subtract the elapsed time
-                            # only if it had started to be processed
-                            hours_to_next -= 0 if start == -1 else self.env.now - start
+                    except simpy.Interrupt as i:
+                        if i.cause == "replacement":
+                            return
+                        hours_to_next -= 0 if start == -1 else self.env.now - start