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 kinetic energy conservation with topography #2960

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dennisYatunin
Copy link
Member

@dennisYatunin dennisYatunin commented Apr 25, 2024

Purpose

This PR introduces some changes that should allow us to conserve kinetic energy in the presence of topography.

To-do

  • Add a test for exact conservation of kinetic energy in the presence of topography.
    • Update the relevant Jacobian terms for the test.

Content

  • Replace ᶠwinterp(ᶜJ, ᶜρ) in the vertical transport equation with ᶠinterp(ᶜJ * ᶜρ) / ᶠJ, since ᶠdiv internally multiplies its argument by ᶠJ.
  • Replace dot(uₕ, CTh(uₕ)) + 2 * dot(ᶜinterp(uᵥ), CT3(uₕ)) in the kinetic energy equation with dot(uₕ, CTh(uₕ) + CTh(ᶜinterp(uᵥ))) + dot(ᶜinterp(uᵥ), CT3(uₕ)), which makes it so that ∫ ᶜρₜ * ᶜK * dz = -∫ ᶜρ * ᶜKₜ * dz.
  • Update the relevant Jacobian terms.

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

@dennisYatunin dennisYatunin force-pushed the dy/conservation_fixes branch 3 times, most recently from 368155a to e0dc193 Compare April 25, 2024 22:43
Copy link
Member

@szy21 szy21 left a comment

Choose a reason for hiding this comment

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

The code change looks good to me.

@dennisYatunin dennisYatunin force-pushed the dy/conservation_fixes branch from e944909 to c376852 Compare April 29, 2024 19:10
@akshaysridhar
Copy link
Member

Thanks @dennisYatunin; I can test this against the dss update branch as well.

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