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.

Sign up for GitHub

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

Jump to bottom

(Remove interstitial variables for land and ice emissivity and update the land and ice emissivity in the routine setemis) Sm sept21 pr #736

Merged
merged 58 commits into from
Oct 22, 2021
Merged

(Remove interstitial variables for land and ice emissivity and update the land and ice emissivity in the routine setemis) Sm sept21 pr #736

merged 58 commits into from
Oct 22, 2021

Conversation

SMoorthi-emc
Copy link
Collaborator

This PR addresses the ccpp-physics issue # 735

@climbfuji climbfuji changed the title Sm sept21 pr (Remove interstitial variables for land and ice emissivity and update the land and ice emissivity in the routine setemis) Sm sept21 pr Sep 22, 2021
@climbfuji
Copy link
Collaborator

@SMoorthi-emc I see that the code changes also affect RUC LSM. Have tests been run with RUC?

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Sep 22, 2021 via email

@climbfuji
Copy link
Collaborator

@tanyasmirnova Would you be able to run a test or two with the suggested changes for land/ice emissivity in RUC?

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Sep 22, 2021 via email

@HelinWei-NOAA
Copy link
Collaborator

This change also removed one inconsistency in the current code for initial value of emiss_ice for RUC LSM.

In the line 414 of GFS_phys_time_vary.fv3.F90, emiss_ice(ix) = 0.97_kind_phys,

but 0.95 is used in GFS_surface_composites.F90:
if (iemsflg == 2 .and. (.not.flag_init .or. flag_restart) .and. lsm == lsm_ruc) then
!-- use emis_ice from RUC LSM with snow effect
semis_ice(i) = emis_ice(i)
else
semis_ice(i) = 0.95_kind_phys
endif

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Sep 23, 2021 via email

tanyasmirnova
tanyasmirnova reviewed Sep 23, 2021
View reviewed changes
physics/radiation_surface.f Outdated
semis_lnd(i) = emsref(8)
else
tmp1 = (fracl(i)-fsno) / fracl(i)
semis_lnd(i) = semis_lnd(i) * tmp1 + (f_one-tmp1)*fsno
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line has a bug: there is no snow emissivity in the second term on the right side.

physics/radiation_surface.f Outdated
semis_lnd(i) = emsref(8)
else
tmp1 = (fracl(i)-fsno) / fracl(i)
semis_lnd(i) = semis_lnd(i)*tmp1 + (f_one-tmp1)*fsno
Copy link
Collaborator

Choose a reason for hiding this comment

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

same bug here

physics/radiation_surface.f
sfcemis_ice = sfcemis_ice*tmp1+emsref(8)*(f_one-tmp1)
else
sfcemis_ice = emsref(8)
endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

@SMoorthi-emc I don't understand why this change is needed. The fsno is a snow cover fraction on ice, thus, emissivity of ice with partial snow cover will be sfcemis_ice*(f_one-fsno)+emsref(8)*fsno. This formulation was in the original code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tanya,
The original code had
" sfcemis_ice = sfcemis_ice*(f_one-fsno)+emsref(8)*fsno"
This assumes that ice fraction is one. This need not be the case with a fractional grid.
If it is a fractional grid, then ice fraction is going to be less than one (this can be true even if there is no land).
What I did is not perfect, but if the snow fraction is less than ice fraction I do the weighted average; if snow fraction is larger than ice fraction, then I assume all ice is covered by snow.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Moorthi, I see now that you are taking care of fractional sea ice. But actually in this part of the code when iemslw=2, the final composition is happening in the last line of the loop:
962 sfcemis(i) = fracl(i)*sfcemis_land + fraco(i)*emsref(1) &
963 & + fraci(i)*sfcemis_ice

Therefore, with the change you made fractional sea ice will be taken into account twice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it is only computing mean value over ice.
The lone
" sfcemis(i) = fracl(i)*sfcemis_land + fraco(i)*emsref(1) &
& + fraci(i)*sfcemis_ice"
does the weight for all three components (land, water and ice)
We could potentially remove local variables, but fo rnow I will not do it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the original code had a bug. It assumed ice fraction as one in calculating sfcemis_ice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@SMoorthi-emc @climbfuji @barlage @yangfanglin
Original radiation_surface.f was not accounting for fractional grid, neither was the sfc_sice.f. With the fractional grid sfc_sice.f should provide sncovr_ice(i) and snwdpth_ice(i) as it is solving energy budget (accumulating/melting snow) over the ice fraction of the grid cell. Similarly, if coupled to CICE, it should provide these variables or computed in CICE semis_ice(i).
RUC LSM has its own treatment of sea ice in case of uncoupled to CICE model only, thus, it computes emissivity over ice internally and it is used in radiation_surface.f. RUC LSM also provides sncovr_ice(i) and snwdpth_ice(i). When CICE is coupled to the model, the radiation_surface.f should use semis_ice, or sncover_ice and snwdpth_ice from CICE.
Noah or Noah MP models do not have the sea ice modules, therefore, with these LSMs snow depths and snow cover fractions over ice should be provided by sfc_sice.f or CICE, which is not done yet. I think this PR is a good time to make it work consistently for all scenarios.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When coupled to CICE, CICE provides upward log wave radiation not emissivity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think there is a potential issue that I am trying to think how to fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The issue I alluded to above is related to emissivity over sea_ice model CICE6. We import upward longwave radiation from CICE6. Unfortunately, there is some inconsistency in the current coupled model. So, what I did yesterday and today is to remove this inconsistency by defining an estimated "emis_ice" over seaice from the imported upward longwave and ice skin temperature from the ice model. I had to make appropriate changes in the radiation_surface code so that it used this emissivity for sea-ice when the model is coupled. Unfortunately, I had to remove the variable "tice" from "lsm_ruc_run" as this variable and "tskin_ice" are the same and I could not define the same name for two variables in the same meta file (I sent an email to Tanya about this, but haven't heard back yet).
Anyway, my repository has been updated it is what it is under the circumstances.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@SMoorthi-emc I agree with your removal of tice, thank you! Also, I think your solution for emissivity from CICE6 makes sense.

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Sep 23, 2021 via email

@climbfuji
Copy link
Collaborator

Done.
…

Thank you!

@junwang-noaa junwang-noaa mentioned this pull request Oct 19, 2021
Merged
JongilHan66
JongilHan66 approved these changes Oct 20, 2021
View reviewed changes
Copy link
Collaborator

@JongilHan66 JongilHan66 left a comment

Choose a reason for hiding this comment

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

The changes look ok to me. One question: in line 187 of GFS_radiation_surface.F90, why is only 'snodl' (not 'snodi') passed to subroutine 'setalb'?

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Oct 20, 2021 via email

@JongilHan66
Copy link
Collaborator

Is the old 'snowd' equivalent with 'snodl' or [(ice fraction)'snodi' + (land fraction)'snodl']? If it is the latter, the latter should be passed to 'setalb'.

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Oct 20, 2021 via email

@tanyasmirnova
Copy link
Collaborator

@climbfuji @SMoorthi-emc I think we still need to run the test before/after the changes. I can run the test with RUC LSM if I have the code before/after.

@climbfuji
Copy link
Collaborator

@climbfuji @SMoorthi-emc I think we still need to run the test before/after the changes. I can run the test with RUC LSM if I have the code before/after.

git clone -b develop --recursive https://github.com/ufs-community/ufs-weather-model BEFORE
git clone -b SM_Sept21_PR --recursive https://github.com/SMoorthi-emc/ufs-weather-model AFTER

@JongilHan66
Copy link
Collaborator

The "snowd" is "the new "snodl". You can see this by looking at "meta" file.
…

I see. Thanks!!

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Oct 20, 2021 via email

@HelinWei-NOAA
Copy link
Collaborator

@SMoorthi-emc also snodi is only available from RUC. Do you define it somewhere for Noah or Noah MP. snodi is used in setemis for all LSMs.

Jongil, but, to some extent you are right. The "setalb" does need to be fixed. I wasn't going to change it, but maybe I should give it a try. Moorthi

@HelinWei-NOAA
Copy link
Collaborator

@SMoorthi-emc They are from sfc_sice. Since lsm is called before sice, snodi from ruc will be overwritten by the value from sice. Right?

@SMoorthi-emc also snodi is only available from RUC. Do you define it somewhere for Noah or Noah MP. snodi is used in setemis for all LSMs.

Jongil, but, to some extent you are right. The "setalb" does need to be fixed. I wasn't going to change it, but maybe I should give it a try. Moorthi

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Oct 20, 2021 via email

@JongilHan66
Copy link
Collaborator

Jongil, After looking at "setalb" again, I have decided to change "snodl" by "snodi" as this array is used only for icy points. However, I admit that "setalb" still does not work right for fractional grid where both land and ice can exist, but that change is beyond the scope of this PR. Helin, yes, snodi is set in sfc_sice for the uncouple non-RUC GFS. For the coupled model it is set in sfc_cice. When RUC lsm is called, you should not call sfc_sice (should not be in SDF).
…

Moorthi, thanks for more clarification and fix. -Jongil

@HelinWei-NOAA
Copy link
Collaborator

@SMoorthi-emc Agree with your modification. snodi is used for icy points for both alb and emiss. Please ignore my last message. Thanks.

Jongil, After looking at "setalb" again, I have decided to change "snodl" by "snodi" as this array is used only for icy points. However, I admit that "setalb" still does not work right for fractional grid where both land and ice can exist, but that change is beyond the scope of this PR. Helin, yes, snodi is set in sfc_sice for the uncouple non-RUC GFS. For the coupled model it is set in sfc_cice. When RUC lsm is called, you should not call sfc_sice (should not be in SDF).
…
On Wed, Oct 20, 2021 at 2:09 PM HelinWei-NOAA @.***> wrote: @SMoorthi-emc https://github.com/SMoorthi-emc They are from sfc_sice. Since lsm is called before sice, snodi from ruc will be overwritten by the value from sice. Right? @SMoorthi-emc https://github.com/SMoorthi-emc also snodi is only available from RUC. Do you define it somewhere for Noah or Noah MP. snodi is used in setemis for all LSMs. Jongil, but, to some extent you are right. The "setalb" does need to be fixed. I wasn't going to change it, but maybe I should give it a try. Moorthi — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#736 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALLVRYXABT453U5ONUNPAXLUH4AVDANCNFSM5EP6BVHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

junwang-noaa
junwang-noaa reviewed Oct 20, 2021
View reviewed changes
physics/radiation_surface.f Outdated
@@ -298,7 +298,7 @@ end subroutine sfc_init
!! \n 1) climatological surface albedo scheme (\cite briegleb_1992)
!! \n 2) MODIS retrieval based scheme from Boston univ.
!!\param slmsk (IMAX), sea(0),land(1),ice(2) mask on fcst model grid
!!\param snowf (IMAX), snow depth water equivalent in mm
!!\param snowf (IMAX), snow depth water equivalent in mm over land
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be over ice?

@SMoorthi-emc
Copy link
Collaborator Author

SMoorthi-emc commented Oct 20, 2021 via email

junwang-noaa
junwang-noaa approved these changes Oct 20, 2021
View reviewed changes
Copy link
Collaborator

@junwang-noaa junwang-noaa left a comment

Choose a reason for hiding this comment

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

The code looks good to me

DeniseWorthen
DeniseWorthen approved these changes Oct 20, 2021
View reviewed changes
Copy link
Collaborator

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

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

The changes look reasonable based on my understanding of the code.

@climbfuji climbfuji merged commit b12e6c3 into NCAR:main Oct 22, 2021
This was referenced Oct 22, 2021
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HelinWei-NOAA HelinWei-NOAA HelinWei-NOAA approved these changes

@JongilHan66 JongilHan66 JongilHan66 approved these changes

@DeniseWorthen DeniseWorthen DeniseWorthen approved these changes

@tanyasmirnova tanyasmirnova tanyasmirnova approved these changes

@junwang-noaa junwang-noaa junwang-noaa approved these changes

@climbfuji climbfuji climbfuji approved these changes

@llpcarson llpcarson llpcarson approved these changes

@grantfirl grantfirl Awaiting requested review from grantfirl grantfirl is a code owner

@dustinswales dustinswales Awaiting requested review from dustinswales dustinswales is a code owner

@DusanJovic-NOAA DusanJovic-NOAA Awaiting requested review from DusanJovic-NOAA

@yangfanglin yangfanglin Awaiting requested review from yangfanglin

@barlage barlage Awaiting requested review from barlage

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@SMoorthi-emc @climbfuji @HelinWei-NOAA @yangfanglin @JongilHan66 @tanyasmirnova @llpcarson @junwang-noaa @DeniseWorthen