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

Improve tracer density #492

Merged
merged 12 commits into from
Mar 28, 2024
Merged

Improve tracer density #492

merged 12 commits into from
Mar 28, 2024

Conversation

ta440
Copy link
Collaborator

@ta440 ta440 commented Mar 26, 2024

Improving the tracer density diagnostic. This constructs a higher-order space for the tracer density based on the function spaces for the mixing ratio and density.
The active tracers of rain, cloud_water, water_vapour, have an added density_name argument to make them compatible with using conservative transport.



class Rain(ActiveTracer):
"""An object encoding the details of rain as a tracer."""
def __init__(self, name='rain', space='theta',
variable_type=TracerVariableType.mixing_ratio,
density_name=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

should the density_name argument come after the transport_eqn argument for consistency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, good spot, thanks!

m_X = state_fields(self.mixing_ratio_name)
rho_d = state_fields(self.density_name)

m_X_space = m_X.function_space()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want all of this code to be protected behind an if self.space_defined is None -- currently this diagnostic allows the user to pass in a space (and we want the user to be able to do that) but it will be overridden by this code. I think we only want to override if space is None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Cool, I've changed it to enable this, although I've used if self.space is None instead of self.space_defined

@@ -1720,4 +1726,36 @@ def setup(self, domain, state_fields):
m_X = state_fields(self.mixing_ratio_name)
rho_d = state_fields(self.density_name)
self.expr = m_X*rho_d
super().setup(domain, state_fields)

if self.space is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Final thing: could you add some comments to explain why we want to have a specific space for the tracer density? Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I've added a few more comments.

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!

@tommbendall tommbendall merged commit 6133055 into main Mar 28, 2024
4 checks passed
@tommbendall tommbendall deleted the improve_tracer_density branch March 28, 2024 17:12
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