-
Notifications
You must be signed in to change notification settings - Fork 150
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
correct_unit branch #496
correct_unit branch #496
Conversation
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.
A few changes need to be reverted since they don't belong into this PR and conflict with other ongoing efforts, others are good to have despite not belonging here. A few questions as well. Please mark the PR as ready for review once you made the changes. Thanks!
@@ -22,7 +22,7 @@ subroutine GFS_rrtmg_pre_run (Model, Grid, Sfcprop, Statein, & ! input | |||
Tbd, Cldprop, Coupling, & | |||
Radtend, & ! input/output | |||
imfdeepcnv, imfdeepcnv_gf, & | |||
f_ice, f_rain, f_rimef, flgmin, cwm, & ! F-A mp scheme only | |||
flgmin, & ! F-A mp scheme only |
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 change is not associated with correcting units, nonetheless it is good to fix this (Dusan reported issues with those arrays not being allocated but having explicit dimensions).
physics/GFS_rrtmg_setup.meta
Outdated
@@ -113,32 +113,32 @@ | |||
intent = in | |||
optional = F | |||
[iovr_sw] | |||
standard_name = flag_for_max_random_overlap_clouds_for_shortwave_radiation | |||
long_name = sw: max-random overlap clouds | |||
standard_name = flag_for_cloud_overlapping_method_for_shortwave_radiation |
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.
Please revert this change.
physics/GFS_rrtmg_setup.meta
Outdated
units = flag | ||
dimensions = () | ||
type = integer | ||
intent = in | ||
optional = F | ||
[iovr_lw] | ||
standard_name = flag_for_max_random_overlap_clouds_for_longwave_radiation | ||
long_name = lw: max-random overlap clouds | ||
standard_name = flag_for_cloud_overlapping_method_for_longwave_radiation |
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.
Please revert this change.
@@ -534,7 +498,7 @@ | |||
standard_name = instantaneous_3d_cloud_fraction | |||
long_name = instantaneous 3D cloud fraction for all MPs | |||
units = frac | |||
dimensions = (horizontal_dimension,vertical_dimension) | |||
dimensions = (horizontal_dimension,adjusted_vertical_layer_dimension_for_radiation) |
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 fixing those wrong dimensions.
physics/GFS_rrtmg_setup.meta
Outdated
units = flag | ||
dimensions = () | ||
type = integer | ||
intent = in | ||
optional = F | ||
[isubc_sw] | ||
standard_name = flag_for_sw_clouds_without_sub_grid_approximation | ||
long_name = flag for sw clouds without sub-grid approximation | ||
standard_name = flag_for_subcolumn_cloud_approximation_for_shortwave_radiation |
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.
Please revert this change.
physics/GFS_rrtmgp_setup.meta
Outdated
units = flag | ||
dimensions = () | ||
type = integer | ||
intent = in | ||
optional = F | ||
[isubc_lw] | ||
standard_name = flag_for_lw_clouds_without_sub_grid_approximation | ||
long_name = flag for lw clouds without sub-grid approximation | ||
standard_name = flag_for_subcolumn_cloud_approximation_for_longwave_radiation |
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.
Please revert this change.
standard_name = in_number_concentration | ||
long_name = IN number concentration | ||
units = kg-1? | ||
standard_name = ice_nucleation_number |
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.
@grantfirl @ligiabernardet has this change been coordinated with you?
standard_name = ccn_number_concentration | ||
long_name = CCN number concentration | ||
units = kg-1? | ||
standard_name = tendency_of_ccn_activated_number |
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.
@grantfirl @ligiabernardet has this change been coordinated with you?
physics/mp_fer_hires.F90
Outdated
@@ -185,7 +184,7 @@ SUBROUTINE mp_fer_hires_run(NCOL, NLEV, DT ,SPEC_ADV & | |||
integer :: I,J,K,N | |||
integer :: lowlyr(1:ncol) | |||
integer :: dx1 | |||
!real(kind_phys) :: mprates(1:ncol,1:nlev,d_ss) | |||
real(kind_phys) :: cwm(1:ncol,1:nlev) |
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.
Please revert this change.
physics/mp_fer_hires.meta
Outdated
@@ -214,15 +214,6 @@ | |||
kind = kind_phys | |||
intent = inout | |||
optional = F | |||
[cwm] |
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.
Please revert this change.
Note: this PR will be pulled into a feature branch feature/transition-to-capgen-1 in the NCAR GitHub repo, which collects several non-answer-changing PRs for that effort of transitioning to |
I reverted changes. It passed all RTs and is ready to review.
…On Tue, Sep 29, 2020 at 6:17 PM Dom Heinzeller ***@***.***> wrote:
Note: this PR will be pulled into a feature branch
feature/transition-to-capgen-1 in the NCAR GitHub repo, which collects
several non-answer-changing PRs for that effort of transitioning to
cap_gen.py.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#496 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AG7TW2SLYSGKAEWY7QRG4E3SIJ2LBANCNFSM4RFJB6XA>
.
|
This PR was merged into #506 for NCAR's feature/transition-to-capgen-1 branch. These changes will be merged into the authoritative branch in a couple of weeks, and this PR should be flagged as "merged' automatically. |
This PR include metadata unit/dimension/name correctness.
It does not change answers on Hera.
The associated PR is NOAA-EMC/fv3atm#170.