Move T_imp approximation before DSS #352
Open
+26
−23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
|
I'm not opposed to these changes, but having moved these pieces around myself, I remember just how complex some of the edge cases are, and I'm a bit uneasy about the current test situation in the code. I'd really like to fix #106 so that it's simple and clear what is being changed here. I don't want to slow down progress, so please feel free to merge if we get a green light on CI and atmos tests look fine. I'll bump the priority of fixing #106. |
charleskawczynski
approved these changes
Jan 22, 2025
dennisYatunin
force-pushed
the
dy/T_imp_approximation
branch
from
8a59247 to
6559429
Compare
dennisYatunin
force-pushed
the
dy/T_imp_approximation
branch
from
4ec8293 to
b44b486
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose
This PR undoes a small change that was accidentally introduced in #348. We were previously approximating the implicit tendency as
(U_after_implicit_solve - U_before_implicit_solve) / (Δt * γ), but in that PR I ended up changing it to(DSS(U_after_implicit_solve) - U_before_implicit_solve) / (Δt * γ). In the absence of roundoff error, these would be exactly identical. Unfortunately, we do not satisfyDSS(DSS(U)) == DSS(U)at a discrete level, so they are not quite identical. Reverting to the original approximation ofT_impmight marginally improve stability.