-
Notifications
You must be signed in to change notification settings - Fork 24
Phase 2 of rk_stratiform CCPPization: diagnostic schemes #234
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: development
Are you sure you want to change the base?
Conversation
Completed in 2639a45 |
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.
Looks good @jimmielin! I did have some questions and suggestions, but hopefully nothing that will be too difficult to resolve (although if that is not the case please let me know). Thanks!
call history_add_field('DLSED', 'tendency_of_cloud_liquid_water_mixing_ratio_wrt_moist_air_and_condensed_water_due_to_sedimentation', 'lev', 'avg', 'kg kg-1 s-1') | ||
call history_add_field('HSED', 'tendency_of_dry_air_enthalpy_at_constant_pressure_due_to_sedimentation', 'lev', 'avg', 'J kg-1 s-1') | ||
call history_add_field('PRECSED', 'stratiform_cloud_water_surface_flux_due_to_sedimentation', horiz_only, 'inst', 'm s-1') | ||
call history_add_field('SNOWSED', 'lwe_cloud_ice_sedimentation_rate_at_surface_due_to_microphysics', horiz_only, 'inst', 'm s-1') |
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.
Is there a reason SNOWSED
has a different long name compare to the PRECSED
and RAINSED
variables? For example I imagine one could do the following instead to better match the other two fields:
stratiform_lwe_cloud_ice_surface_flux_due_to_sedimentation
But maybe there is a reason to keep the long name as-is?
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 the question! SNOWSED
had a previously-assigned standard name from pbuf_SNOW_SED
so it looks different than the others maybe due to the passage of time. So maybe it can be updated to be up to the more recent guidelines / consistency with the current standard names?
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 the explanation @jimmielin! Given that SNOWSED
appears to be the exact same physical quantity as PRECSED
but just for ice I might vote to give the name I specified above. Of course if this is going to involve additional work elsewhere (e.g. in other physics schemes) then feel free to just make an issue for it and we can deal with it later. Thanks!
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 @nusbaume, updated to stratiform_lwe_cloud_ice_surface_flux_due_to_sedimentation
!
schemes/sima_diagnostics/cloud_particle_sedimentation_diagnostics.F90
Outdated
Show resolved
Hide resolved
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.
Looks great to me now! I did leave one comment unresolved with regards to the SNOWSED
variable but I am happy to just make an issue for it and deal with it later if that is easiest.
…x_due_to_sedimentation
Originator(s): @jimmielin
Description (include issue title and the keyword ['closes', 'fixes', 'resolves'] and issue number):
List all namelist files that were added or changed: N/A
List all files eliminated and why: N/A
List all files added and what they do:
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>
)List all automated tests that failed, as well as an explanation for why they weren't fixed:
N/A
Is this an answer-changing PR? If so, is it a new physics package, algorithm change, tuning change, etc?
N/A - SIMA diagnostics only
If yes to the above question, describe how this code was validated with the new/modified features:
N/A