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

Implement and test vertical mass borrowing limiters #2084

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

Conversation

charleskawczynski
Copy link
Member

@charleskawczynski charleskawczynski commented Nov 14, 2024

This is a work in progress. This PR implements vertical mass borrowing limiters.

Closes #2083.

Todo:

  • How should q_min be computed? I think q_min is actually just prescribed.
  • Is pdel the correct interpretation? I think that this is just dz
  • Add meaningful tests. Our tests currently operate on a specific total humidity. Should we also test it on ρq?
  • Add GPU implementation
  • Improve performance?

@charleskawczynski
Copy link
Member Author

This does seem to work in that it removes negative tracer values, and seems to preserve mass (both of which are unit-tested cc @tapios):

On a single column:

    @test 0  minimum(q)
    @test isapprox(sum(q), sum_q_init; atol = 0.00000000001)
    @test isapprox(sum(q), sum_q_init; rtol = 0.01)

On a full cubed sphere:

    @test 0  minimum(q)
    @test isapprox(sum(q), sum_q_init; atol = 0.013)
    @test isapprox(sum(q), sum_q_init; rtol = 0.01)

I think rtol = 0.01 should be sufficient on a sphere, right?

But, the transport/wavespeed doesn't look great:

Screenshot 2024-11-19 at 7 51 47 PM

I guess that's a bit orthogonal, though?

@charleskawczynski charleskawczynski force-pushed the ck/MassBorrowerFixer branch 2 times, most recently from 21528d7 to ec53007 Compare November 20, 2024 02:00
@charleskawczynski
Copy link
Member Author

pdel is a hydrostatic approximation of rho*dz, so we need to update this.

Also, the mass conservation test at the end should be rho*q instead.

Copy link
Contributor

@tapios tapios left a comment

Choose a reason for hiding this comment

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

Mass integral from rho dz, rather than dp

src/Limiters/vertical_mass_borrowing_limiter.jl Outdated Show resolved Hide resolved
src/Limiters/vertical_mass_borrowing_limiter.jl Outdated Show resolved Hide resolved
src/Limiters/vertical_mass_borrowing_limiter.jl Outdated Show resolved Hide resolved
@charleskawczynski
Copy link
Member Author

And let's visualize how this looks on the sphere.

@charleskawczynski charleskawczynski force-pushed the ck/MassBorrowerFixer branch 2 times, most recently from 1278c47 to bae0e8e Compare November 20, 2024 20:05
Update src/Limiters/vertical_mass_borrowing_limiter.jl

Co-authored-by: Tapio Schneider <[email protected]>

Update src/Limiters/vertical_mass_borrowing_limiter.jl

Co-authored-by: Tapio Schneider <[email protected]>

Update src/Limiters/vertical_mass_borrowing_limiter.jl

Co-authored-by: Tapio Schneider <[email protected]>

Use density-dz for pressure thickness
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.

Implement purely vertical water-borrowing limiter operators
2 participants