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

Fix consistency of precomputed quantities for Rosenbrock #305

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

dennisYatunin
Copy link
Member

Purpose

This PR fixes a bug in the Rosenbrock timestepper that results in inconsistent precomputed quantities in ClimaAtmos.

Since the first stage of an s-stage timestepping scheme is typically the same as the final state of the previous timestep, we can chose to update p based on each of the stages U_1--U_s, or we can choose to update it based on the stages U_2--U_s and the final state u. The Rosenbrock timestepper is currently using the first option, whereas the ARK methods use the second option. With the first option, the value of p between timesteps is not consistent with the value of u, since it did not get updated when u was computed and still has the value from U_s (for FSAL timesteppers, u = U_s, but SSPKnoth is not FSAL). The second option fixes this issue, so the Rosenbrock timestepper needs to be updated to use it.

Also, our computation for p modifies the state by updating the boundary face values of u_3. With the first option, only the intermediate stages get updated, and the final state u is left with unconstrained boundary face values of u_3. This has resulted in nonsensical surface velocities in ClimaAtmos simulations with topography (these velocities do not seem to be used for any tendencies or diagnostics, which is why they have gone unnoticed for some time).


  • I have read and checked the items on the review checklist.

@dennisYatunin dennisYatunin force-pushed the dy/rosenbrock_precomputed_quantities branch from 6188d72 to ee16999 Compare July 18, 2024 00:28
@dennisYatunin dennisYatunin merged commit 7cc8f91 into main Jul 18, 2024
7 checks passed
@dennisYatunin dennisYatunin deleted the dy/rosenbrock_precomputed_quantities branch July 18, 2024 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants