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

Safeguard equation_coefficients file to reflect the actual code #447

Merged
merged 3 commits into from
Mar 29, 2023

Conversation

illorenzo7
Copy link
Contributor

@illorenzo7 illorenzo7 commented Feb 14, 2023

This pull requests adds a new subroutine,

Set_Reference_Equation_Coefficients()

to the PDE_Coefficients module, which is called at the end of the Initialize_Reference() subroutine. The subroutine sets ra_constants and ra_functions (except ones having to do with diffusion, heating, and buoyancy) directly from the "ref" object and is independent of reference_type. This reduces code bloat (previously there were several repeated lines at the end of each ...Reference() subroutine specifying ra_constants/ra_functions in the exact same way), should ensure that the equation_coefficients file always reflects the equations the code actually solved, and should make future reference_type's less error-prone.

This pull request is the natural follow on to #443, which safeguarded the ra_constants/ra_functions having to do with the diffusions.

Copy link
Contributor

@feathern feathern left a comment

Choose a reason for hiding this comment

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

Sorry this took so long. I think I reviewed this a while back and somehow forgot to hit 'OK' and merge. I just went through again and this looks good. I do note that viscous_amp and ohmic_amp are nonzero for the constant reference initialization, despite the fact that viscous and ohmic heating are explicitly turned off. These should probably be set to zero for consistency, but that can be done in another PR -- just noting it here.

@feathern feathern merged commit 4eb30c9 into geodynamics:main Mar 29, 2023
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.

2 participants