You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue is to comprehensively address Wattile's handling of time zones. To avoid confusion and minimize user error, I propose:
Model time zone should be defined in configs.json.
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.
Time zone conversion should not occur during initial data reading.
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).
Time zones of input time stamps should always be converted to the model time zone.*
Model output should be localized to the model time zone.
Any input data with time zone naive timestamps should result in an error message.
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:
Always return timestamps in the model time zone
Return timestamps in the same time zone as the method's datetime argument
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.
The text was updated successfully, but these errors were encountered:
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:
configs.json
.configs.json
.prep_for_rnn
(or, in a future refactored code base, the equivalent step of pre-processing the input data prior to feature extraction).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:datetime
argumenta. ...with a default to the model time zone
b. ...with a default to the
datetime
argument's time zoneLeaning toward 3b, but open to other opinions.
The text was updated successfully, but these errors were encountered: