-
Notifications
You must be signed in to change notification settings - Fork 21
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
Split implicit and explicit precomputed quantities #3663
Conversation
ddb3f54
to
03e24f2
Compare
tagging @costachris because there are prognostic edmf changes |
Is it expected to change MSE? |
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.
Overall the changes look fine, but I'm also concerned that there are mse changes. Any idea what is wrong there?
Yes, we do expect MSE changes for simulations with implicit diffusion. We were previously updating the entire cache before computing the implicit tendency, but now we are only updating quantities related to velocity and thermodynamics. In particular, we are no longer updating the diffusivity, so its value in the implicit tendency is now reused from the previous explicit tendency. Since we are computing the implicit tendency with a different diffusivity, we expect the results to change. However, the diffusivity is mostly constant with respect to the state, so we only have small MSE changes and no meaningful differences in the summary plots. I am going to add a simple heat equation test with fixed diffusivity in a separate PR, and then I will be able to confirm that these changes have no effect on implicit diffusion when diffusivity is constant. |
d65aa7b
to
15ae1c0
Compare
15ae1c0
to
b954eee
Compare
@costachris Could you take a look at the prognostic edmf relevant changes in this PR and see if it makes sense to you and/or make sure you understand the change? |
b954eee
to
2569a49
Compare
2569a49
to
ff088a4
Compare
Purpose
This PR splits the precomputed quantities that need to be handled implicitly (i.e., updated on each Newton iteration) from those that can be handled explicitly (i.e., held constant throughout each Newton solve).
This change should improve performance for all simulations with more than one Newton iteration, since we are currently spending a significant chunk of time updating precomputed quantities that can be handled explicitly. Specifically, this should reduce the performance penalty that we have observed in #3662 from increasing the number of Newton iterations for configurations with implicit diffusion.