-
Notifications
You must be signed in to change notification settings - Fork 17
Tr/vertical mass borrowing limiter #2383
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
base: main
Are you sure you want to change the base?
Conversation
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
Add gpu support for vertical mass limiter. This addition needs documentation, and also needs to be made more similar to the cpu version. Other fixes: Correct plotting axis Important TODOS: I think the indexing is actually happening bottom to top... Ask about \geq q_min or < qmin Ask about tolerances in the test Probably a good idea to check performance. I suspect this implementation will have poor performance at h_elem < 30
8ff8caa to
8191082
Compare
8191082 to
1d18d41
Compare
| Plots.plot!(f₀col; label = "initial") | ||
| Plots.savefig("lim.png") | ||
| end | ||
| # plot_results(ρq, ρq_init) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we enable a version of these plots either in the test artifacts or the documentation for this PR?
| for v in 1:nlevels | ||
| CI = CartesianIndex(1, 1, f, v, 1) | ||
| # if the surface layer still needs to borrow mass | ||
| if bmass[] < 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the surface/bottom layer for a given column doesn't have negative tracer mass after the first top-to-bottom pass, the loop over 1:nlevels is unnecessary right ? Is there a need to check this condition at every single level v ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right
| layer k+2. The borrower will proceed this process until the bottom layer. | ||
| If the tracer mass in the bottom layer goes negative, it will repeat the | ||
| process from the bottom to the top. In this way, the borrower works for | ||
| any shape of mass profiles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the reference paper this is true as long as the "the
column-integrated mass is positive" ; We may not need assertions in the code for performance considerations but this is useful in the docstring (It's anyway clear, by design, in the fixer).
Add gpu support for vertical mass limiter.
This addition needs documentation, and also needs
to be made more similar to the cpu version.
Other fixes:
Correct plotting axis
Important TODOS:
I think the indexing is actually happening bottom to top...doneAsk about \geq q_min or < qmin
Ask about tolerances in the test
Probably a good idea to check performance. I suspect this
implementation will have poor performance at h_elem < 30