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

Add P3 terminal velocity functions #425

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Add P3 terminal velocity functions #425

merged 1 commit into from
Jul 25, 2024

Conversation

trontrytel
Copy link
Member

@trontrytel trontrytel commented Jul 23, 2024

This PR:

  • Adds the rain and ice individual particle terminal velocities needed for P3 scheme numerical integrals. A peel off from Add P3 Sink Terms #381.
  • Fixes a typo in one of the velocity difference computations.
  • Removes the TerminalVelocity file and adds the necessary functions in the Microphysics2M (for rain) or P3_terminal_velocity (for ice) files.

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.73%. Comparing base (9cf787d) to head (01ca473).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
- Coverage   96.99%   96.73%   -0.27%     
==========================================
  Files          41       40       -1     
  Lines        1465     1469       +4     
==========================================
  Hits         1421     1421              
- Misses         44       48       +4     
Components Coverage Δ
src 98.61% <77.77%> (-0.29%) ⬇️
ext 69.79% <ø> (ø)

@trontrytel trontrytel changed the title Add P3 collision and melting tendencies Add P3 terminal velocity functions Jul 25, 2024
@trontrytel trontrytel marked this pull request as ready for review July 25, 2024 20:19
@trontrytel trontrytel requested a review from rorlija1 July 25, 2024 20:19
Copy link
Contributor

@rorlija1 rorlija1 left a comment

Choose a reason for hiding this comment

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

I'm not sure if there are rules as to what to comment on in a PR review: But I appreciate the TODOs you added, and I noticed that since the velocity difference function will be used only when we merge the processes, it should be good to merge it even though it's not covered by the tests. Looks great!

@rorlija1 rorlija1 merged commit c5ae288 into main Jul 25, 2024
9 of 12 checks passed
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.

3 participants