Corrects the calculation of solar zenith angle, which was off by a timestep

[jonbob]

The solar zenith angle calculation used by the data models was off by one timestep. This issue was first reported by CESM in ESMCI Issue #3380 and subsequently fixed in ESCOMP PR #123. This PR implements that solution in E3SM.

Fixes #4575

[non-BFB] for configurations with data models

[proteanplanet]

I am running tests on this, and will approve once I've checked the impact on albedo.

[rljacob]

@proteanplanet what did your tests say?

[darincomeau]

I ran two 10 year Cryo JRA G-cases, one with this bug fix and one without, and ran MPAS-Analysis comparison on the two runs. Impacts are minimal. Here's annual SST climatology difference:, with the global max difference being 0.077 degrees.

Impacts on sea ice are indiscernible at the default MPAS-Analysis scales.

Link to full comparison analysis:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.dcomeau/20211011.GMPAS-JRA1p4-DIB-ISMF.TL319_ECwISC30to60E2r1.fix.anvil_vs_control/yrs1-10/

While I'm not familiar with the section of code being altered, approving based on testing revealing no negative impacts to Cryosphere G-cases.

[proteanplanet]

@proteanplanet what did your tests say?

I am swamped at the moment, but am getting to it. I thorough test is needed on a step-by-step basis of several coincident shortwave fields to ensure synchronization, which I am preforming.

[rljacob]

@proteanplanet asking again about your test results.

[proteanplanet]

@rljacob Sorry Rob, I have been snowed under, and hope I can get to this in the coming week. It requires quite a lot of work and needs surgical fastidiousness before I'm willing to approve this. There have been multiple mistakes made, some claiming to fix previous mistakes, with the radiation coupling with sea ice in G-cases in CESM and E3SM in recent years, and I don't want another bug to enter into the system, because they significantly affect model results.

[jonbob]

@proteanplanet is out on extended medical leave and told me to go ahead and merge this PR. I'll remove him and @maltrud as reviewers and get it merged

[rljacob]

@jonbob is this ready then?

[jonbob]

Results from running e3sm_developer on chrysalis with a test merge:
FAILS

FAIL ERS.ELM_USRDAT.I1850ELM.chrysalis_intel.elm-usrdat BASELINE master: DIFF
FAIL ERS.f09_f09.IELM.chrysalis_intel.elm-solar_rad BASELINE master: DIFF
FAIL ERS.f09_g16.I1850ELMCN.chrysalis_intel BASELINE master: DIFF
FAIL ERS.f09_g16.I1850ELMCN.chrysalis_intel.elm-bgcinterface BASELINE master: DIFF
FAIL ERS.f09_g16.I1850GSWCNPRDCTCBC.chrysalis_intel.elm-vstrd BASELINE master: DIFF
FAIL ERS.f09_g16.IELMBC.chrysalis_intel BASELINE master: DIFF
FAIL ERS.f19_f19.I1850ELMCN.chrysalis_intel BASELINE master: DIFF
FAIL ERS.f19_f19.I20TRELMCN.chrysalis_intel BASELINE master: DIFF
FAIL ERS.f19_f19.IELM.chrysalis_intel BASELINE master: DIFF
FAIL ERS.f19_g16.I1850CNECACNTBC.chrysalis_intel.elm-eca BASELINE master: DIFF
FAIL ERS.f19_g16.I1850CNECACTCBC.chrysalis_intel.elm-eca BASELINE master: DIFF
FAIL ERS.f19_g16.I1850CNRDCTCBC.chrysalis_intel.elm-rd BASELINE master: DIFF
FAIL ERS.f19_g16.I1850ELM.chrysalis_intel.elm-betr BASELINE master: DIFF
FAIL ERS.f19_g16.I1850ELM.chrysalis_intel.elm-vst BASELINE master: DIFF
FAIL ERS.f19_g16.I1850GSWCNPECACNTBC.chrysalis_intel.elm-eca_f19_g16_I1850GSWCNPECACNTBC BASELINE master: DIFF
FAIL ERS.f19_g16.I1850GSWCNPRDCTCBC.chrysalis_intel.elm-ctc_f19_g16_I1850GSWCNPRDCTCBC BASELINE master: DIFF
FAIL ERS.f19_g16.I20TRGSWCNPECACNTBC.chrysalis_intel.elm-eca_f19_g16_I20TRGSWCNPECACNTBC BASELINE master: DIFF
FAIL ERS.f19_g16.I20TRGSWCNPRDCTCBC.chrysalis_intel.elm-ctc_f19_g16_I20TRGSWCNPRDCTCBC BASELINE master: DIFF
FAIL ERS_Ld20.f45_f45.IELMFATES.chrysalis_intel.elm-fates BASELINE master: DIFF
FAIL ERS.MOS_USRDAT.RMOSGPCC.chrysalis_intel.mosart-mos_usrdat BASELINE master: DIFF
FAIL ERS.ne11_oQU240.I20TRELM.chrysalis_intel BASELINE master: DIFF
FAIL ERS.r05_r05.IELM.chrysalis_intel.elm-V2_ELM_MOSART_features BASELINE master: DIFF
FAIL ERS.r05_r05.RMOSGPCC.chrysalis_intel.mosart-gpcc_1972 BASELINE master: DIFF
FAIL SMS.f09_g16_a.IGELM_MLI.chrysalis_intel BASELINE master: DIFF
FAIL SMS_Ld1.hcru_hcru.I1850CRUELMCN.chrysalis_intel BASELINE master: DIFF
FAIL SMS_Ld20.f45_f45.IELMFATES.chrysalis_intel.elm-fates_eca BASELINE master: DIFF
FAIL SMS_Ld20.f45_f45.IELMFATES.chrysalis_intel.elm-fates_rd BASELINE master: DIFF
FAIL SMS_Ld30.f45_f45.IELMFATES.chrysalis_intel.elm-fates_satphen BASELINE master: DIFF
FAIL SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-force_netcdf_pio BASELINE master: DIFF
FAIL SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-lulcc_sville BASELINE master: DIFF
FAIL SMS_Ly2_P1x1.1x1_smallvilleIA.IELMCNCROP.chrysalis_intel.elm-per_crop BASELINE master: DIFF
FAIL SMS.MOS_USRDAT.RMOSGPCC.chrysalis_intel.mosart-unstructure BASELINE master: DIFF
FAIL SMS.r05_r05.I1850ELMCN.chrysalis_intel.elm-qian_1948 BASELINE master: DIFF
FAIL SMS.r05_r05.IELM.chrysalis_intel.elm-topounit BASELINE master: DIFF

So it's pretty much just making DIFFs for I-cases, which is expected. OK to merge, @rljacob? Or should we get someone from ELM to review?

[jonbob]

@bishtgautam - I requested your review just because this changes I-case results, whenever you have time

[bishtgautam]

@jonbob This is an important fix and thanks for bringing this into e3sm master.

[jonbob]

previously tested with e3sm_developer, new merge passes sanity testing.

Merged to next

[jonbob]

merged to master and expected DIFFs blessed

[rljacob]

@susburrows @jenniferholm @bpbond Do you want this fix in 2.1 simulations? If not, we'll create the maint-2.1 branch before this for you.

[rljacob]

Actually this doesn't affect the F-cases you're going to for v2.1 so nevermind.

[bishtgautam]

(Not sure if this matters) This PR will result in a non-BFB offline spinup of land BGC.