Skip to content

Date aware examples#325

Open
joewallwork wants to merge 9 commits into
mainfrom
date-aware-examples
Open

Date aware examples#325
joewallwork wants to merge 9 commits into
mainfrom
date-aware-examples

Conversation

@joewallwork

Copy link
Copy Markdown
Contributor

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.

@joewallwork joewallwork requested a review from tkarna September 30, 2022 08:38
stephankramer
stephankramer previously approved these changes Feb 29, 2024

@stephankramer stephankramer left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread examples/north_sea/model_config.py Outdated
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)

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor Author

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.

@joewallwork

Copy link
Copy Markdown
Contributor Author

[Merged in master to fix conflicts.]

@stephankramer

Copy link
Copy Markdown
Contributor

@joewallwork : could you resolve the conflicts - if you still want this in, I think this is good to go - otherwise could you close?

@joewallwork

Copy link
Copy Markdown
Contributor Author

@joewallwork : could you resolve the conflicts - if you still want this in, I think this is good to go - otherwise could you close?

I think it should be sufficient to just go with main in the conflict - let's see what the CI says.

@cpjordan

cpjordan commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Whilst you're looking at this, I've made some changes as part of #438 to speed up CI by reducing num_subfaults_par x num_subfaults_perp (CI only). I assume this is sensible and you could do this now to speed up testing if it's helpful: #438 (comment)

@joewallwork joewallwork requested a review from stephankramer June 5, 2026 10:37
Affects when options.simulation_initial_date is set *and*
self.simulation_time is nonzero (i.e. when running from checkpoint with
load_state()).

@stephankramer stephankramer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also added a fix for restarting with simulation_initial_date

Comment on lines +123 to +124
default_start_date = datetime.datetime(2011, 3, 11, 14, 46, tzinfo=sim_tz)
default_end_date = datetime.datetime(2011, 3, 11, 16, 46, tzinfo=sim_tz)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
default_start_date = datetime.datetime(2011, 3, 11, 14, 46, tzinfo=sim_tz)
default_end_date = datetime.datetime(2011, 3, 11, 16, 46, tzinfo=sim_tz)
default_start_date = sim_tz.localize(datetime.datetime(2011, 3, 11, 14, 46))
default_end_date = sim_tz.localize(datetime.datetime(2011, 3, 11, 16, 46))

If you do:
print(datetime.datetime(2011, 3, 11, 14, 46, tzinfo=sim_tz), sim_tz.localize(datetime.datetime(2011, 3, 11, 14, 46)))
you get
2011-03-11 14:46:00+09:19 2011-03-11 14:46:00+09:00
i.e. the former uses a time zone of UTC+9:19 instead of the correct UTC+9. If I understand correctly, datetime in the standard library only has limited support for timezones, and it doesn't handle the fact that timezones change historically (i.e. Japan used to be on UTC+9:19 but is now UTC+9

if os.getenv("THETIS_REGRESSION_TEST") is not None:
t_end = 5 * t_export
model_options["simulation_initial_date"] = default_start_date
model_options["simulation_end_date"] = datetime.datetime(2011, 3, 11, 14, 51, tzinfo=sim_tz)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
model_options["simulation_end_date"] = datetime.datetime(2011, 3, 11, 14, 51, tzinfo=sim_tz)
model_options["simulation_end_date"] = sim_tz.localize(datetime.datetime(2011, 3, 11, 14, 51))

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 this pull request may close these issues.

3 participants