-
Notifications
You must be signed in to change notification settings - Fork 27
Improve Initialization from ERA5 and MSLP Diagnostic #4098
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
Conversation
6f20da6 to
1ce4418
Compare
| @assert all(map(x -> x in (keys(ncin)), in_dims)) "Source file $source_file is missing subset of the required dimensions: $in_dims" | ||
|
|
||
| # assert ncin has required variables | ||
| req_vars = ["u", "v", "w", "t", "q", "skt", "sp"] |
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 it breaks without surface_geopotential catch it with the @assert here
| end | ||
| var_obj = defVar(ncout, dst_name, FT, ("lon", "lat", "z"), attrib = var_attrib) | ||
| # Read first time slice and coalesce; follow same convention as sp (use [:, :, 1]) | ||
| data2d = FT.(coalesce.(ncin[src_name][:, :, 1], NaN)) |
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.
Do we need a coalesce? Are there instances when the data is NaN and our model will still run? If not then we could consider asserting not NaN
src/diagnostics/core_diagnostics.jl
Outdated
| bracket = similar(t_level) | ||
| @. bracket = max(ϵ, 1 + Γ * z_level / t_level) | ||
|
|
||
| # exponent = g / (R_m Γ) | ||
| exponent = similar(R_m_surf) | ||
| @. exponent = g / (R_m_surf * Γ) |
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.
This will allocate new arrays which is slow. Maybe @dennisYatunin could recommend a fix?
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 consolidated the logic and reduced the number of new variables. But also diagnostics are computed on the CPU so I'm not sure we need to worry about allocations as much here (note we already materialize a lot of our lazy fields throughout diagnostics as is). But maybe @dennisYatunin can comment how much it matters in the end.
Julians42
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.
Left some comments to improve clarity and then one performance optimization we should do in the diagnostics for Dennis. Glad this is looking so much better!
b961ecc to
0e88889
Compare
9deb1c7 to
2644b8b
Compare
0d327d8 to
4754f88
Compare
4754f88 to
9e6854a
Compare
…re. Account for lapse rate in mslp sea level reduction (diagnostics).
9e6854a to
ec99268
Compare
Improvements to WeatherQuest ERA5 initial conditions:
The colorbars vary, but note each improvement significantly reduces the RMSE/Bias.



Current 😭
Topography correction in surface pressure init 👀
Topography correction + account for lapse rate in MSLP diagnostic 🥳