Replace Non-test Assertions with Value Errors

smart_control/environment/environment.py

@@ -713,9 +713,13 @@ def _get_observation_spec(
   def _get_observation_spec_histogram_reducer(
       self, devices: Sequence[DeviceInfo]
   ) -> tuple[types.ArraySpec, Sequence[str]]:
-    """Returns an observation spec and a list of field names as histogram."""
+    """Returns an observation spec and a list of field names as histogram"""
 
-    assert self._observation_histogram_reducer is not None
+    if self._observation_histogram_reducer is None:
+      raise ValueError(
+          "Observation histogram reducer must be configured before building "
+          "histogram spec."
+      )
 
     observable_fields = []
 
@@ -1019,7 +1023,11 @@ def _normalized_observation_response_to_observation_map_histogram_reducer(
       Dict of (device, field): measurement
     """
 
-    assert self._observation_histogram_reducer is not None
+    if self._observation_histogram_reducer is None:
+      raise ValueError(
+          "Observation histogram reducer must be set before reducing "
+          "observation response."
+      )
 
     feature_tuples = regression_building_utils.get_feature_tuples(
         normalized_observation_response
@@ -1115,7 +1123,10 @@ def _write_summary_reward_response_metrics(
 
   def _commit_reward_metrics(self) -> None:
     """Aggregates and writes reward metrics, and resets accumulator."""
-    assert self._summary_writer is not None
+    if self._summary_writer is None:
+      raise ValueError(
+          "Summary writer must be initialized before committing reward metrics."
+      )
 
     if self._global_step_count % self._metrics_reporting_interval == 0:
       with (  # pylint: disable=not-context-manager # TODO: consider adding comments to provide more context

smart_control/reward/natural_gas_energy_cost.py

@@ -39,9 +39,9 @@ class NaturalGasEnergyCost(BaseEnergyCost):
   def __init__(
       self, gas_price_by_month: Sequence[float] = GAS_PRICE_BY_MONTH_SOURCE
   ):
-    assert (
-        len(gas_price_by_month) == 12
-    ), 'Gas price per month must have exactly 12 values.'
+    if len(gas_price_by_month) != 12:
+      raise ValueError('Gas price per month must have exactly 12 values.')
+
     # Convert the month-by-month gas price from $/1000 cubic feet to $/Joule.
     self._month_gas_price = (
         np.array(gas_price_by_month)

smart_control/reward/natural_gas_energy_cost_test.py

@@ -69,6 +69,15 @@ def test_invalid_carbon_cost(self):
     energy_rate = -1.0
     self.assertEqual(0.0, cost.cost(start_time, end_time, energy_rate))
 
+  def test_invalid_gas_price_by_month_length(self):
+    """ValueError if gas_price_by_month does not have exactly 12 values."""
+    with self.assertRaisesRegex(
+        ValueError, 'Gas price per month must have exactly 12 values'
+    ):
+      natural_gas_energy_cost.NaturalGasEnergyCost(
+          gas_price_by_month=[1.0, 2.0, 3.0]  # Only 3 values instead of 12
+      )
+
 
 if __name__ == '__main__':
   absltest.main()

smart_control/reward/setpoint_energy_carbon_regret.py

@@ -124,10 +124,14 @@ def __init__(
     self._energy_cost_weight = energy_cost_weight
     self._carbon_emission_weight = carbon_emission_weight
 
-    assert (
+    if (
         self._max_productivity_personhour_usd
-        > self._min_productivity_personhour_usd
-    )
+        <= self._min_productivity_personhour_usd
+    ):
+      raise ValueError(
+          'Maximum productivity per person-hour must be greater '
+          'than minimum productivity.'
+      )
 
   def compute_reward(
       self, reward_info: smart_control_reward_pb2.RewardInfo

smart_control/reward/setpoint_energy_carbon_regret_test.py

@@ -280,6 +280,29 @@ def _get_test_reward_info(
 
     return info
 
+  def test_invalid_productivity_bounds(self):
+    """ValueError if max_productivity <= min_productivity."""
+    electricity_cost = TestEnergyCost(usd_per_kwh=0.05, kg_per_kwh=0.01)
+    natural_gas_cost = TestEnergyCost(usd_per_kwh=0.05, kg_per_kwh=0.01)
+
+    with self.assertRaisesRegex(
+        ValueError,
+        'Maximum productivity per person-hour must be greater than minimum',
+    ):
+      setpoint_energy_carbon_regret.SetpointEnergyCarbonRegretFunction(
+          max_productivity_personhour_usd=100.0,
+          min_productivity_personhour_usd=200.0,  # min > max: invalid
+          max_electricity_rate=10000.0,
+          max_natural_gas_rate=10000.0,
+          productivity_midpoint_delta=1.5,
+          productivity_decay_stiffness=4.3,
+          electricity_energy_cost=electricity_cost,
+          natural_gas_energy_cost=natural_gas_cost,
+          productivity_weight=1.0,
+          energy_cost_weight=1.0,
+          carbon_emission_weight=1.0,
+      )
+
 
 class TestEnergyCost(BaseEnergyCost):
   """Calculates energy cost and carbon emissions based on fixed rates.

smart_control/simulator/boiler.py

@@ -293,7 +293,11 @@ def compute_thermal_dissipation_rate(
       thermal loss rate of the tank in Watts
     """
 
-    assert water_temp >= outside_temp
+    if water_temp < outside_temp:
+      raise ValueError(
+          'Water temperature must be >= outside temperature. '
+          f'Got water_temp={water_temp}, outside_temp={outside_temp}.'
+      )
     delta_temp = water_temp - outside_temp
     numerator = self._tank_length * 2.0 * np.pi * delta_temp
     interior_radius = self._tank_radius

smart_control/simulator/boiler_test.py

@@ -161,7 +161,7 @@ def test_compute_thermal_energy_rate(
         places=3,
     )
 
-  def test_compute_thermal_energy_rate_raises_assertion_error(self):
+  def test_compute_thermal_energy_rate_raises_value_error(self):
     return_water_temp = 200
     total_flow_rate = 0.5
     reheat_water_setpoint = 100
@@ -177,7 +177,7 @@ def test_compute_thermal_energy_rate_raises_assertion_error(self):
 
     b.add_demand(total_flow_rate)
 
-    with self.assertRaises(AssertionError):
+    with self.assertRaises(ValueError):
       _ = b.compute_thermal_energy_rate(return_water_temp, outside_temp)
 
   @parameterized.parameters(
@@ -426,7 +426,7 @@ def test_compute_thermal_dissipation_rate_zero(self):
 
   def test_compute_thermal_dissipation_rate_invalid(self):
     b = self.get_default_boiler()
-    with self.assertRaises(AssertionError):
+    with self.assertRaises(ValueError):
       _ = b.compute_thermal_dissipation_rate(240.0, 290.0)
 
   def test_action_field_names(self):

smart_control/simulator/randomized_arrival_departure_occupancy.py

@@ -43,14 +43,24 @@ def __init__(
       latest_expected_departure_hour: int,
       step_size: pd.Timedelta,
       random_state: np.random.RandomState,
-      time_zone: Union[datetime.tzinfo, str] = 'UTC',
+      time_zone: Union[datetime.tzinfo, str] = "UTC",
   ):
-    assert (
+
+    if not (
         earliest_expected_arrival_hour
         < latest_expected_arrival_hour
         < earliest_expected_departure_hour
         < latest_expected_departure_hour
-    )
+    ):
+      raise ValueError(
+          "Arrival and departure hours must be strictly increasing: "
+          "earliest_arrival < latest_arrival < earliest_departure < "
+          "latest_departure. "
+          f"Got: {earliest_expected_arrival_hour}, "
+          f"{latest_expected_arrival_hour}, "
+          f"{earliest_expected_departure_hour}, "
+          f"{latest_expected_departure_hour}."
+      )
 
     self._earliest_expected_arrival_hour = earliest_expected_arrival_hour
     self._latest_expected_arrival_hour = latest_expected_arrival_hour
@@ -76,9 +86,15 @@ def _to_local_time(self, timestamp: pd.Timestamp) -> pd.Timestamp:
 
   def _get_event_probability(self, start_hour, end_hour):
     """Returns the probability of an event based on the number of time steps."""
-    assert start_hour < end_hour
+
+    if start_hour >= end_hour:
+      raise ValueError(
+          "Start hour must be less than end hour to calculate event "
+          f"probability: start_hour={start_hour}, end_hour={end_hour}"
+      )
+
     # The window is the number of Bernoulli trials (i.e. tests for arrival).
-    window = pd.Timedelta(end_hour - start_hour, unit='hour')
+    window = pd.Timedelta(end_hour - start_hour, unit="hour")
     # The halfway point is the firts half of the trials.
     n_halfway = window / self._step_size / 2.0
     # We'd like to return the probability of event happening in a single time-
@@ -154,11 +170,11 @@ def __init__(
       latest_expected_departure_hour: int,
       time_step_sec: int,
       seed: Optional[int] = 17321,
-      time_zone: str = 'UTC',
+      time_zone: str = "UTC",
   ):
     self._zone_assignment = zone_assignment
     self._zone_occupants = {}
-    self._step_size = pd.Timedelta(time_step_sec, unit='second')
+    self._step_size = pd.Timedelta(time_step_sec, unit="second")
     self._earliest_expected_arrival_hour = earliest_expected_arrival_hour
     self._latest_expected_arrival_hour = latest_expected_arrival_hour
     self._earliest_expected_departure_hour = earliest_expected_departure_hour

smart_control/simulator/randomized_arrival_departure_occupancy_test.py

@@ -141,6 +141,42 @@ def test_peek(self, tz):
         )
       current_time += pd.Timedelta(5, unit='minute')
 
+  def test_zone_occupant_invalid_hour_order(self):
+    """ValueError when arrival/departure hours are not strictly increasing."""
+    random_state = np.random.RandomState(seed=55213)
+    step_size = pd.Timedelta(5, unit='minute')
+
+    # latest_arrival >= earliest_departure is invalid
+    with self.assertRaisesRegex(
+        ValueError, 'Arrival and departure hours must be strictly increasing'
+    ):
+      randomized_arrival_departure_occupancy.ZoneOccupant(
+          earliest_expected_arrival_hour=8,
+          latest_expected_arrival_hour=14,  # > earliest_departure (13)
+          earliest_expected_departure_hour=13,
+          latest_expected_departure_hour=18,
+          step_size=step_size,
+          random_state=random_state,
+      )
+
+  def test_get_event_probability_invalid_hours(self):
+    """ValueError when start_hour >= end_hour."""
+    random_state = np.random.RandomState(seed=55213)
+    step_size = pd.Timedelta(5, unit='minute')
+    occupant = randomized_arrival_departure_occupancy.ZoneOccupant(
+        earliest_expected_arrival_hour=8,
+        latest_expected_arrival_hour=12,
+        earliest_expected_departure_hour=13,
+        latest_expected_departure_hour=18,
+        step_size=step_size,
+        random_state=random_state,
+    )
+
+    with self.assertRaisesRegex(
+        ValueError, 'Start hour must be less than end hour'
+    ):
+      occupant._get_event_probability(start_hour=12, end_hour=8)
+
   def test_average_zone_occupancy_matches_manual_two_steps(self):
     """average_zone_occupancy should equal the mean of per-step counts."""
     step = pd.Timedelta(minutes=5)
@@ -173,10 +209,7 @@ def test_average_zone_occupancy_matches_manual_two_steps(self):
     manual_avg = sum(manual_counts) / 2.0
 
     result = occ.average_zone_occupancy('zone_0', t0, t1)
-
-    # In the old implementation this would have returned manual_counts[0]
-    # and the assertion would fail. With the fix, it matches the average.
-    assert result == manual_avg
+    self.assertEqual(result, manual_avg)
 
 
 if __name__ == '__main__':

smart_control/simulator/simulator.py

@@ -113,7 +113,12 @@ def _get_corner_cv_temp_estimate(
     neighbor_temps = [temperature_estimates[nx][ny] for nx, ny in neighbors]
 
     # Ensure corner CV.
-    assert len(neighbors) == 2
+    if len(neighbors) != 2:
+      raise ValueError(
+          f'Expected 2 neighbors for a corner CV, but found {len(neighbors)} '
+          f'at coordinates {cv_coordinates}. '
+          'This indicates an invalid building structure.'
+      )
 
     t0 = density * delta_x**2 * heat_capacity / delta_t / 2.0
     retained_heat = t0 * last_temp
@@ -159,7 +164,12 @@ def _get_edge_cv_temp_estimate(
     neighbor_temps = [temperature_estimates[nx][ny] for nx, ny in neighbors]
 
     # Ensure edge CV.
-    assert len(neighbors) == 3
+    if len(neighbors) != 3:
+      raise ValueError(
+          f'Expected 3 neighbors for an edge CV, but found {len(neighbors)} '
+          f'at coordinates {cv_coordinates}. '
+          'This indicates an invalid building structure.'
+      )
 
     t0 = density * delta_x**2 / 2 * heat_capacity / delta_t
     retained_heat = t0 * last_temp
@@ -264,7 +274,12 @@ def _get_interior_cv_temp_estimate(
     neighbors = self.building.neighbors[x][y]
     neighbor_temps = [temperature_estimates[nx][ny] for nx, ny in neighbors]
     # Ensure interior CV.
-    assert len(neighbors) == 4
+    if len(neighbors) != 4:
+      raise ValueError(
+          'Expected 4 neighbors for an interior CV, but found'
+          f' {len(neighbors)} at coordinates {cv_coordinates}. This indicates'
+          ' an invalid building structure.'
+      )
 
     alpha = conductivity / density / heat_capacity
 

smart_control/simulator/stochastic_occupancy.py

@@ -54,13 +54,26 @@ def __init__(
       random_state: np.random.RandomState,
       time_zone: Union[datetime.tzinfo, str] = "UTC",
   ):
-    assert (
+    # Validate that the time bounds are in chronological order
+    if not (
         earliest_expected_arrival_hour
         < latest_expected_arrival_hour
         < earliest_expected_departure_hour
         < latest_expected_departure_hour
-    )
-    assert lunch_start_hour < lunch_end_hour
+    ):
+      raise ValueError(
+          "Arrival and departure hours must be strictly increasing: "
+          "earliest_arrival < latest_arrival < earliest_departure < "
+          "latest_departure. "
+          f"Got: {earliest_expected_arrival_hour}, "
+          f"{latest_expected_arrival_hour}, "
+          f"{earliest_expected_departure_hour}, "
+          f"{latest_expected_departure_hour}."
+      )
+
+    # Validate lunch time bounds
+    if lunch_start_hour >= lunch_end_hour:
+      raise ValueError("lunch_start_hour must be before lunch_end_hour.")
 
     self._earliest_expected_arrival_hour = earliest_expected_arrival_hour
     self._latest_expected_arrival_hour = latest_expected_arrival_hour

smart_control/simulator/stochastic_occupancy_test.py

@@ -98,6 +98,42 @@ def test_peek(self, tz):
         self.assertEqual(OccupancyStateEnum.WORK, state)
       current_time += STEP_SIZE
 
+  def test_zone_occupant_invalid_hour_order(self):
+    """ValueError when arrival/departure hours are not strictly increasing."""
+    random_state = np.random.RandomState(seed=SEED)
+
+    with self.assertRaisesRegex(
+        ValueError, 'Arrival and departure hours must be strictly increasing'
+    ):
+      ZoneOccupant(
+          earliest_expected_arrival_hour=8,
+          latest_expected_arrival_hour=17,  # > earliest_departure (16)
+          earliest_expected_departure_hour=16,
+          latest_expected_departure_hour=18,
+          lunch_start_hour=12,
+          lunch_end_hour=14,
+          step_size=STEP_SIZE,
+          random_state=random_state,
+      )
+
+  def test_zone_occupant_invalid_lunch_hours(self):
+    """ValueError when lunch_start_hour >= lunch_end_hour."""
+    random_state = np.random.RandomState(seed=SEED)
+
+    with self.assertRaisesRegex(
+        ValueError, 'lunch_start_hour must be before lunch_end_hour'
+    ):
+      ZoneOccupant(
+          earliest_expected_arrival_hour=8,
+          latest_expected_arrival_hour=10,
+          earliest_expected_departure_hour=16,
+          latest_expected_departure_hour=18,
+          lunch_start_hour=14,  # >= lunch_end_hour
+          lunch_end_hour=12,
+          step_size=STEP_SIZE,
+          random_state=random_state,
+      )
+
 
 if __name__ == '__main__':
   absltest.main()