google / google/sbsim

Ambiguity between BaseBuilding's `time_step_sec` property and Environment's `step_interval` values

Open
#20 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
123
Forks
56
Avg merge
17d 18h
Merged PRs (30d)
3

Description

In the `smart_control/environment/environment.py`, the `Environment` class has a parameter `step_interval: pd.Timedelta = pd.Timedelta(15, unit="minutes")`. This parameter is used to calculate the number of timesteps in the episode:
````
self._num_timesteps_in_episode = int(
(self._end_timestamp - self._start_timestamp) / self._step_interval
)
````

However, in the `smart_control/models/base_building.py` file, the `BaseBuilding` class has a `@property def time_step_sec(self) -> float:` property, which is used by the `Environment` class to calculate the `steps_per_episode` property:

````
@property
def steps_per_episode(self) -> int:
return (
self._end_timestamp - self._start_timestamp
).total_seconds() // self.building.time_step_sec
````

This seems ambiguous to me: if the value of `step_interval` does not correspond to the same value as `time_step_sec`, there will be a divergence in the number of steps

Here, do these two parameters indicate the same concept? (In which case we can get rid of the `step_interval` parameter of the `Environment` class?)

Or do they indicate different concepts? (In which case the implementation of `Environment` must be changed?)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.