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 column integrals for deep atmosphere #2119

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dennisYatunin
Copy link
Member

This PR modifies the column_integral_definite! and column_integral_indefinite! functions for deep atmosphere configurations by weighting the discrete lengths Δz with discrete areas ΔA. Since the values of ΔA are constant within each column when using the shallow atmosphere approximation, this change has no effect for shallow atmosphere configurations, and all current unit tests generate the same results.

The purpose of this change is to fix the errors observed in the deep atmosphere PR's buildkite run, where the vertical integrals for zero-moment microphysics appear to not be conserving water mass.

  • Code follows the style guidelines OR N/A.
  • Unit tests are included OR N/A.
  • Code is exercised in an integration test OR N/A.
  • Documentation has been added/updated OR N/A.

@charleskawczynski
Copy link
Member

Nice, thank you, @dennisYatunin!

Can we please add deep atmosphere unit tests in ClimaCore for this function?

@dennisYatunin dennisYatunin force-pushed the dy/deep_column_integrals branch 2 times, most recently from 7b05841 to 05ff05d Compare January 11, 2025 03:29
@dennisYatunin
Copy link
Member Author

dennisYatunin commented Jan 11, 2025

@charleskawczynski I have added a unit test to test/Operators/integrals.jl that checks for integral consistency in both shallow and deep configurations. We are now always able to achieve consistency up to several times machine epsilon, including with topography. The consistency is also exact for deep atmospheres with Float64 values on CPUs, which is even better than I was expecting.

I had to add some keyword arguments to the space constructors in order to ensure that topography and deep atmospheres are initialized correctly. Please take another look when you have some time.

@dennisYatunin dennisYatunin force-pushed the dy/deep_column_integrals branch from 05ff05d to 3f1fc0d Compare January 12, 2025 10:17
@dennisYatunin dennisYatunin force-pushed the dy/deep_column_integrals branch from 3f1fc0d to 930818e Compare January 12, 2025 10:21
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