Bug fixes related to solar diag package: COS(zenith angle), non-existent hydrometeors, shortwave irradiances in time series

[twjuliano]

TYPE: bug fix, enhancement

KEYWORDS: solar diagnostic, shortwave irradiance, solar zenith, hydrometeor, cloud

SOURCE: Timothy W. Juliano, Ju-Hye Kim, Pedro A. Jimenez, Jared Lee, Thomas Brummet (NCAR/RAL)

DESCRIPTION OF CHANGES:
Bug fixes
Problem:
1. Value of cosine solar zenith angle (coszen) can be slightly <-1 or slightly >1, which results in a nonphysical value for diagnosis of the solar zenith angle
2. Calculating cloud parameters in solar diagnostic package when hydrometeor type is not present

Solution:
1. Bound the values of coszen to be [-1, 1]
2. Add conditional check in solar diagnostic module for presence of hydrometeor type before calculating cloud parameters. Also add check for summation of water path variables to ensure >=0.0 kg/m2.

Enhancement
We have added the Global Horizontal Irradiance (SWDOWN), Direct Normal Irradiance (SWDDNI), and Diffuse Horizontal Irradiance (SWDDIF), as well as the clear-sky Global Horizontal Irradiance (SWDOWNC) and clear-sky Direct Normal Irradiance (SWDDNIC) variables to the tslist when the solar diagnostic package is activated. Furthermore, we add these five variables as calculated by FARMS: SWDOWN2, SWDDNI2, SWDDIF2, SWDOWNC2, and SWDDNIC2.

All 10 variables are output regardless of model physics configuration. Variables SWDOWN, SWDDNI, and SWDDIF are calculated when any shortwave radiation scheme is active. If the RRTMG (ra_sw_physics=4) or RRTMG FAST (ra_sw_physics=24) shortwave radiation scheme or FARMS (swint_opt=2) is activated, then variables SWDOWNC and SWDDNIC are calculated [otherwise, set to MISSING (=-999.0)]. If FARMS (swint_opt=2) is activated, then variables SWDOWN2, SWDDNI2, SWDDIF2, SWDOWNC2, and SWDDNIC2 are calculated [otherwise, set to MISSING (=-999.0)].

LIST OF MODIFIED FILES:
M Registry/Registry.EM_COMMON
M Registry/registry.solar_fields
M phys/module_diag_solar.F
M phys/module_diagnostics_driver.F
M phys/module_radiation_driver.F
M run/README.tslist
M share/wrf_timeseries.F

TESTS CONDUCTED:
1. We have run simulations with and without the fixes. Problem 1: When coszen is within the range [-1, 1], the model results are bit for bit identical before and after the change. Problem 2: The model output confirms that the cloud parameters are not calculated when the hydrometeor type is not present and also that the water path variables are positive. Enhancement: The tslist output is written as expected.
2. Jenkins testing is all PASS.

RELEASE NOTE: Several fixes are introduced for the solar diagnostic package: 1) Correct solar zenith angle calculation; and 2) check presence of hydrometeor type before calculating cloud parameters. An enhancement to the package is also introduced: Adding shortwave irradiance variables to the tslist when activating the RRTMG or RRTMG FAST shortwave radiation scheme or FARMS.

[davegill]

@twjuliano
Tim,
In the PR title, include "incorrect time" and "cloud parameters" for more specificity.

[twjuliano]

@davegill @pedro-jm
Dave,
I have addressed your comments above.

We have also added a fix for the coszen calculation so that it does not result in nonphysical values when the solar zenith angle is diagnosed. Moreover, shortwave irradiance variables from WRF-Solar are added to the tslist when the solar diagnostic package is activated. We forgot to include these previously, and they are important variables to output when using WRF-Solar.

Thanks,
Tim

[davegill]

@weiwangncar @dudhia
Folks,
Are we OK that there are bug fixes and new enhancements together in a single PR? I can go either way on this. Are there existing post-processors that would have trouble reading the newly manufactured files? Seems like it is just extra data that is tagged on the end of a formatted WRITE, so there should be no impact.

[davegill]

@twjuliano

Furthermore, we add these five variables as calculated by FARMS if the scheme is activated (swint_opt=2). These variables from FARMS are: SWDOWN2, SWDDNI2, SWDDIF2, SWDOWNC2, and SWDDNIC2.

Tim,
So the additional variables are not output unless swint_opt=2? Has this been run without swint_opt=2 to verify the behavior is OK?

[twjuliano]

@davegill

Tim,
So the additional variables are not output unless swint_opt=2? Has this been run without swint_opt=2 to verify the behavior is OK?

Dave,
Apologies for the confusion; the additional variables are output regardless of namelist configuration. The values are set to MISSING (=-999.0) depending upon the namelist options. Therefore, no issues with tslist writing behavior. I've clarified this in the PR text.

[weiwangncar]

I would prefer the bug fixes are separated from enhancement, if this is not too much to ask.. And that is definitely the thing to do in the future. If they are separated, it could mean the enhancement may not get into release branch until next year...

[pedro-jm]

Adding the time series of the irradiances can be considered a bug fix on our end. The main motivation for developing the solar diagnostic package was to have the irradiance time series which was the last of our old WRF-Solar functionality to be ported. And somehow we forgot to add these vars! If we cannot do it now we'll wait obviously.

[dudhia]

I have no problem with this as I think it only affects the time series. Which modules are affected by this?

…

On Mon, Sep 14, 2020 at 4:17 PM pedro-jm ***@***.***> wrote: Adding the time series of the irradiances can be considered a bug fix on our end. The main motivation for developing the solar diagnostic package was to have the irradiance time series which was the last of our old WRF-Solar functionality to be ported. And somehow we forgot to add these vars! If we cannot do it now we'll wait obviously. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1284 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77HNSUF2DYAAZRA2J3DSF2JAXANCNFSM4RH5U5LA> .

[twjuliano]

@dudhia

I have no problem with this as I think it only affects the time series. Which modules are affected by this?

That is correct. Aside from Registry files and README.tslist, only share/wrf_timeseries.F is impacted by this.

[vikramjakhr]

Reopening for testing

[weiwangncar]

@twjuliano Can you confirm that this PR does not impact WRF model results apart from FARMS output and time series?

[twjuliano]

@twjuliano Can you confirm that this PR does not impact WRF model results apart from FARMS output and time series?

@weiwangncar If solar_diagnostics=1, then this PR changes the model results because the coszen calculation is different, which ultimately impacts the radiation calculations. However, if solar_diagnostics=0, then this this PR does not impact WRF model results with the exception of possible instances when coszen <-1 or >1. In my tests, the model results before and after the PR using solar_diagnostics=0 were identical using diffwrf, so @pedro-jm will have to comment on how often this coszen bounding may occur, as I have not experienced this issue. Thanks.

[weiwangncar]

@twjuliano Can you explain the last change?

[twjuliano]

@twjuliano Can you explain the last change?

@weiwangncar After discussing with @pedro-jm we have decided that my modification for including a different time in the subroutine call for calc_coszen is not correct. I initially made this modification because the output coszen value is not correct when the solar diagnostic package is activated. However, I did not realize that it would change the code in an undesirable way. Therefore, for this PR, we will leave this issue untouched. We will think about the best approach to fix it and leave it for a future PR. Apologies for the confusion.

Now, with the latest commit, I can confirm that this PR does not impact WRF model results (i.e., bit for bit reproducibility between simulations using the old and new code when coszen is within the range [-1, 1]).

[weiwangncar]

@twjuliano Thanks. @dudhia Can you comment on the issue of coszen value falling out of the range [-1,1]?

[dudhia]

I think that change was previously not correct because it would change the results with the diagnostics on due to the change in effective xtime.

…

On Thu, Oct 8, 2020 at 4:48 PM weiwangncar ***@***.***> wrote: @twjuliano <https://github.com/twjuliano> Can you explain the last change? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1284 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77A77HITCQ36HH6XECTSJY6VRANCNFSM4RH5U5LA> .

[dudhia]

Regarding the limiting. I am not sure how often it happens or how far above 1 it gets in magnitude. Would be good to know. Adding this limit is a bandaid, but I think the diagnostics need it within this range to work, so it is valid.

[twjuliano]

I think that change was previously not correct because it would change the results with the diagnostics on due to the change in effective xtime.
…
On Thu, Oct 8, 2020 at 4:48 PM weiwangncar @.***> wrote: @twjuliano https://github.com/twjuliano Can you explain the last change? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1284 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEIZ77A77HITCQ36HH6XECTSJY6VRANCNFSM4RH5U5LA .

@dudhia Yes, that is correct.

Regarding the limiting. I am not sure how often it happens or how far above 1 it gets in magnitude. Would be good to know. Adding this limit is a bandaid, but I think the diagnostics need it within this range to work, so it is valid.

@dudhia I will defer to @pedro-jm regarding how often it happens, as he introduced this bug fix. Thanks.

[pedro-jm]

I do not think this happens very often. I took a look at one point and coszen was slightly lower than -1 or slightly higher than 1 in one time step only. It was something like a round off error. Although small, the solar diagnostic package needs it because the solar zenit angle is calculated as the inverse function of the cosine (arc cos). If cosezen does not belong to [-1, 1] then we have a bug.
Pedro.

[dudhia]

Seems reasonable to have. It is the only change outside the diagnostics.

…

On Fri, Oct 9, 2020 at 3:26 PM pedro-jm ***@***.***> wrote: I do not think this happens very often. I took a look at one point and coszen was slightly lower than -1 or slightly higher than 1 in one time step only. It was something like a round off error. Although small, the solar diagnostic package needs it because the solar zenit angle is calculated as the inverse function of the cosine (arc cos). If cosezen does not belong to [-1, 1] then we have a bug. Pedro. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1284 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77BFZRX3F2RF3VKDHJ3SJ55YRANCNFSM4RH5U5LA> .

[weiwangncar]

@dudhia Should we move this change to a separate PR? This will have an impact, however small, to the bit wise results.

[dudhia]

I think it’s ok. It is needed for the new code. Jimy

…

On Fri, Oct 9, 2020 at 4:25 PM weiwangncar ***@***.***> wrote: @dudhia <https://github.com/dudhia> Should we move this change to a separate PR? This will have an impact, however small, to the bit wise results. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1284 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIZ77ANP2FZXXIZBHEI7X3SJ6EVZANCNFSM4RH5U5LA> .

[dudhia]

When the diagnostics are set as MISSING how do they appear in the output versus before the fix?

[weiwangncar]

@twjuliano Would you like to comment on Jimy's last post?

[davegill]

@weiwangncar @dudhia
Folks,
So the status of this PR:

1. The "enhancement" portion (adding in the time series variables) really is also part of the bug fix, as the time series code was accidentally left out of the original PR. I am OK with that explanation.
2. I agree that the desired bounding of the COS to be between [-1,1] should be included. I am not concerned about bit-wise differences that occur when necessary fixes are applied.
3. The tests for cloud, ice, and snow account for unavailable fields in the computations.

From a position of evaluating this PR mechanically, I am OK with this code.

[davegill]

jenkins

Please find result of the WRF regression test cases in the attachment. This build is for Commit ID: 19d71b854a5eabae8a7bc82fbcf48533628b05f0, requested by: twjuliano for PR: https://github.com/wrf-model/WRF/pull/1284. For any query please send e-mail to David Gill.

    Test Type              | Expected  | Received |  Failed
    = = = = = = = = = = = = = = = = = = = = = = = =  = = = =
    Number of Tests        : 19           18
    Number of Builds       : 48           46
    Number of Simulations  : 166           164        0
    Number of Comparisons  : 105           104        0

    Failed Simulations are: 
    None
    Which comparisons are not bit-for-bit: 
    None

[twjuliano]

When the diagnostics are set as MISSING how do they appear in the output versus before the fix?

@twjuliano Would you like to comment on Jimy's last post?

@dudhia @weiwangncar Before the fix, the diagnostics would have value '0' and now they show '-999' (i.e., MISSING).

[weiwangncar]

@davegill Can you comment on my last comment?

[davegill]

@weiwangncar
Thanks for the reminder. I commented in the block of code for module_solar_diag.F, lines 278-280.

[twjuliano]

@weiwangncar @davegill
Wei and Dave-- Thank you for your help to make this code up to standards. Hopefully my latest commit resolves any issues.

Test cases with WSM3 and Thompson MP have been run to ensure that results are OK with new logic.

[davegill]

Approved