refactor: address review comments - use parameterized tests, named params, numpy methods

Author: igopalakrishna

smart_control/reward/setpoint_energy_carbon_regret_test.py

@@ -282,8 +282,8 @@ def _get_test_reward_info(
 
   def test_invalid_productivity_bounds(self):
     """ValueError if max_productivity <= min_productivity."""
-    electricity_cost = TestEnergyCost(0.05, 0.01)
-    natural_gas_cost = TestEnergyCost(0.05, 0.01)
+    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,

smart_control/simulator/randomized_arrival_departure_occupancy_test.py

@@ -164,7 +164,12 @@ def test_get_event_probability_invalid_hours(self):
     random_state = np.random.RandomState(seed=55213)
     step_size = pd.Timedelta(5, unit='minute')
     occupant = randomized_arrival_departure_occupancy.ZoneOccupant(
-        8, 12, 13, 18, step_size, random_state
+        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(

smart_control/simulator/vav.py

@@ -166,6 +166,17 @@ def compute_zone_supply_temp(
       input_water_temp: Temperature in K of input water.
     """
 
+    # Ensure damper_setting and max_air_flow_rate are positive
+    if self.damper_setting <= 0:
+      raise ValueError(
+          f'Damper setting must be greater than 0, got {self.damper_setting}.'
+      )
+    if self._max_air_flow_rate <= 0:
+      raise ValueError(
+          'Maximum air flow rate must be greater than 0, '
+          f'got {self._max_air_flow_rate}.'
+      )
+
     reheat_flow_rate = (
         self._reheat_valve_setting * self._reheat_max_water_flow_rate
     )

smart_control/simulator/vav_test.py

@@ -131,21 +131,22 @@ def test_setters_raise_error(self):
     with self.assertRaises(ValueError):
       v.damper_setting = -0.1
 
-  def test_max_air_flow_rate_setter_raises_value_error(self):
+  @parameterized.parameters(0.0, -0.5)
+  def test_max_air_flow_rate_setter_raises_value_error(self, invalid_value):
     """ValueError when max_air_flow_rate is set to 0 or negative."""
     t = _get_default_thermostat()
     b = _get_default_boiler()
-    v = vav.Vav(0.6, 0.4, t, b)
-
-    with self.assertRaisesRegex(
-        ValueError, 'Maximum air flow rate must be greater than 0'
-    ):
-      v.max_air_flow_rate = 0.0
+    v = vav.Vav(
+        max_air_flow_rate=0.6,
+        reheat_max_water_flow_rate=0.4,
+        therm=t,
+        boiler=b,
+    )
 
     with self.assertRaisesRegex(
         ValueError, 'Maximum air flow rate must be greater than 0'
     ):
-      v.max_air_flow_rate = -0.5
+      v.max_air_flow_rate = invalid_value
 
   @parameterized.parameters(
       (pd.Timestamp('2021-05-09 14:00'), 293, 0.1, 0.0),

smart_control/utils/conversion_utils_test.py

@@ -46,33 +46,25 @@ def test_normalize_hod(self):
     self.assertEqual(conversion_utils.normalize_hod(0), -1.0)
     self.assertEqual(conversion_utils.normalize_hod(23), 1.0)
 
-  def test_normalize_hod_invalid(self):
+  @parameterized.parameters(24, -1)
+  def test_normalize_hod_invalid_raises_error(self, invalid_hod):
     """ValueError when hour of day is outside [0, 23]."""
     with self.assertRaisesRegex(
         ValueError, r'Hour of day \(hod\) must be within the range \[0, 23\]'
     ):
-      conversion_utils.normalize_hod(24)
-
-    with self.assertRaisesRegex(
-        ValueError, r'Hour of day \(hod\) must be within the range \[0, 23\]'
-    ):
-      conversion_utils.normalize_hod(-1)
+      conversion_utils.normalize_hod(invalid_hod)
 
   def test_normalize_dow(self):
     self.assertEqual(conversion_utils.normalize_dow(0), -1.0)
     self.assertEqual(conversion_utils.normalize_dow(6), 1.0)
 
-  def test_normalize_dow_invalid(self):
+  @parameterized.parameters(7, -1)
+  def test_normalize_dow_invalid_raises_error(self, invalid_dow):
     """ValueError when day of week is outside [0, 6]."""
     with self.assertRaisesRegex(
         ValueError, r'Day of week \(dow\) must be within the range \[0, 6\]'
     ):
-      conversion_utils.normalize_dow(7)
-
-    with self.assertRaisesRegex(
-        ValueError, r'Day of week \(dow\) must be within the range \[0, 6\]'
-    ):
-      conversion_utils.normalize_dow(-1)
+      conversion_utils.normalize_dow(invalid_dow)
 
   @parameterized.parameters(
       (pd.Timestamp('2021-09-27 10:00:00-08:00'), 0),

smart_control/utils/energy_utils.py

@@ -63,20 +63,32 @@ def get_humidity_ratio(
 
   Returns: water mass to air mass ratio in kg Water / kg Air
   """
-  if len(temps) != len(relative_humidities) or len(temps) != len(pressures):
+  if not (len(temps) == len(relative_humidities) == len(pressures)):  # pylint: disable=superfluous-parens
     raise ValueError(
         f'Input arrays must have equal length. Got: temps={len(temps)}, '
         f'relative_humidities={len(relative_humidities)}, '
         f'pressures={len(pressures)}.'
     )
 
-  # Sanity-check each RH and pressure
-  for i, rh in enumerate(relative_humidities):
-    if rh <= 0.0 or rh > 1.0:
-      raise ValueError(f'relative_humidities[{i}] must be in [0,1], got {rh}.')
-  for i, p in enumerate(pressures):
-    if p <= 0.0:
-      raise ValueError(f'pressures[{i}] must be greater than 0 (bar), got {p}.')
+  # Sanity-check each RH and pressure using numpy
+  relative_humidities_array = np.array(relative_humidities)
+  pressures_array = np.array(pressures)
+
+  invalid_rh_indices = np.where(
+      (relative_humidities_array <= 0.0) | (relative_humidities_array > 1.0)
+  )[0]
+  if len(invalid_rh_indices) > 0:
+    i = invalid_rh_indices[0]
+    raise ValueError(
+        f'Relative humidities must be in (0,1], got {relative_humidities[i]}.'
+    )
+
+  invalid_p_indices = np.where(pressures_array <= 0.0)[0]
+  if len(invalid_p_indices) > 0:
+    i = invalid_p_indices[0]
+    raise ValueError(
+        f'Pressures must be greater than 0 (bar), got {pressures[i]}.'
+    )
 
   psat = [p / 1000.0 for p in get_water_vapor_partial_pressure(temps)]
   return [

smart_control/utils/energy_utils_test.py

@@ -44,30 +44,39 @@ def test_get_humidity_ratio_mismatched_lengths(self):
           pressures=[1.02],  # 1 element
       )
 
-  def test_get_humidity_ratio_invalid_relative_humidity(self):
+  @parameterized.parameters(
+      (1.5, r'Relative humidities must be in \(0,1\]'),
+      (0.0, r'Relative humidities must be in \(0,1\]'),
+      (-0.1, r'Relative humidities must be in \(0,1\]'),
+  )
+  def test_get_humidity_ratio_invalid_relative_humidity(
+      self, invalid_rh, expected_pattern
+  ):  # pylint: disable=line-too-long
     """ValueError when relative_humidity is outside (0, 1]."""
-    with self.assertRaisesRegex(
-        ValueError, r'relative_humidities\[0\] must be in \[0,1\]'
-    ):
+    with self.assertRaisesRegex(ValueError, expected_pattern):
       energy_utils.get_humidity_ratio(
-          temps=[293], relative_humidities=[1.5], pressures=[1.02]
+          temps=[293], relative_humidities=[invalid_rh], pressures=[1.02]
       )
 
-  def test_get_humidity_ratio_invalid_pressure(self):
+  @parameterized.parameters(
+      (-1.0, r'Pressures must be greater than 0'),
+      (0.0, r'Pressures must be greater than 0'),
+  )
+  def test_get_humidity_ratio_invalid_pressure(
+      self, invalid_pressure, expected_pattern
+  ):
     """ValueError when pressure <= 0."""
-    with self.assertRaisesRegex(
-        ValueError, r'pressures\[0\] must be greater than 0'
-    ):
+    with self.assertRaisesRegex(ValueError, expected_pattern):
       energy_utils.get_humidity_ratio(
-          temps=[293], relative_humidities=[0.6], pressures=[-1.0]
+          temps=[293], relative_humidities=[0.6], pressures=[invalid_pressure]
       )
 
   def test_get_air_conditioning_energy_rate(self):
     power = energy_utils.get_air_conditioning_energy_rate(
         air_flow_rates=[0.170],
-        outside_temps=[15 + 273.0],
+        outside_temps=[288],
         outside_relative_humidities=[0.75],
-        supply_temps=[120 + 273.0],
+        supply_temps=[393],
         ambient_pressures=[1.025],
     )
     self.assertAlmostEqual(18230.6705, power[0], 4)
@@ -79,9 +88,9 @@ def test_get_air_conditioning_energy_rate_mismatched_lengths(self):
     ):
       energy_utils.get_air_conditioning_energy_rate(
           air_flow_rates=[0.170, 0.180],  # 2 elements
-          outside_temps=[15 + 273.0],  # 1 element
+          outside_temps=[288],  # 1 element
           outside_relative_humidities=[0.75],
-          supply_temps=[120 + 273.0],
+          supply_temps=[393],
           ambient_pressures=[1.025],
       )