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

time convention in Modelica #1192

Closed
Mathadon opened this issue Aug 19, 2019 · 1 comment · Fixed by #1193
Closed

time convention in Modelica #1192

Mathadon opened this issue Aug 19, 2019 · 1 comment · Fixed by #1193
Assignees

Comments

@Mathadon
Copy link
Member

Mathadon commented Aug 19, 2019

Once more, I'd like to open an issue about the meaning of the variable time in our simulations, since it keeps confusing me and I have the feeling we can document it better. The way I understand it now, we can summarize the convention as follows:

time is the index variable of the weather file, which by convention starts with data for midnight January 1st in the local time zone at time=0. timZon from the weather file shifts this time such that the solar position is correct in the solar irradiation computations. And that's pretty much all there's to it.

I think this should be clearly mentioned in the documentation of ReaderTMY3 and CalendarTime.

With respect to CalendarTime I think we actually have a bug on our hands since the unix time stamp is defined as It is the number of seconds that have elapsed since 00:00:00 Thursday, 1 January 1970,[2] Coordinated Universal Time (UTC), minus leap seconds (Wikipedia) whereas we use the local time instead of UTC to compute the hour, day, etc in CalendarTime. Using a time stamp without specifying the time zone thus seems incorrect, although it is more intuitive for someone who hasn't had to worry about time zones before. I propose nonetheless to revise the implementation and to force the user to provide a time zone when using unix time stamps.

If all agree then I will propose an implementation.

@Mathadon Mathadon self-assigned this Aug 19, 2019
@mwetter
Copy link
Contributor

mwetter commented Aug 19, 2019

I agree with these changes to the documentation and models. Please make a pull request.

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.

2 participants