-
Notifications
You must be signed in to change notification settings - Fork 28
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
Date aware examples #325
base: master
Are you sure you want to change the base?
Date aware examples #325
Conversation
255e76d
to
fff2666
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that looks fine. Sorry this seems to have sat there a while - not sure you were still intending to merge
examples/north_sea/model_config.py
Outdated
start_date = model_options.pop("start_date", default_start_date) | ||
end_date = model_options.pop("end_date", default_end_date) | ||
dt = 3600.0 | ||
t_export = 3600.0 | ||
t_end = (end_date - start_date).total_seconds() | ||
if os.getenv("THETIS_REGRESSION_TEST") is not None: | ||
t_end = 5 * t_export | ||
end_date = datetime.datetime(2022, 1, 1, 0, 5, tzinfo=sim_tz) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's 5 minutes past midnight, no? but the timestep is in hours?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot! Fixed in 1b07342.
[Merged in |
A short PR to make the Tohoku example date-aware. I also noticed that the North Sea example wasn't set up properly for testing: a shorter
t_end
value was set but unused.