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

Use deep atmosphere equations by default #3422

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

Conversation

akshaysridhar
Copy link
Member

@akshaysridhar akshaysridhar commented Nov 4, 2024

Draft PR simplifies result tracking for tests with deep-atmos equations as part of the default configuration.
Target config: 70km depth, 99 vertical centers (100 face levels).

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.

Let's have one shallow atmosphere example in ci (you can choose anything, e.g. baroclinic wave).

	modified:   config/default_configs/default_config.yml
@charleskawczynski
Copy link
Member

I think we may want to write a few more unit tests in ClimaCore to show that this is working for our finite difference stencils. In particular, I think it may be helpful to show that our FD operators are conservative. Thoughts?

@szy21
Copy link
Member

szy21 commented Nov 22, 2024

I think we may want to write a few more unit tests in ClimaCore to show that this is working for our finite difference stencils. In particular, I think it may be helpful to show that our FD operators are conservative. Thoughts?

What unit tests do we have now that is only for shallow atmosphere?

@charleskawczynski
Copy link
Member

What unit tests do we have now that is only for shallow atmosphere?

I don't know that we have conservation tests in a deep atmosphere

@szy21
Copy link
Member

szy21 commented Nov 22, 2024

What unit tests do we have now that is only for shallow atmosphere?

I don't know that we have conservation tests in a deep atmosphere

We should add them if we don't have them. Could you point me to the tests for shallow atmosphere in ClimaCore?

@charleskawczynski
Copy link
Member

Could you point me to the tests for shallow atmosphere in ClimaCore?

That's a good question, I don't actually see any at first glance. I only some conservation tests for horizontal limiters.

We should probably add some, I think we can spin off a test from one of the column heat/wave examples without too much work.

@szy21
Copy link
Member

szy21 commented Nov 25, 2024

I'm a bit concerned about the two orders of magnitude increase in conservation error in sphere_baroclinic_wave_conservation_source. This test is not exactly accurate, but I don't expect such a difference between shallow and deep atmosphere. Maybe check if sum is implemented correctly for deep atmosphere?

@szy21
Copy link
Member

szy21 commented Dec 2, 2024

Also this one aquaplanet_rhoe_equil_clearsky_tvinsol_0M_slabocean_ft64 has several orders of magnitude increase in the conservation error. This test should conserve within floating point error (see the current main build).

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