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

Tracer conservative transport #469

Merged
merged 25 commits into from
Mar 19, 2024
Merged

Conversation

ta440
Copy link
Collaborator

@ta440 ta440 commented Dec 19, 2023

This pull request enables the conservative transport of tracers.

Mixing ratios are transported using a conservative equation, as opposed to the previous advective version. This is requires an additional transport equation type (tracer_conservative), form (tracer_conservative_form) and upwind transport method (upwind_tracer_conservative form). A reference density needs to be specified when using this transport form.

The equation class CoupledTransportEquation has been changed to generate the correct mass terms when using tracer_conservative. A conservative transport test of this equation has been added to integration-tests/equations/test_coupled_transport.py.

To work with the tracer_conservative form timestepping equation, the explicit multistage schemes need to have the gradient evaluation divided by the reference density. The ExplicitMultistage class now has an additional option of increment_form, which defaults to True, but should be set to False for use with the conservative transport scheme. A test of this scheme is added.

@ta440 ta440 marked this pull request as ready for review March 15, 2024 17:17
Copy link
Contributor

@tommbendall tommbendall left a comment

Choose a reason for hiding this comment

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

Thanks so much Tim. I think this looks really good and it's very exciting to have this working.

I've got two very small requests. I think it would also be helpful to have @atb1995 look over the changes to time discretisation to ensure that he's happy

gusto/equations.py Outdated Show resolved Hide resolved
# Extract associated density variable:
# This does assume the same ordering of tracers and fields...
tracer_idx = self.equation.field_names.index(variable)
tracer = self.equation.active_tracers[tracer_idx]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way that we can avoid this assumption?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These two lines have been replaced with: tracer = next(x for x in self.equation.active_tracers if x.name == variable).

@tommbendall tommbendall requested a review from atb1995 March 18, 2024 08:17
Copy link
Collaborator

@atb1995 atb1995 left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of minor comments

if self.limiter is not None:
self.limiter.apply(self.field_i[stage])

# Obtain field_ip1 = field_n - dt* sum_m{c_im*F[field_m]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Switch to a_im to be in keeping with previous comments

Comment on lines 934 to 938
for stage in range(self.nStages):
r = self.residual.label_map(
lambda t: t.has_label(time_derivative),
map_if_true=replace_subject(self.field_i[0], old_idx=self.idx),
map_if_false=replace_subject(self.field_i[0], old_idx=self.idx))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for stage in range(self.nStages):
r = self.residual.label_map(
lambda t: t.has_label(time_derivative),
map_if_true=replace_subject(self.field_i[0], old_idx=self.idx),
map_if_false=replace_subject(self.field_i[0], old_idx=self.idx))
for stage in range(self.nStages):
r = self.residual.label_map(
all_terms,
map_if_true=replace_subject(self.field_i[0], old_idx=self.idx))

tommbendall
tommbendall previously approved these changes Mar 19, 2024
Copy link
Contributor

@tommbendall tommbendall left a comment

Choose a reason for hiding this comment

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

Thanks Tim, this is fantastic!

@tommbendall tommbendall merged commit 7f1a106 into main Mar 19, 2024
4 checks passed
@tommbendall tommbendall deleted the tracer_conservative_transport branch March 19, 2024 16:40
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