Skip to content

Conversation

@rgknox
Copy link
Contributor

@rgknox rgknox commented Nov 20, 2025

These changes ensure that zenith angles are passed to FATES on non-continue restarts on the first step. The legacy FATES radiative transfer scheme (Norman) was not sensitive to this, but the newer two-stream scheme needs this information at this point in the call sequence.

Comment on lines 1377 to 1379
if ( (is_cold_start .and. get_nstep() == 1) .or. &
((fates_radiation_model == 'twostream') .and. (get_nstep()== 1) .and. (.not.use_fates_sp) &
.and. (.not.is_cold_start) .and. (nsrest == nsrStartup)) ) then
Copy link
Contributor

Choose a reason for hiding this comment

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

@rgknox , can we simplify these conditionals or break them down so they are a bit more manageable to understand?

if (get_nstep() == 1) then

   do_update = .false.

   if (is_cold_start)
       do_update = .true.
   else if (<check-second-conditional>)
       do_update = .true.
   endif

   if (do_update) then
      call alm_fates%wrap_canopy_radiation(bounds_clump,surfalb_vars,nextsw_cday,declinp1)
   endif

endif 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, good idea! I'll submit a change

@rgknox
Copy link
Contributor Author

rgknox commented Dec 30, 2025

Hehe, @bishtgautam , I was able to make it way simpler. I don't know how all that extra logic got in there. Anyway, take a look and see what you think. Thanks for the suggestion

@rgknox
Copy link
Contributor Author

rgknox commented Dec 30, 2025

NorESM has these land-model tests called Land Tuning Mode. I think that these tests simulate frequency of the radiation boundary conditions when running coupled with the atmosphere, although it still uses a data atmosphere for the information. Do we have these types of tests in E3SM? I would add them to the FATES test list.

@bishtgautam
Copy link
Contributor

Hi @rgknox, thanks for simplifying the logic. How about also adding a comment explaining what is happening? something like the following, based on your first comment.

! On the first step, send the solar zenith angles to FATES on non-continue restarts
! for the two-stream radiation scheme.

@glemieux
Copy link
Contributor

glemieux commented Jan 5, 2026

@bishtgautam I talked with Ryan and I'll add your comment and then kick off e3sm land developer tests soon.

@glemieux
Copy link
Contributor

glemieux commented Jan 5, 2026

Regression testing e3sm_land_developer against the master baseline is underway on perlmutter.

@glemieux glemieux moved this from Under Review to Final Testing in FATES Pull Request Planning and Status Jan 6, 2026
@glemieux
Copy link
Contributor

glemieux commented Jan 7, 2026

Regression testing against the master baseline on perlmutter is complete and showing only B4B differences for the subset of FATES test mods. @rgknox is this expected?

Results: /global/homes/g/glemieux/scratch/e3sm-tests/pr7899-zenith.fates.pm-cpu..E3d3963866b-F5bb36cba2

@glemieux
Copy link
Contributor

glemieux commented Jan 8, 2026

Regression testing against the master baseline on perlmutter is complete and showing only B4B differences for the subset of FATES test mods. @rgknox is this expected?

Results: /global/homes/g/glemieux/scratch/e3sm-tests/pr7899-zenith.fates.pm-cpu..E3d3963866b-F5bb36cba2

I'm thinking this is likely due to the fact that I didn't rebase this against master prior to testing. The code only impacts two-stream runs and Norman is currently the default. I'm currently retesting this after rebasing.

@glemieux
Copy link
Contributor

glemieux commented Jan 9, 2026

I'm thinking this is likely due to the fact that I didn't rebase this against master prior to testing. The code only impacts two-stream runs and Norman is currently the default. I'm currently retesting this after rebasing.

I'm still seeing unexpected diffs. I'm generating my own baseline using master to triple check, just in case the master baseline wasn't built with the commits I think it's should be representing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ELM land model FATES

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants