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

Allow reference profiles to be updated #542

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tommbendall
Copy link
Contributor

@tommbendall tommbendall commented Aug 19, 2024

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.

@tommbendall tommbendall added enhancement Pull requests or issues relating to adding a new capability research problem Pull requests or issues involving new science, with possibly unknown outcome labels Aug 19, 2024
Copy link
Contributor

@jshipton jshipton left a 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)
Copy link
Contributor

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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests or issues relating to adding a new capability research problem Pull requests or issues involving new science, with possibly unknown outcome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants