-
Notifications
You must be signed in to change notification settings - Fork 11
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
Allow reference profiles to be updated #542
base: main
Are you sure you want to change the base?
Conversation
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.
I just have a couple of small questions... (see comments)
gusto/core/io.py
Outdated
step = 1 | ||
last_ref_update_time = None | ||
initial_steps = None | ||
time_data = (t, step, initial_steps, last_ref_update_time) |
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.
I feel slightly concerned about having a tuple of things that I might need to remember the order of! Could we use a named tuple?
Something like (completely untested!):
TimeData = namedtuple('TimeData', ['t', 'step', 'initial_steps', 'last_ref_update_time'])
time_data = TimeData(t, step, initial_steps, last_ref_update_time)
Then you can later access these like: t = time_data.t
.
@@ -302,8 +339,7 @@ def timestep(self): | |||
|
|||
for inner in range(self.num_inner): | |||
|
|||
# TODO: this is where to update the reference state |
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.
Was this comment just wrong?
This PR allows the reference profiles used in the Semi-Implicit Quasi Newton time stepper to be updated with time.
This is set by an argument to the time stepper. When the reference profiles are updated, the compressible linear solver also needs to recompute some operators. I have also attempted to make this work with checkpointing.