intelligent-environments-lab / CityLearn

Official reinforcement learning environment for demand response and load shaping
MIT License
462 stars 167 forks source link

[BUG] The day_type returned by get_periodic_observation_metadata function in building.py might be wrong #61

Closed Skywuuuu closed 1 year ago

Skywuuuu commented 1 year ago

Issue Description

Hi, I think the day_type returned by get_periodic_observation_metadata function in building.py might be wrong

Current Code

def get_periodic_observation_metadata(self) -> Mapping[str, int]:
    r"""Get periodic observation names and their minimum and maximum values for periodic/cyclic normalization.

    Returns
    -------
    periodic_observation_metadata : Mapping[str, int]
        Observation low and high limits.
    """

    return {
        'hour': range(1, 25), 
        'day_type': range(1, 9), 
        'month': range(1, 13)
    }

Possible Solution

According to the description written in document, day of week ranging from 1 (Monday) through 7 (Sunday). I think the correct range of day_type should be 1 to 7. And it can be verified since I cannot find 8 in day_type when I check the building csv files.

    def get_periodic_observation_metadata(self) -> Mapping[str, int]:
        r"""Get periodic observation names and their minimum and maximum values for periodic/cyclic normalization.

        Returns
        -------
        periodic_observation_metadata : Mapping[str, int]
            Observation low and high limits.
        """

        return {
            'hour': range(1, 25), 
            'day_type': range(1, 8), 
            'month': range(1, 13)
        }
kingsleynweye commented 1 year ago

Hi @Skywuuuu, thanks for the observation. It isn't a bug. Depending on the dataset you work with, day_type=8 can be valid. The citylearn_challenge_2020 and citylearn_challenge_2021 datasets include 8 as a day_type to mean special days e.g. national holidays. The classification comes from EnergyPlus as those datasets were generated using EnergyPlus building models.

Skywuuuu commented 1 year ago

Hi @kingsleynweye, thanks for your explanation!