Skip to content
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

Improve Wattile time zone handling #294

Open
8 tasks
stephen-frank opened this issue Dec 11, 2023 · 0 comments
Open
8 tasks

Improve Wattile time zone handling #294

stephen-frank opened this issue Dec 11, 2023 · 0 comments
Labels
enhancement New feature or request user experience User experience / useability / "quality of life" issue

Comments

@stephen-frank
Copy link
Member

This is a bigger fix for the issue discussed in #293. At the moment, Wattile converts all timestamps to UTC when reading data prior from file to training, but after that more-or-less ignores time zones. This can lead to some very confusing behavior, as documented in NREL/nrelWattileExt#23 and NREL/nrelWattileExt#47.

This issue is to comprehensively address Wattile's handling of time zones. To avoid confusion and minimize user error, I propose:

  1. Model time zone should be defined in configs.json.
  2. If unspecified at training time, the model time zone should default to UTC. This should (a) result in a warning, and (b) result in modified configs.json.
  3. Time zone conversion should not occur during initial data reading.
  4. Instead, time zone conversion should be handled in prep_for_rnn (or, in a future refactored code base, the equivalent step of pre-processing the input data prior to feature extraction).
  5. Time zones of input time stamps should always be converted to the model time zone.*
  6. Model output should be localized to the model time zone.
  7. Any input data with time zone naive timestamps should result in an error message.
  8. Helper methods like get_input_window_for_output_time() should also be time zone aware and should convert to the model time zone internally.**

*Why not just use UTC?

Many time-based features make more sense in local time, especially when daylight savings time is used. For example, when using binary indicator variables, a model for a building with occupancy hours 8 AM - 5 PM (local time) may strongly leverage the 8-9 AM and 4-5 PM indicator variables. If the timestamps were converted to UTC, these indicator variables would be weaker, because for half the year they would be 1 hour shifted. In addition, the indicator variables are more interpretable when referenced to local time.

Is this a small thing? Probably, but robust handling of time zones and daylight savings time is a prerequisite for any software to be really polished.

**What time zone should get_input_window_for_output_time() return?

Several possibilities for what get_input_window_for_output_time() should return that I can think of:

  1. Always return timestamps in the model time zone
  2. Return timestamps in the same time zone as the method's datetime argument
  3. Allow the user to specify what time zone they want
    a. ...with a default to the model time zone
    b. ...with a default to the datetime argument's time zone

Leaning toward 3b, but open to other opinions.

@stephen-frank stephen-frank added enhancement New feature or request user experience User experience / useability / "quality of life" issue labels Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request user experience User experience / useability / "quality of life" issue
Projects
None yet
Development

No branches or pull requests

1 participant