Pre-submit Checks: Google-specific (#51)

Update files to make pre-submit checks pass, so we can merge the code into Google.

Addresses all remaining Google-specific linting checks, except for "g-bad-todo" and "g-bad-import-order".

Author: s2t2

smart_control/reinforcement_learning/observers/rendering_observer.py

@@ -44,9 +44,9 @@ class RenderingObserver(Observer):
   def __init__(
       self,
       render_interval_steps: int = 10,
-      env=None,  # TODO: do we want to use `Optional[environment.Environment] = None` here?
-      render_fn: Optional[Callable] = None,  # pylint: disable=g-bare-generic # TODO: use more specific type hint if possible
-      plot_fn: Optional[Callable] = None,  # pylint: disable=g-bare-generic # TODO: use more specific type hint if possible
+      env=None,  # consider: `Optional[environment.Environment] = None`
+      render_fn: Optional[Callable] = None,  # pylint: disable=g-bare-generic
+      plot_fn: Optional[Callable] = None,  # pylint: disable=g-bare-generic
       clear_output_before_render: bool = True,
       time_zone: str = DEFAULT_TIME_ZONE,
       save_path: str = RENDERS_PATH,

smart_control/reinforcement_learning/utils/config.py

@@ -32,7 +32,7 @@
 # Path to the root directory of the project:
 ROOT_DIR = os.path.join(os.path.dirname(__file__), "..", "..", "..")
 # fmt: off
-#pylint: disable=line-too-long
+# pylint: disable=line-too-long
 DATA_PATH = os.path.join(ROOT_DIR, "smart_control", "configs", "resources", "sb1")
 CONFIG_PATH = os.path.join(ROOT_DIR, "smart_control", "configs", "resources", "sb1", "train_sim_configs")
 METRICS_PATH = os.path.join(ROOT_DIR, "smart_control", "reinforcement_learning", "experiment_results", "metrics")

smart_control/reward/base_setpoint_energy_carbon_reward.py

@@ -55,6 +55,18 @@ def compute_reward(
   def _sum_zone_productivities(
       self, energy_reward_info: smart_control_reward_pb2.RewardInfo
   ) -> Tuple[float, float]:
+    """Calculates cumulative productivity and total occupancy across all zones.
+
+    Args:
+      energy_reward_info: A RewardInfo object containing zone-specific
+        information, including setpoint temperatures, zone air temperatures, and
+        average occupancies.
+
+    Returns:
+      A tuple containing:
+        - The cumulative productivity across all zones (float).
+        - The total average occupancy across all zones (float).
+    """
     time_interval_sec = self._get_delta_time_sec(energy_reward_info)
     cumulative_productivity = 0.0
     total_occupancy = 0.0
@@ -63,12 +75,8 @@ def _sum_zone_productivities(
       occupancy = energy_reward_info.zone_reward_infos[zid].average_occupancy
       total_occupancy += occupancy
       cumulative_productivity += self._get_zone_productivity_reward(
-          energy_reward_info.zone_reward_infos[
-              zid
-          ].heating_setpoint_temperature,
-          energy_reward_info.zone_reward_infos[
-              zid
-          ].cooling_setpoint_temperature,
+          energy_reward_info.zone_reward_infos[zid].heating_setpoint_temperature,  # pylint:disable=line-too-long
+          energy_reward_info.zone_reward_infos[zid].cooling_setpoint_temperature,  # pylint:disable=line-too-long
           energy_reward_info.zone_reward_infos[zid].zone_air_temperature,
           time_interval_sec,
           occupancy,

smart_control/reward/base_setpoint_energy_carbon_reward_test.py

@@ -153,7 +153,7 @@ class TestEnergyCost(BaseEnergyCost):
 
   Used for testing purposes.
 
-  TODO: https://github.com/google/sbsim/issues/49 - consider refactoring with similar classes in:
+  TODO: https://github.com/google/sbsim/issues/49 - refactor identical classes:
     smart_control/reward/setpoint_energy_carbon_regret_test.py
     smart_control/reward/setpoint_energy_carbon_reward_test.py
 
@@ -163,7 +163,7 @@ class TestEnergyCost(BaseEnergyCost):
   def __init__(self, usd_per_kwh: float, kg_per_kwh: float):
     # Energy price in USD/Watt second (fixed schedule)
     # To convert denominator units hours to seconds, divide by 3600.0, and to
-    # convert kW to W, divide by 1000. This leaves us with an enegy price
+    # convert kW to W, divide by 1000. This leaves us with an energy price
     # in USD /W /s and carbon rate of kg /W /s.
     self._energy_price = usd_per_kwh / 3600.0 / 1000.0
     self._carbon_rate = kg_per_kwh / 3600.0 / 1000.0

smart_control/reward/setpoint_energy_carbon_regret_test.py

@@ -301,15 +301,15 @@ class TestEnergyCost(BaseEnergyCost):
 
   Used for testing purposes.
 
-  TODO: https://github.com/google/sbsim/issues/49 - consider refactoring with similar classes in:
+  TODO: https://github.com/google/sbsim/issues/49 - refactor identical classes:
     smart_control/reward/base_setpoint_energy_carbon_reward_test.py
     smart_control/reward/setpoint_energy_carbon_reward_test.py
   """
 
   def __init__(self, usd_per_kwh: float, kg_per_kwh: float):
     # Energy price in USD/Watt second (fixed schedule)
     # To convert denominator units hours to seconds, divide by 3600.0, and to
-    # convert kW to W, divide by 1000. This leaves us with an enegy price
+    # convert kW to W, divide by 1000. This leaves us with an energy price
     # in USD /W /s and carbon rate of kg /W /s.
     self._energy_price = usd_per_kwh / 3600.0 / 1000.0
     self._carbon_rate = kg_per_kwh / 3600.0 / 1000.0

smart_control/reward/setpoint_energy_carbon_reward_test.py

@@ -235,15 +235,15 @@ class TestEnergyCost(BaseEnergyCost):
 
   Used for testing purposes.
 
-  TODO: https://github.com/google/sbsim/issues/49 - consider refactoring with similar classes in:
+  TODO: https://github.com/google/sbsim/issues/49 - refactor identical classes:
     smart_control/reward/base_setpoint_energy_carbon_reward_test.py
     smart_control/reward/setpoint_energy_carbon_regret_test.py
   """
 
   def __init__(self, usd_per_kwh: float, kg_per_kwh: float):
     # Energy price in USD/Watt second (fixed schedule)
     # To convert denominator units hours to seconds, divide by 3600.0, and to
-    # convert kW to W, divide by 1000. This leaves us with an enegy price
+    # convert kW to W, divide by 1000. This leaves us with an energy price
     # in USD /W /s and carbon rate of kg /W /s.
     self._energy_price = usd_per_kwh / 3600.0 / 1000.0
     self._carbon_rate = kg_per_kwh / 3600.0 / 1000.0

smart_control/simulator/base_convection_simulator.py

@@ -36,4 +36,8 @@ def apply_convection(
     """Applies convection to the temperature array in place.
 
     Splits up rooms via room_dict.
+
+    Args:
+      room_dict: A dictionary mapping of room coordinates.
+      temp: An array of temperatures.
     """

smart_control/simulator/building.py

@@ -599,7 +599,7 @@ def apply_thermal_power_zone(
 
     The thermal power [W] is applied to zones `zone_x` and `zone_y`.
 
-     Args:
+    Args:
        zone_coordinates: Tuple containing x and y coordinates for zone.
        power: Watts to apply to zone.
     """
@@ -625,9 +625,10 @@ class FloorPlanBasedBuilding(BaseSimulatorBuilding):
       width and length of each room.
     building_shape: 2-Tuple representing the number of rooms in the width and
       length of the building.
+    floor_plan: an np.ndarray representing the building's floor plan.
     temp: The current temp in K of each control volume.
     conductivity: Thermal conductivity in of each control volume W/m/K.
-    heat_capacity: Thermal heat cpacity of each control volume in J/kg/K.
+    heat_capacity: Thermal heat capacity of each control volume in J/kg/K.
     density: Material density in kg/m3 of each control volume.
     input_q: Heat energy applied (sign indicates heating/cooling) at the CV in W
       (J/s).
@@ -871,7 +872,7 @@ def get_zone_temp_stats(self, zone_name: str) -> Tuple[float, float, float]:  #
     return np.min(temps), np.max(temps), np.mean(temps)
 
   def get_zone_average_temps(self) -> Dict[str, Any]:
-    """Returns a dict of zone average temps,
+    """Returns a dict of zone average temps.
 
     The dict is formatted as: {`zone_coordinates`: `temp`}.
     """

smart_control/simulator/randomized_arrival_departure_occupancy_test.py

@@ -23,7 +23,7 @@
 from smart_control.simulator import randomized_arrival_departure_occupancy
 
 # fmt: off
-#pylint: disable=bad-continuation
+# pylint: disable=bad-continuation
 _EXPECTED_ZONE_OCCUPANCIES_PACIFIC = [
   0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0,
   0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0,
@@ -89,8 +89,9 @@
   1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
   1, 1, 1, 1, 1, 1,
 ]
-#pylint: disable=bad-continuation
-#fmt: off
+# pylint: disable=bad-continuation
+# fmt: off
+
 
 class RandomizedArrivalDepartureOccupancyTest(parameterized.TestCase):
 

smart_control/simulator/simulator.py

@@ -492,6 +492,9 @@ def _get_zone_reward_infos(
     """Returns a map of messages with zone data.
 
     This data is used to compute the instantaneous reward.
+
+    Args:
+      occupancy_function: An occupancy function.
     """
     zone_reward_infos = {}
     for (

smart_control/simulator/simulator_flexible_floor_plan.py

@@ -228,6 +228,9 @@ def _get_zone_reward_infos(
     """Returns a map of messages with zone data.
 
     This data is used to compute the instantaneous reward.
+
+    Args:
+      occupancy_function: An occupancy function.
     """
     zone_reward_infos = {}
     for (

smart_control/simulator/stochastic_convection_simulator.py

@@ -68,6 +68,10 @@ def apply_convection(
     """Applies convection to the temperature array given.
 
     Splits up rooms via room_dict.
+
+    Args:
+      room_dict: A dictionary mapping of room coordinates.
+      temp: An array of temperatures.
     """
     p = self._p
     distance = self._distance

smart_control/simulator/stochastic_occupancy.py

@@ -159,11 +159,20 @@ def _occupant_departed(self, timestamp: pd.Timestamp) -> bool:
     return departed
 
   def peek(self, current_time: pd.Timestamp) -> OccupancyStateEnum:
+    """Checks the current occupancy state based on the provided timestamp.
+
+    This method determines the occupancy state (AWAY or WORK) based on
+    the current time, considering workdays, arrival/departure times,
+    and a lunch break.
+
+    Args:
+        current_time: The current timestamp to evaluate.
+
+    Returns:
+        The current `OccupancyStateEnum` (AWAY or WORK).
+    """
     local_timestamp = self._to_local_time(current_time)
-    # print(f"Inside peek: current_time={current_time}")
-    local_time = (
-        local_timestamp.time()
-    )  # Extracts time as a datetime.time object
+    local_time = local_timestamp.time()  # a datetime.time object
     if debug_print:
       print(
           f"Peek called: current_time={current_time},"

smart_control/simulator/tf_simulator.py

@@ -219,7 +219,7 @@ def _cv_corner_type(cv_neighbors: Sequence[tuple[int, int]]) -> CVType:
           corner=CVCornerOrientationType.BOTTOM_RIGHT
       )
     raise ValueError(
-        f"wasn't able to determine which corner the CV {(i,j)} is."
+        f"Wasn't able to determine which corner the CV {(i, j)} is."
     )
 
   def _cv_type_edge_factory(
@@ -242,7 +242,7 @@ def _cv_edge_type(cv_neighbors: Sequence[tuple[int, int]]) -> CVType:
       return edge(CVEdgeOrientationType.LEFT)
     if set([(i - 1, j), (i, j - 1), (i + 1, j)]) == set(cv_neighbors):
       return edge(CVEdgeOrientationType.RIGHT)
-    raise ValueError(f"wasn't able to determine which edge the CV {(i,j)} is.")
+    raise ValueError(f"Wasn't able to determine which edge the CV {(i, j)} is.")
 
   i, j = coords
 
@@ -260,7 +260,7 @@ def _cv_edge_type(cv_neighbors: Sequence[tuple[int, int]]) -> CVType:
       return CVType(position=CVPositionType.INTERIOR)
     case _:
       raise ValueError(
-          f"wasn't able to determine which CV type the CV {(i,j)} is."
+          f"Wasn't able to determine which CV type the CV {(i, j)} is."
       )
 
 

smart_control/utils/plot_utils.py

@@ -19,15 +19,15 @@
 import os
 import pathlib
 
+from matplotlib import cm
 from matplotlib import patches
-import matplotlib.cm as cm
 import matplotlib.dates as mdates
 import matplotlib.pyplot as plt
 from matplotlib.ticker import MaxNLocator
 import numpy as np
 import pandas as pd
 
-K_TO_C = 273.0  # TODO: consider importing and using `int(KELVIN_TO_CELSIUS)` constant here (see https://github.com/google/sbsim/issues/25)
+K_TO_C = 273.0  # TODO: https://github.com/google/sbsim/issues/25 - consider importing and using `int(KELVIN_TO_CELSIUS)` constant here # pylint:disable=line-too-long
 
 
 def get_temp_colors(min_k, max_k):

smart_control/utils/regression_building_utils.py

@@ -216,7 +216,12 @@ def get_action_tuples(
 ) -> Set[Tuple[str, str, str]]:
   """Returns the tuples from ActionResponse.
 
-  The tuples are formatted as (_ACTION_PREFIX, device_id, setpoint).
+  Args:
+    action_response: The ActionResponse from which to extract action tuples.
+
+  Returns:
+    A set of tuples, where each tuple is in the format
+      (`_ACTION_PREFIX`, `device_id`, `setpoint`).
   """
   action_tuples = set()
   for request in action_response.request.single_action_requests:
@@ -433,7 +438,13 @@ def get_device_action_tuples(
 ) -> Sequence[Tuple[str, str, str]]:
   """Converts DeviceInfos into action tuples.
 
-  The tuples are formatted as (_ACTION_PREFIX, device, setpoint).
+  Args:
+    devices: A sequence of DeviceInfo objects.
+
+  Returns:
+    A sequence of tuples, where each tuple is in the format
+      (`_ACTION_PREFIX`, `device_id`, `setpoint`).
+
   """
   device_action_tuples = []
   for device_info in devices:
@@ -745,9 +756,21 @@ def get_zone_reward_infos(
     zone_infos: Sequence[smart_control_building_pb2.ZoneInfo],
     device_infos: Sequence[smart_control_building_pb2.DeviceInfo],
 ) -> Mapping[str, smart_control_reward_pb2.RewardInfo.ZoneRewardInfo]:
-  """Returns a mapping of messages with zone data.
+  """Get zone reward information.
 
   This is used to compute the instantaneous reward.
+
+  Args:
+    current_timestamp: The current timestamp.
+    step_interval: The time duration of a single step.
+    current_observation_mapping: A mapping for the current observation.
+    occupancy_function: An occupancy function.
+    setpoint_schedule: A setpoint schedule.
+    zone_infos: A sequence of ZoneInfo objects.
+    device_infos: A sequence of DeviceInfo objects.
+
+  Returns:
+    A mapping of messages with zone data.
   """
   zone_reward_infos = {}
   zone_device_mapping = {