-
Notifications
You must be signed in to change notification settings - Fork 6
Added gravity-wave parameters #238
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
=======================================
Coverage 91.78% 91.78%
=======================================
Files 1 1
Lines 146 146
=======================================
Hits 134 134
Misses 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nefrathenrici
left a comment
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.
Thanks for adding these parameters! I left two minor comments and have one general request: the parameter names should be made more specific, since they exist in the global namespace. One quick fix would be to prefix them with ogw_. In particular, density_scale_factor and critical_height_threshold are quite generic.
Thank you for these suggestions! :3 I added |
The placeholder can not be merged, since removing it would be a future breaking change. Instead of merging this PR now, you could create a custom TOML file with these parameters and use the file to test these parameters. import ClimaParams
ClimaParams.create_toml_dict(Float64; override_file="path/to/my/parameters.toml")In ClimaAtmos, you will need to add this to your config for the time being: toml: ["path/to/my/parameters.toml"] |
| type = "float" | ||
| description = "placeholder" | ||
|
|
||
| # Orographic gravity wave (OGW) |
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.
The equation references aren't accompanied by a DOI paper link; could you please include that here as well ?
| [ogw_mountain_shape_parameter] | ||
| value = 0.5 | ||
| type = "float" | ||
| description = "Mountain shape parameter (β), L(z) = L_b(1 - z/h)^β (equation 12), β=1 for triangular mountains and β<1 for blunt mounrains, β>1 for pointy mountains." |
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.
| description = "Mountain shape parameter (β), L(z) = L_b(1 - z/h)^β (equation 12), β=1 for triangular mountains and β<1 for blunt mounrains, β>1 for pointy mountains." | |
| description = "Mountain shape parameter (β), L(z) = L_b(1 - z/h)^β (equation 12), β=1 for triangular mountains and β<1 for blunt mountains, β>1 for pointy mountains." |
|
Hey y'all, just wanted to let you know that I will work on this PR when I return to completing the gravity-wave parameterization. But I am letting this sit for now. |
Purpose
Added the parameters for the orographic gravity wave parametrisation. For now, a placeholder is used for the non-orographic gravity wave module.
To-do
Move the hard-coded non-orographic gravity-wave parameters into ClimaParams.
Content