From a2ef2e3346b58c34e171f1e0d07298cb825614e1 Mon Sep 17 00:00:00 2001 From: Gopala Krishna Date: Sun, 27 Jul 2025 11:06:56 -0400 Subject: [PATCH 1/6] feat: Replace assert statements with explicit exceptions in non-test code --- smart_control/environment/environment.py | 13 +++++--- .../reward/natural_gas_energy_cost.py | 6 ++-- .../reward/setpoint_energy_carbon_regret.py | 8 ++--- smart_control/simulator/boiler.py | 6 +++- .../randomized_arrival_departure_occupancy.py | 20 +++++++++-- smart_control/simulator/simulator.py | 18 ++++++++-- .../simulator/stochastic_occupancy.py | 15 +++++++-- smart_control/simulator/vav.py | 15 +++++++-- smart_control/utils/controller_reader.py | 9 +++-- smart_control/utils/conversion_utils.py | 23 +++++++++---- smart_control/utils/energy_utils.py | 33 ++++++++++++++++--- .../utils/regression_building_utils.py | 14 ++++++-- 12 files changed, 139 insertions(+), 41 deletions(-) diff --git a/smart_control/environment/environment.py b/smart_control/environment/environment.py index fb252059..a5057d6d 100644 --- a/smart_control/environment/environment.py +++ b/smart_control/environment/environment.py @@ -713,9 +713,12 @@ 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 for this method i.e., before building histogram spec." + ) observable_fields = [] @@ -1019,7 +1022,8 @@ 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 +1119,8 @@ 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 diff --git a/smart_control/reward/natural_gas_energy_cost.py b/smart_control/reward/natural_gas_energy_cost.py index f945970b..4833e8ed 100644 --- a/smart_control/reward/natural_gas_energy_cost.py +++ b/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) diff --git a/smart_control/reward/setpoint_energy_carbon_regret.py b/smart_control/reward/setpoint_energy_carbon_regret.py index c840a3d2..8ffa18a6 100644 --- a/smart_control/reward/setpoint_energy_carbon_regret.py +++ b/smart_control/reward/setpoint_energy_carbon_regret.py @@ -124,10 +124,10 @@ def __init__( self._energy_cost_weight = energy_cost_weight self._carbon_emission_weight = carbon_emission_weight - assert ( - self._max_productivity_personhour_usd - > self._min_productivity_personhour_usd - ) + if self._max_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 diff --git a/smart_control/simulator/boiler.py b/smart_control/simulator/boiler.py index 33b89a0b..37a8b68b 100644 --- a/smart_control/simulator/boiler.py +++ b/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 greater than or equal to 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 diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy.py b/smart_control/simulator/randomized_arrival_departure_occupancy.py index fc6948c1..4ee7f9e4 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy.py +++ b/smart_control/simulator/randomized_arrival_departure_occupancy.py @@ -45,12 +45,20 @@ def __init__( random_state: np.random.RandomState, 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( + "Expected arrival/departure hours must be strictly increasing: " + f"earliest_expected_arrival_hour={earliest_expected_arrival_hour}, " + f"latest_expected_arrival_hour={latest_expected_arrival_hour}, " + f"earliest_expected_departure_hour={earliest_expected_departure_hour}, " + f"latest_expected_departure_hour={latest_expected_departure_hour}" + ) self._earliest_expected_arrival_hour = earliest_expected_arrival_hour self._latest_expected_arrival_hour = latest_expected_arrival_hour @@ -76,7 +84,13 @@ 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 probability: " + f"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') # The halfway point is the firts half of the trials. diff --git a/smart_control/simulator/simulator.py b/smart_control/simulator/simulator.py index 5d79b838..6898510e 100644 --- a/smart_control/simulator/simulator.py +++ b/smart_control/simulator/simulator.py @@ -113,7 +113,11 @@ 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 +163,11 @@ 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 @@ -208,7 +216,11 @@ def _get_interior_cv_temp_estimate( neighbor_temps = [temperature_estimates[nx][ny] for nx, ny in neighbors] # Ensure interior CV. - assert len(neighbors) == 4 + if len(neighbors) != 4: + raise ValueError( + f"Expected 4 neighbors for an interior CV, but found {len(neighbors)} " + f"at coordinates {cv_coordinates}. This indicates an invalid building structure." + ) alpha = conductivity / density / heat_capacity diff --git a/smart_control/simulator/stochastic_occupancy.py b/smart_control/simulator/stochastic_occupancy.py index b270d897..f1b29c5e 100644 --- a/smart_control/simulator/stochastic_occupancy.py +++ b/smart_control/simulator/stochastic_occupancy.py @@ -54,13 +54,22 @@ 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( + "Time bounds must be in chronological order i.e., " + "expected arrival/departure hours must be strictly increasing:" "earliest_expected_arrival_hour < latest_expected_arrival_hour " + "< earliest_expected_departure_hour < 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 diff --git a/smart_control/simulator/vav.py b/smart_control/simulator/vav.py index 27bebd21..4494c94d 100644 --- a/smart_control/simulator/vav.py +++ b/smart_control/simulator/vav.py @@ -108,7 +108,8 @@ def max_air_flow_rate(self) -> float: @max_air_flow_rate.setter def max_air_flow_rate(self, value: float): - assert value > 0 + if value <= 0: + raise ValueError(f"Maximum air flow rate must be greater than 0 (got {value}).") self._max_air_flow_rate = value @property @@ -162,13 +163,21 @@ def compute_zone_supply_temp( supply_air_temp: Temperature in K of input air. input_water_temp: Temperature in K of input water. """ - assert self.damper_setting > 0 - assert self._max_air_flow_rate > 0 + reheat_flow_rate = ( self._reheat_valve_setting * self._reheat_max_water_flow_rate ) air_flow_rate = self._damper_setting * self._max_air_flow_rate + # This single check replaces the two individual asserts + # It directly ensures air_flow_rate is positive to avoid ZeroDivisionError + if air_flow_rate <= 0: + raise ValueError( + "Calculated air flow rate must be greater than 0 to compute zone supply temperature. " + f"Current damper_setting={self.damper_setting} and max_air_flow_rate={self._max_air_flow_rate} " + "resulted in a non-positive air flow rate." + ) + heat_difference = ( constants.AIR_HEAT_CAPACITY * air_flow_rate - constants.WATER_HEAT_CAPACITY * reheat_flow_rate diff --git a/smart_control/utils/controller_reader.py b/smart_control/utils/controller_reader.py index cf599190..83ef5869 100644 --- a/smart_control/utils/controller_reader.py +++ b/smart_control/utils/controller_reader.py @@ -149,9 +149,12 @@ def _select_shards( def _read_timestamp(filepath: str) -> pd.Timestamp: """Reads the timestamp from the filepath.""" - assert filepath - ts = pd.Timestamp(re.findall(r'\d{4}\.\d{2}\.\d{2}\.\d{2}', filepath)[-1]) - return ts + if not filepath: + raise ValueError("Filepath cannot be empty when reading timestamp.") + matches = re.findall(r'\d{4}\.\d{2}\.\d{2}\.\d{2}', filepath) + if not matches: + raise ValueError(f"Could not extract timestamp from filepath: {filepath!r}") + return pd.Timestamp(matches[-1]) def _between( timestamp: pd.Timestamp, diff --git a/smart_control/utils/conversion_utils.py b/smart_control/utils/conversion_utils.py index 50f22c2f..8017a365 100644 --- a/smart_control/utils/conversion_utils.py +++ b/smart_control/utils/conversion_utils.py @@ -64,22 +64,31 @@ def floor_plan_based_zone_identifier_to_id(identifier: str) -> str: def zone_id_to_coordinates(zone_id: str) -> Tuple[int, int]: - p = r'^zone_id_[(](\d+), (\d+)[)]' - m = re.match(p, zone_id) - if m: - return int(m.group(1)), int(m.group(2)) - raise ValueError('Could not convert zone_id to coordinates!') + # Expect exactly "zone_id_(,)" (optional spaces after comma) + m = re.match(r'^zone_id_\((\d+),\s*(\d+)\)$', zone_id) + if not m: + raise ValueError( + f"Invalid zone_id format: {zone_id!r}. " + "Expected 'zone_id_(,)'" + ) + return int(m.group(1)), int(m.group(2)) def normalize_dow(dow: int) -> float: """Returns a normalized day of week, mapping [0, 6] to [-1., 1.].""" - assert dow <= 6 and dow >= 0 + if not (0 <= dow <= 6): # More Pythonic way to express range check + raise ValueError( + f"Day of week (dow) must be within the range [0, 6] (got {dow})." + ) return (float(dow) - 3.0) / 3.0 def normalize_hod(hod: int) -> float: """Returns a normlized hour of day, mapping [0,23] to [-1., 1.].""" - assert hod <= 23 and hod >= 0 + if not (0 <= hod <= 23): # More Pythonic way to express range check + raise ValueError( + f"Hour of day (hod) must be within the range [0, 23] (got {hod})." + ) return (float(hod) - 11.5) / 11.5 diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index e2a9fd87..e066c10c 100644 --- a/smart_control/utils/energy_utils.py +++ b/smart_control/utils/energy_utils.py @@ -63,7 +63,20 @@ def get_humidity_ratio( Returns: water mass to air mass ratio in kg Water / kg Air """ - assert len(temps) == len(relative_humidities) == len(pressures) + if not (len(temps) == len(relative_humidities) == len(pressures)): + raise ValueError( + "Input sequences (temps, relative_humidities, pressures) must have the same length. " + f"Got lengths: temps={len(temps)}, relative_humidities={len(relative_humidities)}, pressures={len(pressures)}." + ) + + # Sanity-check each RH and pressure + for i, rh in enumerate(relative_humidities): + if not (0.0 < 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}.") + psat = [p / 1000.0 for p in get_water_vapor_partial_pressure(temps)] return [ 0.622 * psat[i] / (pressures[i] / relative_humidities[i] - psat[i]) @@ -97,13 +110,22 @@ def get_air_conditioning_energy_rate( Returns: Thermal power applied to heat the air to supply temp [W] """ - assert ( + # Combined check for all input vector lengths + if not ( len(air_flow_rates) == len(outside_temps) == len(outside_relative_humidities) == len(supply_temps) == len(ambient_pressures) - ), 'All input vectors must be of the same length.' + ): + raise ValueError( + "All input vectors must be of the same length. " + f"Got lengths: air_flow_rates={len(air_flow_rates)}, " + f"outside_temps={len(outside_temps)}, " + f"outside_relative_humidities={len(outside_relative_humidities)}, " + f"supply_temps={len(supply_temps)}, " + f"ambient_pressures={len(ambient_pressures)}." + ) x = get_humidity_ratio( temps=outside_temps, @@ -168,11 +190,12 @@ def get_fan_power( if motor_factor is None: motor_factor = 0.85 - if brake_hp: + # If the caller explicitly passed brake_hp (even if 0.0), honor it + # otherwise fall back to design_hp × motor_factor + if brake_hp is not None: hp = brake_hp else: hp = motor_factor * design_hp - # Fan is operational if the supply_static_pressure > threshold for # supply fan. Exhaust fan doesn't report static pressure, so assume on # when fan_speed_percentage is > 0. diff --git a/smart_control/utils/regression_building_utils.py b/smart_control/utils/regression_building_utils.py index 0d9572a8..2a409c3d 100644 --- a/smart_control/utils/regression_building_utils.py +++ b/smart_control/utils/regression_building_utils.py @@ -101,7 +101,12 @@ def expand_time_features( feature_names = get_time_feature_names(n, label) - assert len(feature_names) == len(sin_component) + len(cos_component) + if len(feature_names) != (len(sin_component) + len(cos_component)): + raise ValueError( + f"Mismatch between number of feature names ({len(feature_names)}) " + f"and combined sine/cosine components ({len(sin_component) + len(cos_component)}). " + "This indicates an internal logic error in feature expansion." + ) return { feature_name: value for feature_name, value in zip( @@ -384,7 +389,12 @@ def get_matching_indexes( input_indexes.append(ts_input) output_indexes.append(ts_output) - assert len(output_indexes) == len(input_indexes) + if len(output_indexes) != len(input_indexes): + raise ValueError( + f"Mismatch in matched input and output index lengths: " + f"input_indexes={len(input_indexes)}, output_indexes={len(output_indexes)}. " + "Matching logic failed to produce equal-length sequences." + ) return input_indexes, output_indexes From e6681b1262e4a30529620abaf79f677c62e3bc6f Mon Sep 17 00:00:00 2001 From: Gopala Krishna Date: Mon, 5 Jan 2026 23:50:43 +0530 Subject: [PATCH 2/6] refactor: replace non-test asserts with ValueError and update tests --- smart_control/environment/environment.py | 12 ++++-- .../reward/natural_gas_energy_cost_test.py | 9 +++++ .../setpoint_energy_carbon_regret_test.py | 24 +++++++++++ smart_control/simulator/boiler.py | 8 ++-- smart_control/simulator/boiler_test.py | 6 +-- .../randomized_arrival_departure_occupancy.py | 12 +++--- ...omized_arrival_departure_occupancy_test.py | 31 ++++++++++++++ .../simulator/stochastic_occupancy.py | 10 +++-- .../simulator/stochastic_occupancy_test.py | 36 +++++++++++++++++ smart_control/simulator/vav.py | 9 ++--- smart_control/simulator/vav_test.py | 22 ++++++++-- smart_control/utils/conversion_utils_test.py | 24 +++++++++++ smart_control/utils/energy_utils.py | 7 ++-- smart_control/utils/energy_utils_test.py | 40 +++++++++++++++++++ 14 files changed, 221 insertions(+), 29 deletions(-) diff --git a/smart_control/environment/environment.py b/smart_control/environment/environment.py index a5057d6d..4a1a4ba4 100644 --- a/smart_control/environment/environment.py +++ b/smart_control/environment/environment.py @@ -717,7 +717,8 @@ def _get_observation_spec_histogram_reducer( if self._observation_histogram_reducer is None: raise ValueError( - "Observation histogram reducer must be configured for this method i.e., before building histogram spec." + "Observation histogram reducer must be configured before building " + "histogram spec." ) observable_fields = [] @@ -1023,7 +1024,10 @@ def _normalized_observation_response_to_observation_map_histogram_reducer( """ if self._observation_histogram_reducer is None: - raise ValueError("Observation histogram reducer must be set before reducing observation response.") + raise ValueError( + "Observation histogram reducer must be set before reducing " + "observation response." + ) feature_tuples = regression_building_utils.get_feature_tuples( normalized_observation_response @@ -1120,7 +1124,9 @@ def _write_summary_reward_response_metrics( def _commit_reward_metrics(self) -> None: """Aggregates and writes reward metrics, and resets accumulator.""" if self._summary_writer is None: - raise ValueError("Summary writer must be initialized before committing reward metrics.") + 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 diff --git a/smart_control/reward/natural_gas_energy_cost_test.py b/smart_control/reward/natural_gas_energy_cost_test.py index 9ada9990..0f7341fe 100644 --- a/smart_control/reward/natural_gas_energy_cost_test.py +++ b/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() diff --git a/smart_control/reward/setpoint_energy_carbon_regret_test.py b/smart_control/reward/setpoint_energy_carbon_regret_test.py index 9c04e5ff..149ab045 100644 --- a/smart_control/reward/setpoint_energy_carbon_regret_test.py +++ b/smart_control/reward/setpoint_energy_carbon_regret_test.py @@ -281,6 +281,30 @@ def _get_test_reward_info( return 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) + + 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. diff --git a/smart_control/simulator/boiler.py b/smart_control/simulator/boiler.py index 37a8b68b..9214edce 100644 --- a/smart_control/simulator/boiler.py +++ b/smart_control/simulator/boiler.py @@ -294,10 +294,10 @@ def compute_thermal_dissipation_rate( """ if water_temp < outside_temp: - raise ValueError( - "Water temperature must be greater than or equal to outside temperature " - f"(got water_temp={water_temp}, outside_temp={outside_temp})" - ) + raise ValueError( + f"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 diff --git a/smart_control/simulator/boiler_test.py b/smart_control/simulator/boiler_test.py index 8b8e76f9..df5733da 100644 --- a/smart_control/simulator/boiler_test.py +++ b/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): diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy.py b/smart_control/simulator/randomized_arrival_departure_occupancy.py index 4ee7f9e4..c1794597 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy.py +++ b/smart_control/simulator/randomized_arrival_departure_occupancy.py @@ -53,11 +53,13 @@ def __init__( < latest_expected_departure_hour ): raise ValueError( - "Expected arrival/departure hours must be strictly increasing: " - f"earliest_expected_arrival_hour={earliest_expected_arrival_hour}, " - f"latest_expected_arrival_hour={latest_expected_arrival_hour}, " - f"earliest_expected_departure_hour={earliest_expected_departure_hour}, " - f"latest_expected_departure_hour={latest_expected_departure_hour}" + "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 diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py b/smart_control/simulator/randomized_arrival_departure_occupancy_test.py index ac12b4a0..400fed90 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py +++ b/smart_control/simulator/randomized_arrival_departure_occupancy_test.py @@ -139,6 +139,37 @@ 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( + 8, 12, 13, 18, step_size, random_state + ) + + with self.assertRaisesRegex( + ValueError, "Start hour must be less than end hour" + ): + occupant._get_event_probability(start_hour=12, end_hour=8) + if __name__ == '__main__': absltest.main() diff --git a/smart_control/simulator/stochastic_occupancy.py b/smart_control/simulator/stochastic_occupancy.py index f1b29c5e..a93239b1 100644 --- a/smart_control/simulator/stochastic_occupancy.py +++ b/smart_control/simulator/stochastic_occupancy.py @@ -62,9 +62,13 @@ def __init__( < latest_expected_departure_hour ): raise ValueError( - "Time bounds must be in chronological order i.e., " - "expected arrival/departure hours must be strictly increasing:" "earliest_expected_arrival_hour < latest_expected_arrival_hour " - "< earliest_expected_departure_hour < latest_expected_departure_hour." + "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 diff --git a/smart_control/simulator/stochastic_occupancy_test.py b/smart_control/simulator/stochastic_occupancy_test.py index 4fd5863c..d880834b 100644 --- a/smart_control/simulator/stochastic_occupancy_test.py +++ b/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() diff --git a/smart_control/simulator/vav.py b/smart_control/simulator/vav.py index 4494c94d..35f4a9f4 100644 --- a/smart_control/simulator/vav.py +++ b/smart_control/simulator/vav.py @@ -169,13 +169,12 @@ def compute_zone_supply_temp( ) air_flow_rate = self._damper_setting * self._max_air_flow_rate - # This single check replaces the two individual asserts - # It directly ensures air_flow_rate is positive to avoid ZeroDivisionError + # Ensure air_flow_rate is positive to avoid ZeroDivisionError if air_flow_rate <= 0: raise ValueError( - "Calculated air flow rate must be greater than 0 to compute zone supply temperature. " - f"Current damper_setting={self.damper_setting} and max_air_flow_rate={self._max_air_flow_rate} " - "resulted in a non-positive air flow rate." + f"Air flow rate must be > 0 to compute zone supply temp. " + f"damper_setting={self.damper_setting}, " + f"max_air_flow_rate={self._max_air_flow_rate}." ) heat_difference = ( diff --git a/smart_control/simulator/vav_test.py b/smart_control/simulator/vav_test.py index 2bd99773..b2c44479 100644 --- a/smart_control/simulator/vav_test.py +++ b/smart_control/simulator/vav_test.py @@ -131,6 +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): + """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 + + with self.assertRaisesRegex( + ValueError, "Maximum air flow rate must be greater than 0" + ): + v.max_air_flow_rate = -0.5 + @parameterized.parameters( (pd.Timestamp('2021-05-09 14:00'), 293, 0.1, 0.0), (pd.Timestamp('2021-05-10 09:00'), 296, 1.0, 0.0), @@ -225,7 +241,7 @@ def test_compute_zone_supply_temp( v.compute_zone_supply_temp(supply_air_temp, input_water_temp), expected ) - def test_compute_zone_supply_temp_asserts_error(self): + def test_compute_zone_supply_temp_raises_value_error(self): reheat_valve_setting = 0.5 max_air_flow_rate = 0.3 reheat_max_water_flow_rate = 0.4 @@ -237,12 +253,12 @@ def test_compute_zone_supply_temp_asserts_error(self): v.reheat_valve_setting = reheat_valve_setting v.damper_setting = 0 - with self.assertRaises(AssertionError): + with self.assertRaises(ValueError): v.compute_zone_supply_temp(supply_air_temp, input_water_temp) v.damper_setting = 0.5 v._max_air_flow_rate = 0 - with self.assertRaises(AssertionError): + with self.assertRaises(ValueError): v.compute_zone_supply_temp(supply_air_temp, input_water_temp) @parameterized.parameters( diff --git a/smart_control/utils/conversion_utils_test.py b/smart_control/utils/conversion_utils_test.py index f01d7596..747abb8b 100644 --- a/smart_control/utils/conversion_utils_test.py +++ b/smart_control/utils/conversion_utils_test.py @@ -46,10 +46,34 @@ 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): + """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) + 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): + """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) + @parameterized.parameters( (pd.Timestamp('2021-09-27 10:00:00-08:00'), 0), (pd.Timestamp('2021-10-10 18:25:00+02:00'), 6.0 / 7.0 * 2 * np.pi), diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index e066c10c..0f68b0a1 100644 --- a/smart_control/utils/energy_utils.py +++ b/smart_control/utils/energy_utils.py @@ -64,9 +64,10 @@ def get_humidity_ratio( Returns: water mass to air mass ratio in kg Water / kg Air """ if not (len(temps) == len(relative_humidities) == len(pressures)): - raise ValueError( - "Input sequences (temps, relative_humidities, pressures) must have the same length. " - f"Got lengths: temps={len(temps)}, relative_humidities={len(relative_humidities)}, pressures={len(pressures)}." + raise ValueError( + f"Input arrays must have equal length. " + f"Got: temps={len(temps)}, relative_humidities={len(relative_humidities)}, " + f"pressures={len(pressures)}." ) # Sanity-check each RH and pressure diff --git a/smart_control/utils/energy_utils_test.py b/smart_control/utils/energy_utils_test.py index 8e5a1ed8..e8b60eb9 100644 --- a/smart_control/utils/energy_utils_test.py +++ b/smart_control/utils/energy_utils_test.py @@ -33,6 +33,33 @@ def test_get_humidity_ratio(self): actual = energy_utils.get_humidity_ratio([293], [0.6], [1.02]) self.assertAlmostEqual(expected, actual[0], 4) + def test_get_humidity_ratio_mismatched_lengths(self): + """ValueError when input arrays have different lengths.""" + with self.assertRaisesRegex(ValueError, "Input arrays must have equal length"): + energy_utils.get_humidity_ratio( + temps=[293, 300], # 2 elements + relative_humidities=[0.6], # 1 element + pressures=[1.02], # 1 element + ) + + def test_get_humidity_ratio_invalid_relative_humidity(self): + """ValueError when relative_humidity is outside (0, 1].""" + with self.assertRaisesRegex( + ValueError, r"relative_humidities\[0\] must be in \[0,1\]" + ): + energy_utils.get_humidity_ratio( + temps=[293], relative_humidities=[1.5], pressures=[1.02] + ) + + def test_get_humidity_ratio_invalid_pressure(self): + """ValueError when pressure <= 0.""" + with self.assertRaisesRegex( + ValueError, r"pressures\[0\] must be greater than 0" + ): + energy_utils.get_humidity_ratio( + temps=[293], relative_humidities=[0.6], pressures=[-1.0] + ) + def test_get_air_conditioning_energy_rate(self): power = energy_utils.get_air_conditioning_energy_rate( air_flow_rates=[0.170], @@ -43,6 +70,19 @@ def test_get_air_conditioning_energy_rate(self): ) self.assertAlmostEqual(18230.6705, power[0], 4) + def test_get_air_conditioning_energy_rate_mismatched_lengths(self): + """ValueError when input vectors have different lengths.""" + with self.assertRaisesRegex( + ValueError, "All input vectors must be of the same length" + ): + energy_utils.get_air_conditioning_energy_rate( + air_flow_rates=[0.170, 0.180], # 2 elements + outside_temps=[15 + 273.0], # 1 element + outside_relative_humidities=[0.75], + supply_temps=[120 + 273.0], + ambient_pressures=[1.025], + ) + @parameterized.named_parameters( ('brake_hp', None, 8.0, 100.0, 0.8, 0.85, 3, 17.904), ('design_hp', 10.0, None, None, None, None, 1, 6.3410), From 6daf53eab9a2fc3c611e9636e5d2ff449f37deef Mon Sep 17 00:00:00 2001 From: Gopala Krishna Date: Tue, 27 Jan 2026 16:43:44 -0500 Subject: [PATCH 3/6] fix: resolve pylint errors (line length, quote consistency, superfluous parens) --- .../reward/natural_gas_energy_cost.py | 2 +- .../reward/natural_gas_energy_cost_test.py | 2 +- .../reward/setpoint_energy_carbon_regret.py | 8 ++++-- .../setpoint_energy_carbon_regret_test.py | 3 +-- smart_control/simulator/boiler.py | 4 +-- .../randomized_arrival_departure_occupancy.py | 12 ++++----- ...omized_arrival_departure_occupancy_test.py | 6 +++-- smart_control/simulator/simulator.py | 15 ++++++----- .../simulator/stochastic_occupancy_test.py | 4 +-- smart_control/simulator/vav.py | 10 ++++--- smart_control/simulator/vav_test.py | 4 +-- smart_control/utils/controller_reader.py | 6 +++-- smart_control/utils/conversion_utils.py | 11 ++++---- smart_control/utils/conversion_utils_test.py | 8 +++--- smart_control/utils/energy_utils.py | 26 +++++++++---------- smart_control/utils/energy_utils_test.py | 10 ++++--- .../utils/regression_building_utils.py | 14 +++++----- 17 files changed, 80 insertions(+), 65 deletions(-) diff --git a/smart_control/reward/natural_gas_energy_cost.py b/smart_control/reward/natural_gas_energy_cost.py index 4833e8ed..e87a2b43 100644 --- a/smart_control/reward/natural_gas_energy_cost.py +++ b/smart_control/reward/natural_gas_energy_cost.py @@ -40,7 +40,7 @@ def __init__( self, gas_price_by_month: Sequence[float] = GAS_PRICE_BY_MONTH_SOURCE ): if len(gas_price_by_month) != 12: - raise ValueError("Gas price per month must have exactly 12 values.") + 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 = ( diff --git a/smart_control/reward/natural_gas_energy_cost_test.py b/smart_control/reward/natural_gas_energy_cost_test.py index 0f7341fe..1aa563e2 100644 --- a/smart_control/reward/natural_gas_energy_cost_test.py +++ b/smart_control/reward/natural_gas_energy_cost_test.py @@ -72,7 +72,7 @@ def test_invalid_carbon_cost(self): 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" + 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 diff --git a/smart_control/reward/setpoint_energy_carbon_regret.py b/smart_control/reward/setpoint_energy_carbon_regret.py index 8ffa18a6..0349bd4d 100644 --- a/smart_control/reward/setpoint_energy_carbon_regret.py +++ b/smart_control/reward/setpoint_energy_carbon_regret.py @@ -124,9 +124,13 @@ def __init__( self._energy_cost_weight = energy_cost_weight self._carbon_emission_weight = carbon_emission_weight - if self._max_productivity_personhour_usd <= self._min_productivity_personhour_usd: + if ( + self._max_productivity_personhour_usd + <= self._min_productivity_personhour_usd + ): raise ValueError( - "Maximum productivity per person-hour must be greater than minimum productivity." + 'Maximum productivity per person-hour must be greater ' + 'than minimum productivity.' ) def compute_reward( diff --git a/smart_control/reward/setpoint_energy_carbon_regret_test.py b/smart_control/reward/setpoint_energy_carbon_regret_test.py index 149ab045..57f51cc1 100644 --- a/smart_control/reward/setpoint_energy_carbon_regret_test.py +++ b/smart_control/reward/setpoint_energy_carbon_regret_test.py @@ -280,7 +280,6 @@ def _get_test_reward_info( return info - def test_invalid_productivity_bounds(self): """ValueError if max_productivity <= min_productivity.""" electricity_cost = TestEnergyCost(0.05, 0.01) @@ -288,7 +287,7 @@ def test_invalid_productivity_bounds(self): with self.assertRaisesRegex( ValueError, - "Maximum productivity per person-hour must be greater than minimum", + 'Maximum productivity per person-hour must be greater than minimum', ): setpoint_energy_carbon_regret.SetpointEnergyCarbonRegretFunction( max_productivity_personhour_usd=100.0, diff --git a/smart_control/simulator/boiler.py b/smart_control/simulator/boiler.py index 9214edce..0667d8a8 100644 --- a/smart_control/simulator/boiler.py +++ b/smart_control/simulator/boiler.py @@ -295,8 +295,8 @@ def compute_thermal_dissipation_rate( if water_temp < outside_temp: raise ValueError( - f"Water temperature must be >= outside temperature. " - f"Got water_temp={water_temp}, outside_temp={outside_temp}." + '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 diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy.py b/smart_control/simulator/randomized_arrival_departure_occupancy.py index 3be93f21..e7446dd9 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy.py +++ b/smart_control/simulator/randomized_arrival_departure_occupancy.py @@ -43,7 +43,7 @@ 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", ): if not ( @@ -89,12 +89,12 @@ def _get_event_probability(self, start_hour, end_hour): if start_hour >= end_hour: raise ValueError( - "Start hour must be less than end hour to calculate event probability: " - f"start_hour={start_hour}, end_hour={end_hour}" + "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- @@ -170,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 diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py b/smart_control/simulator/randomized_arrival_departure_occupancy_test.py index 082cb764..de0f4caa 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py +++ b/smart_control/simulator/randomized_arrival_departure_occupancy_test.py @@ -148,7 +148,7 @@ def test_zone_occupant_invalid_hour_order(self): # latest_arrival >= earliest_departure is invalid with self.assertRaisesRegex( - ValueError, "Arrival and departure hours must be strictly increasing" + ValueError, 'Arrival and departure hours must be strictly increasing' ): randomized_arrival_departure_occupancy.ZoneOccupant( earliest_expected_arrival_hour=8, @@ -167,7 +167,9 @@ def test_get_event_probability_invalid_hours(self): 8, 12, 13, 18, step_size, random_state ) - with self.assertRaisesRegex(ValueError, "Start hour must be less than end hour"): + 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): diff --git a/smart_control/simulator/simulator.py b/smart_control/simulator/simulator.py index 46905f3b..8da44a8b 100644 --- a/smart_control/simulator/simulator.py +++ b/smart_control/simulator/simulator.py @@ -115,8 +115,9 @@ def _get_corner_cv_temp_estimate( # Ensure corner CV. 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." + 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 @@ -165,8 +166,9 @@ def _get_edge_cv_temp_estimate( # Ensure edge CV. 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." + 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 @@ -274,8 +276,9 @@ def _get_interior_cv_temp_estimate( # Ensure interior CV. if len(neighbors) != 4: raise ValueError( - f"Expected 4 neighbors for an interior CV, but found {len(neighbors)} " - f"at coordinates {cv_coordinates}. This indicates an invalid building structure." + '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 diff --git a/smart_control/simulator/stochastic_occupancy_test.py b/smart_control/simulator/stochastic_occupancy_test.py index d880834b..fb17cd70 100644 --- a/smart_control/simulator/stochastic_occupancy_test.py +++ b/smart_control/simulator/stochastic_occupancy_test.py @@ -103,7 +103,7 @@ def test_zone_occupant_invalid_hour_order(self): random_state = np.random.RandomState(seed=SEED) with self.assertRaisesRegex( - ValueError, "Arrival and departure hours must be strictly increasing" + ValueError, 'Arrival and departure hours must be strictly increasing' ): ZoneOccupant( earliest_expected_arrival_hour=8, @@ -121,7 +121,7 @@ def test_zone_occupant_invalid_lunch_hours(self): random_state = np.random.RandomState(seed=SEED) with self.assertRaisesRegex( - ValueError, "lunch_start_hour must be before lunch_end_hour" + ValueError, 'lunch_start_hour must be before lunch_end_hour' ): ZoneOccupant( earliest_expected_arrival_hour=8, diff --git a/smart_control/simulator/vav.py b/smart_control/simulator/vav.py index 35f4a9f4..69f4b87d 100644 --- a/smart_control/simulator/vav.py +++ b/smart_control/simulator/vav.py @@ -109,7 +109,9 @@ def max_air_flow_rate(self) -> float: @max_air_flow_rate.setter def max_air_flow_rate(self, value: float): if value <= 0: - raise ValueError(f"Maximum air flow rate must be greater than 0 (got {value}).") + raise ValueError( + f'Maximum air flow rate must be greater than 0 (got {value}).' + ) self._max_air_flow_rate = value @property @@ -172,9 +174,9 @@ def compute_zone_supply_temp( # Ensure air_flow_rate is positive to avoid ZeroDivisionError if air_flow_rate <= 0: raise ValueError( - f"Air flow rate must be > 0 to compute zone supply temp. " - f"damper_setting={self.damper_setting}, " - f"max_air_flow_rate={self._max_air_flow_rate}." + 'Air flow rate must be > 0 to compute zone supply temp. ' + f'damper_setting={self.damper_setting}, ' + f'max_air_flow_rate={self._max_air_flow_rate}.' ) heat_difference = ( diff --git a/smart_control/simulator/vav_test.py b/smart_control/simulator/vav_test.py index b2c44479..52237923 100644 --- a/smart_control/simulator/vav_test.py +++ b/smart_control/simulator/vav_test.py @@ -138,12 +138,12 @@ def test_max_air_flow_rate_setter_raises_value_error(self): v = vav.Vav(0.6, 0.4, t, b) with self.assertRaisesRegex( - ValueError, "Maximum air flow rate must be greater than 0" + ValueError, 'Maximum air flow rate must be greater than 0' ): v.max_air_flow_rate = 0.0 with self.assertRaisesRegex( - ValueError, "Maximum air flow rate must be greater than 0" + ValueError, 'Maximum air flow rate must be greater than 0' ): v.max_air_flow_rate = -0.5 diff --git a/smart_control/utils/controller_reader.py b/smart_control/utils/controller_reader.py index 83ef5869..34bcbf99 100644 --- a/smart_control/utils/controller_reader.py +++ b/smart_control/utils/controller_reader.py @@ -150,10 +150,12 @@ def _select_shards( def _read_timestamp(filepath: str) -> pd.Timestamp: """Reads the timestamp from the filepath.""" if not filepath: - raise ValueError("Filepath cannot be empty when reading timestamp.") + raise ValueError('Filepath cannot be empty when reading timestamp.') matches = re.findall(r'\d{4}\.\d{2}\.\d{2}\.\d{2}', filepath) if not matches: - raise ValueError(f"Could not extract timestamp from filepath: {filepath!r}") + raise ValueError( + f'Could not extract timestamp from filepath: {filepath!r}' + ) return pd.Timestamp(matches[-1]) def _between( diff --git a/smart_control/utils/conversion_utils.py b/smart_control/utils/conversion_utils.py index 8017a365..8581fa5a 100644 --- a/smart_control/utils/conversion_utils.py +++ b/smart_control/utils/conversion_utils.py @@ -68,26 +68,25 @@ def zone_id_to_coordinates(zone_id: str) -> Tuple[int, int]: m = re.match(r'^zone_id_\((\d+),\s*(\d+)\)$', zone_id) if not m: raise ValueError( - f"Invalid zone_id format: {zone_id!r}. " - "Expected 'zone_id_(,)'" + f"Invalid zone_id format: {zone_id!r}. Expected 'zone_id_(,)'" ) return int(m.group(1)), int(m.group(2)) def normalize_dow(dow: int) -> float: """Returns a normalized day of week, mapping [0, 6] to [-1., 1.].""" - if not (0 <= dow <= 6): # More Pythonic way to express range check + if dow < 0 or dow > 6: raise ValueError( - f"Day of week (dow) must be within the range [0, 6] (got {dow})." + f'Day of week (dow) must be within the range [0, 6] (got {dow}).' ) return (float(dow) - 3.0) / 3.0 def normalize_hod(hod: int) -> float: """Returns a normlized hour of day, mapping [0,23] to [-1., 1.].""" - if not (0 <= hod <= 23): # More Pythonic way to express range check + if hod < 0 or hod > 23: raise ValueError( - f"Hour of day (hod) must be within the range [0, 23] (got {hod})." + f'Hour of day (hod) must be within the range [0, 23] (got {hod}).' ) return (float(hod) - 11.5) / 11.5 diff --git a/smart_control/utils/conversion_utils_test.py b/smart_control/utils/conversion_utils_test.py index 747abb8b..f66437c6 100644 --- a/smart_control/utils/conversion_utils_test.py +++ b/smart_control/utils/conversion_utils_test.py @@ -49,12 +49,12 @@ def test_normalize_hod(self): def test_normalize_hod_invalid(self): """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\]" + 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\]" + ValueError, r'Hour of day \(hod\) must be within the range \[0, 23\]' ): conversion_utils.normalize_hod(-1) @@ -65,12 +65,12 @@ def test_normalize_dow(self): def test_normalize_dow_invalid(self): """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\]" + 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\]" + ValueError, r'Day of week \(dow\) must be within the range \[0, 6\]' ): conversion_utils.normalize_dow(-1) diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index 0f68b0a1..d109b691 100644 --- a/smart_control/utils/energy_utils.py +++ b/smart_control/utils/energy_utils.py @@ -63,20 +63,20 @@ def get_humidity_ratio( Returns: water mass to air mass ratio in kg Water / kg Air """ - if not (len(temps) == len(relative_humidities) == len(pressures)): + if len(temps) != len(relative_humidities) or len(temps) != len(pressures): raise ValueError( - f"Input arrays must have equal length. " - f"Got: temps={len(temps)}, relative_humidities={len(relative_humidities)}, " - f"pressures={len(pressures)}." + 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 not (0.0 < rh <= 1.0): - raise ValueError(f"relative_humidities[{i}] must be in [0,1], got {rh}.") + 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}.") + raise ValueError(f'pressures[{i}] must be greater than 0 (bar), got {p}.') psat = [p / 1000.0 for p in get_water_vapor_partial_pressure(temps)] return [ @@ -120,12 +120,12 @@ def get_air_conditioning_energy_rate( == len(ambient_pressures) ): raise ValueError( - "All input vectors must be of the same length. " - f"Got lengths: air_flow_rates={len(air_flow_rates)}, " - f"outside_temps={len(outside_temps)}, " - f"outside_relative_humidities={len(outside_relative_humidities)}, " - f"supply_temps={len(supply_temps)}, " - f"ambient_pressures={len(ambient_pressures)}." + 'All input vectors must be of the same length. ' + f'Got lengths: air_flow_rates={len(air_flow_rates)}, ' + f'outside_temps={len(outside_temps)}, ' + f'outside_relative_humidities={len(outside_relative_humidities)}, ' + f'supply_temps={len(supply_temps)}, ' + f'ambient_pressures={len(ambient_pressures)}.' ) x = get_humidity_ratio( diff --git a/smart_control/utils/energy_utils_test.py b/smart_control/utils/energy_utils_test.py index e8b60eb9..be277845 100644 --- a/smart_control/utils/energy_utils_test.py +++ b/smart_control/utils/energy_utils_test.py @@ -35,7 +35,9 @@ def test_get_humidity_ratio(self): def test_get_humidity_ratio_mismatched_lengths(self): """ValueError when input arrays have different lengths.""" - with self.assertRaisesRegex(ValueError, "Input arrays must have equal length"): + with self.assertRaisesRegex( + ValueError, 'Input arrays must have equal length' + ): energy_utils.get_humidity_ratio( temps=[293, 300], # 2 elements relative_humidities=[0.6], # 1 element @@ -45,7 +47,7 @@ def test_get_humidity_ratio_mismatched_lengths(self): def test_get_humidity_ratio_invalid_relative_humidity(self): """ValueError when relative_humidity is outside (0, 1].""" with self.assertRaisesRegex( - ValueError, r"relative_humidities\[0\] must be in \[0,1\]" + ValueError, r'relative_humidities\[0\] must be in \[0,1\]' ): energy_utils.get_humidity_ratio( temps=[293], relative_humidities=[1.5], pressures=[1.02] @@ -54,7 +56,7 @@ def test_get_humidity_ratio_invalid_relative_humidity(self): def test_get_humidity_ratio_invalid_pressure(self): """ValueError when pressure <= 0.""" with self.assertRaisesRegex( - ValueError, r"pressures\[0\] must be greater than 0" + ValueError, r'pressures\[0\] must be greater than 0' ): energy_utils.get_humidity_ratio( temps=[293], relative_humidities=[0.6], pressures=[-1.0] @@ -73,7 +75,7 @@ def test_get_air_conditioning_energy_rate(self): def test_get_air_conditioning_energy_rate_mismatched_lengths(self): """ValueError when input vectors have different lengths.""" with self.assertRaisesRegex( - ValueError, "All input vectors must be of the same length" + ValueError, 'All input vectors must be of the same length' ): energy_utils.get_air_conditioning_energy_rate( air_flow_rates=[0.170, 0.180], # 2 elements diff --git a/smart_control/utils/regression_building_utils.py b/smart_control/utils/regression_building_utils.py index 2a409c3d..edd52e49 100644 --- a/smart_control/utils/regression_building_utils.py +++ b/smart_control/utils/regression_building_utils.py @@ -103,9 +103,10 @@ def expand_time_features( if len(feature_names) != (len(sin_component) + len(cos_component)): raise ValueError( - f"Mismatch between number of feature names ({len(feature_names)}) " - f"and combined sine/cosine components ({len(sin_component) + len(cos_component)}). " - "This indicates an internal logic error in feature expansion." + f'Mismatch between number of feature names ({len(feature_names)}) ' + 'and combined sine/cosine components ' + f'({len(sin_component) + len(cos_component)}). ' + 'This indicates an internal logic error in feature expansion.' ) return { feature_name: value @@ -391,9 +392,10 @@ def get_matching_indexes( if len(output_indexes) != len(input_indexes): raise ValueError( - f"Mismatch in matched input and output index lengths: " - f"input_indexes={len(input_indexes)}, output_indexes={len(output_indexes)}. " - "Matching logic failed to produce equal-length sequences." + 'Mismatch in matched input and output index lengths: ' + f'input_indexes={len(input_indexes)}, ' + f'output_indexes={len(output_indexes)}. ' + 'Matching logic failed to produce equal-length sequences.' ) return input_indexes, output_indexes From 0e246590b6950e37b6e5038168227453a6a73bfb Mon Sep 17 00:00:00 2001 From: Gopala Krishna Date: Wed, 28 Jan 2026 23:38:06 -0500 Subject: [PATCH 4/6] refactor: address review comments - use parameterized tests, named params, numpy methods --- .../setpoint_energy_carbon_regret_test.py | 4 +- ...omized_arrival_departure_occupancy_test.py | 7 +++- smart_control/simulator/vav.py | 11 ++++++ smart_control/simulator/vav_test.py | 17 +++++---- smart_control/utils/conversion_utils_test.py | 20 +++------- smart_control/utils/energy_utils.py | 28 ++++++++++---- smart_control/utils/energy_utils_test.py | 37 ++++++++++++------- 7 files changed, 77 insertions(+), 47 deletions(-) diff --git a/smart_control/reward/setpoint_energy_carbon_regret_test.py b/smart_control/reward/setpoint_energy_carbon_regret_test.py index 57f51cc1..9edbd1a4 100644 --- a/smart_control/reward/setpoint_energy_carbon_regret_test.py +++ b/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, diff --git a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py b/smart_control/simulator/randomized_arrival_departure_occupancy_test.py index de0f4caa..82e57f5a 100644 --- a/smart_control/simulator/randomized_arrival_departure_occupancy_test.py +++ b/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( diff --git a/smart_control/simulator/vav.py b/smart_control/simulator/vav.py index 69f4b87d..980579b3 100644 --- a/smart_control/simulator/vav.py +++ b/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 ) diff --git a/smart_control/simulator/vav_test.py b/smart_control/simulator/vav_test.py index 52237923..3b69e2e5 100644 --- a/smart_control/simulator/vav_test.py +++ b/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), diff --git a/smart_control/utils/conversion_utils_test.py b/smart_control/utils/conversion_utils_test.py index f66437c6..9c3beead 100644 --- a/smart_control/utils/conversion_utils_test.py +++ b/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), diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index d109b691..9ce01a82 100644 --- a/smart_control/utils/energy_utils.py +++ b/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 [ diff --git a/smart_control/utils/energy_utils_test.py b/smart_control/utils/energy_utils_test.py index be277845..1d5e6a79 100644 --- a/smart_control/utils/energy_utils_test.py +++ b/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], ) From b228dcba0d4beee2e66377be461afc1e1114c113 Mon Sep 17 00:00:00 2001 From: Gopala Krishna Date: Thu, 29 Jan 2026 22:25:07 -0500 Subject: [PATCH 5/6] refactor: simplify validation using np.any() and remove invalid values from error messages --- smart_control/utils/energy_utils.py | 18 +++++----------- smart_control/utils/energy_utils_test.py | 27 +++++++++--------------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index 9ce01a82..34848582 100644 --- a/smart_control/utils/energy_utils.py +++ b/smart_control/utils/energy_utils.py @@ -74,21 +74,13 @@ def get_humidity_ratio( relative_humidities_array = np.array(relative_humidities) pressures_array = np.array(pressures) - invalid_rh_indices = np.where( + if np.any( (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]}.' - ) + ): + raise ValueError('Relative humidities must be in the range (0, 1].') - 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]}.' - ) + if np.any(pressures_array <= 0.0): + raise ValueError('Pressures must be greater than 0 (bar).') psat = [p / 1000.0 for p in get_water_vapor_partial_pressure(temps)] return [ diff --git a/smart_control/utils/energy_utils_test.py b/smart_control/utils/energy_utils_test.py index 1d5e6a79..3a17a7c5 100644 --- a/smart_control/utils/energy_utils_test.py +++ b/smart_control/utils/energy_utils_test.py @@ -44,29 +44,22 @@ def test_get_humidity_ratio_mismatched_lengths(self): pressures=[1.02], # 1 element ) - @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 + @parameterized.parameters(1.5, 0.0, -0.1) + def test_get_humidity_ratio_invalid_relative_humidity(self, invalid_rh): """ValueError when relative_humidity is outside (0, 1].""" - with self.assertRaisesRegex(ValueError, expected_pattern): + with self.assertRaisesRegex( + ValueError, r'Relative humidities must be in the range \(0, 1\]' + ): energy_utils.get_humidity_ratio( temps=[293], relative_humidities=[invalid_rh], pressures=[1.02] ) - @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 - ): + @parameterized.parameters(-1.0, 0.0) + def test_get_humidity_ratio_invalid_pressure(self, invalid_pressure): """ValueError when pressure <= 0.""" - with self.assertRaisesRegex(ValueError, expected_pattern): + with self.assertRaisesRegex( + ValueError, r'Pressures must be greater than 0' + ): energy_utils.get_humidity_ratio( temps=[293], relative_humidities=[0.6], pressures=[invalid_pressure] ) From 74f27f6b79bcb67db1e49c6e9825a971d0ad8fba Mon Sep 17 00:00:00 2001 From: Michael Rossetti Date: Fri, 30 Jan 2026 11:19:18 -0500 Subject: [PATCH 6/6] shorter variable name for improved readability --- smart_control/utils/energy_utils.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/smart_control/utils/energy_utils.py b/smart_control/utils/energy_utils.py index 34848582..b5abe0e1 100644 --- a/smart_control/utils/energy_utils.py +++ b/smart_control/utils/energy_utils.py @@ -71,14 +71,11 @@ def get_humidity_ratio( ) # Sanity-check each RH and pressure using numpy - relative_humidities_array = np.array(relative_humidities) - pressures_array = np.array(pressures) - - if np.any( - (relative_humidities_array <= 0.0) | (relative_humidities_array > 1.0) - ): + humidities_array = np.array(relative_humidities) + if np.any((humidities_array <= 0.0) | (humidities_array > 1.0)): raise ValueError('Relative humidities must be in the range (0, 1].') + pressures_array = np.array(pressures) if np.any(pressures_array <= 0.0): raise ValueError('Pressures must be greater than 0 (bar).')