Skip to content

Ambiguity between BaseBuilding's time_step_sec property and Environment's step_interval values #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gabriel-trigo opened this issue Mar 21, 2025 · 1 comment · May be fixed by #22

Comments

@gabriel-trigo
Copy link
Contributor

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?)

@gabriel-trigo
Copy link
Contributor Author

@gabriel-trigo gabriel-trigo linked a pull request Mar 27, 2025 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant