Fix time-averaged radiation variables#1038
Conversation
Qingfu-Liu
requested review from
DeniseWorthen,
DusanJovic-NOAA,
JessicaMeixner-NOAA,
RuiyuSun,
danielabdi-noaa,
dpsarmie,
dustinswales,
grantfirl,
junwang-noaa and
yangfanglin
and removed request for
danielabdi-noaa
|
@Qingfu-Liu , thanks for testing this fix. I can help out with running the full regression test suite if you'd like. |
|
I ran a case w/ this change to examine the shape of the exported SW components. The fixes in this PR do not "fix" the exported fields, since the exported fields are the instantaneous (not averaged) values at each timestep. The exported fields still contain the sawtooth shape, primarily in the diffuse components, as shown in the original issue. I think this is the expected result. |
|
@denise Worthen - NOAA Affiliate ***@***.***> Can you show a
plot for the problem?
…On Fri, Nov 21, 2025 at 10:45 AM Denise Worthen ***@***.***> wrote:
*DeniseWorthen* left a comment (NOAA-EMC/ufsatm#1038)
<#1038 (comment)>
I ran a case w/ this change to examine the shape of the exported SW
components.
The fixes in this PR do not "fix" the exported fields, since the exported
fields are the instantaneous (not averaged) values at each timestep.
—
Reply to this email directly, view it on GitHub
<#1038 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTS6URNILYSSJMTJIJ3XM3354XRNAVCNFSM6AAAAACM2HUP5SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNRTGU4DCOJTG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ *dpsarmie Thank you very much. Just wait to see what the problem @denise
discussed.
|
|
@Qingfu-Liu The plots are show at the beginning of the original Issue (#1767). I made my test case based on the changes shown in this comment but I can re-test w/ your actual PR. |
|
@DeniseWorthen I use time step 720s and output every 12 minutes |
|
@Qingfu-Liu Your fix is for time-averaged radiation fields that are written to the history files, right? For the coupled model, the ATM exports the instantaneous (not average) SW components Lines 3496 to 3506 in f6dc207 Your changes don't impact variables like |
|
@DeniseWorthen, I only check the output files from the regression tests (include instantaneous variables and the time-averaged variables), not sure if the changes impact the coupled variables. The following variables has large differences after the code changes:
csdlf:long_name = "Clear Sky Downward Long Wave Flux"
csdsf:long_name = "Clear Sky Downward Short Wave Flux"
cduvb_ave:long_name = "Clear sky UV-B Downward Solar Flux"
csulf:long_name = "Clear Sky Upward Long Wave Flux"
csulftoa:long_name = "Clear Sky Upward Long Wave Flux at toa"
csusf:long_name = "Clear Sky Upward Short Wave Flux"
csusftoa:long_name = "Clear Sky Upward Short Wave Flux at toa"
dswrf_ave:long_name = "averaged surface downward shortwave flux"
dswrf_avetoa:long_name = "top of atmos downward shortwave flux"
cduvb_ave:long_name = "Clear sky UV-B Downward Solar Flux"
nbdsf_ave:long_name = "Near IR Beam Downward Solar Flux"
nddsf_ave:long_name = "Near IR Diffuse Downward Solar Flux"
tcdc_aveclm:long_name = "atmos column total cloud cover"
tcdc_avehcl:long_name = "high cloud level total cloud cover"
tcdc_avelcl:long_name = "low cloud level total cloud cover"
tcdc_avemcl:long_name = "mid cloud level total cloud cover"
ulwrf_avetoa:long_name = "top of atmos upward longwave flux"
uswrf_ave:long_name = "averaged surface upward shortwave flux"
uswrf_avetoa:long_name = "top of atmos upward shortwave flux"
vbdsf_ave:long_name = "Visible Beam Downward Solar Flux"
vddsf_ave:long_name = "Visible Diffuse Downward Solar Flux"
…On Fri, Nov 21, 2025 at 11:11 AM Denise Worthen ***@***.***> wrote:
*DeniseWorthen* left a comment (NOAA-EMC/ufsatm#1038)
<#1038 (comment)>
@Qingfu-Liu <https://github.com/Qingfu-Liu> Your fix is for time-averaged
radiation fields that are written to the history files, right?
For the coupled model, the ATM exports the instantaneous (not average) SW
components
https://github.com/NOAA-EMC/ufsatm/blob/f6dc2070945042f89c7ab87aaaf3a6a02922b6c4/fv3/atmos_model.F90#L3496-L3506
Your changes don't impact variables like dvisbmi_cpl, right?
—
Reply to this email directly, view it on GitHub
<#1038 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGTS6UXWTQFMOZ77I7AP7233542SRAVCNFSM6AAAAACM2HUP5SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNRTG4YDQMZYGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@dpsarmie Can you run the regression tests? Although the code changes do not fix the coupled variables (which using extrapolation to estimate the instantaneous values) , but it does fix the time-averaged radiation variables. The fix should be added to the repository |
Yes, I will run the regression tests and get the logs posted. But I agree that we should also make sure that the fix is working as intended before moving forward. EPIC has been made aware that this will have priority and it will be merged when everything checks out. |
|
More comments on this PR: There are two different problems related to the radiation fields, one is the instantaneous values used for the coupling (the radiation time step is 3600s, and the physics time step or dynamic time steps are much smaller, for example 360s or 720s), so the coupling need to estimate the radiation flux at the coupling time step which needs to extrapolate the radiation fluxes from the earlier radiation calculation. This extrapolation is not a bug, but not accurate. The second problem is the time-averaged calculation which is a bug. The time-averaged quantities related to the radiation variables are wrong in current UFS community repository. |
|
@Qingfu-Liu I agree that the sawtooth in the exported SW is due to the extrapolation (inaccurate). I did not expect your fix for the history files to impact it. |
|
@Qingfu-Liu I ran the full regression test suite on Ursa for your UFS branch (https://github.com/Qingfu-Liu/ufs-weather-model/tree/fix_time_averaged_radiation) but I didn't see any baseline changes, which seemed odd. I reran on Hercules just to make sure and double-checked that these UFSATM changes were checked out. The rerun still had no changes to the baselines. Ruiyu's fix/reversion (ufs-community/ufs-weather-model@4ec82c9) had changes to baselines. I'm going to give this another look but I wanted to confirm. |
|
@dpsarmie Did you check the code change in the following two files: io/fv3atm_history_io.F90 and fv3/atmos_model.F90 compared to the community develop repository? |
|
@dpsarmie See the code difference here: https://github.com/ufs-community/ufs-weather-model/pull/2996/files |
|
Yes, here's a sample of the code that was compiled on Hercules: Path on Hercules is |
|
The changes in this PR have impact only in configurations where the output interval is smaller than the averaging interval, which I do not think is the case in any of our regression tests. |
Ok, thank you Dusan. I just wanted confirmation that this was expected behavior before moving forward with the PR. |
|
@Qingfu-Liu I'm going to go ahead and edit your UFS#2996 PR and reopen it so I can upload the logs and get it into the queue if that's ok with you. |
|
@dpsarmie and @DusanJovic-NOAA Thank you very much. |
@DusanJovic-NOAA Is this something that we want to allow? If not, we could add a test to atmos_init to check. |
I don't know. But that was the configuration that Larissa showed here (ufs-community/ufs-weather-model#1767 (comment)), and which was attempted to be fixed in a PR that we had to revert. If we restrict the output interval to be at least as the averaging, then this PR is not necessary. |
|
Are additional PR reviews needed before ufs-community/ufs-weather-model#2996 can mark that it's sub component PRs are reviewed? |
yangfanglin
left a comment
yangfanglin
left a comment
There was a problem hiding this comment.
agreed to move ahead with this update. The instantaneous flux used for coupling could be further improved for accuracy in the future.
|
@dpsarmie I saw PR#1038 has a line says that "Qingfu-Liu wants to merge 1 commit into NOAA-EMC:develop from Qingfu-Liu:develop". Why not from Qingfu-Liu:fix_time_averaged_radiation? Did I made a mistake to point to a wrong branch? By the way, I do see that your tests are using the correct branch. |
|
@DusanJovic-NOAA The calendar says @Qingfu-Liu is out for the next 3weeks. Can you update his branch w/ latest develop? I don't think I have permissions for that, do you? |
I can not push to his develop branch. |
|
OK, I'll re-create his branch and make a new PR. |
|
Merged via #1043. Closing. |
7 participants
Description
This PR fixes some of the UFS-ATM issue discussed in ufs-weather-model issues #1767.
ufs-community/ufs-weather-model#1767
The problem in retro tests has been discussed in detail in GFS issues #43:
NOAA-EMC/GFS#43 (comment)
@JongilHan66 noticed the time averaged cloud cover, which should range from 0-100%, only range across 0-4% in his experiment based on the retrotest16update branch . Abnormal ranges happen to all time averaged cloud cover, and related radiative fluxes. The instantaneous cloud covers and radiative fluxes are fine. The issue is from the UWM and exists in the sfc history files.
There are two different problems related to the radiation fields, one is the instantaneous values used for the coupling (the radiation time step is 3600s, and the physics time step or dynamic time steps are much smaller, for example 360s or 720s), so the coupling need to estimate the radiation flux at the coupling time step which needs to extrapolate the radiation fluxes from the earlier radiation calculation. This extrapolation is not a bug, but not accurate. The second problem is the time-averaged calculation which is a bug. The time-averaged quantities related to the radiation variables are wrong in current UFS community repository.
As Fanglin mentioned in his e-mail:
"This outstanding issue has been affecting us for some time. A number of people have been involved in the discussion or contributed code changes in the past few years. Thank you, in particular, Denise for bringing the issue to everyone's attention in the first place, Grant, for taking this over after Larissa's departure, Dusan, for providing the working solution, and Qingfu, for the testing and confirmation."
The changes of answers are expected from this PR.
Issue(s) addressed
In coupled configs, SW exported by ATM has odd time-dependence ufs-community/ufs-weather-model#1767
error in the averaged cloud cover and radiative fluxes GFS#43 (comment)
Testing
The code changes have been tested in ufs-weather-model regression tests, and the results have been plotted/diagnosed for all the radiation related variables.
The code updates change the regression test baseline if the output interval is smaller than the averaging interval, otherwise, the code will not change the test baseline.
(no regression test log files since I have not run all the baseline tests. Hope someone can help to run the regression tests and provide the log files)
Dependencies
No dependency