Fix sea surface temperature cycling

[spencerkclark]

This is the alternative fix mentioned in #92. This works without having to change the regression test checksums, which we might hope would be the case considering that the regression test only runs for 30 minutes and has a cycling frequency of 24 hours. In theory any change we make here shouldn't change answers, since the model isn't run long enough for cycling to be called.

Tests can be found in this notebook. EMC seems to think this fix is the correct approach.

[oliverwm1]

LGTM, thanks for the fix! Do you an idea of the constant SST is an issue in the frac_grid=True case as well?

[spencerkclark]

Do you an idea of the constant SST is an issue in the frac_grid=True case as well?

Good question -- it appears that it is an issue in the existing codebase, but this fix applies to it too. See the experiments at the end of this notebook.

[spencerkclark]

Good question -- it appears that it is an issue in the existing codebase, but this fix applies to it too. See the experiments at the end of this notebook.

Oops I take this back...upon reading the code again, this code change would not be active in the frac_grid = .true. case. This left me puzzled as to why my test cases behaved the way they did. Apparently the frac_grid parameter is automatically reset to .false. if a lake_frac variable cannot be found in the orography files (meaning we need updated orography files to run with frac_grid = .true.).

See this note in the logs:

NOTE from PE     0: gfs_driver::surface_props_input - will computing lakefrac
  resetting Model%frac_grid= F

and this spot in the code:
https://github.com/VulcanClimateModeling/fv3gfs-fortran/blob/0db9e15aa92df353b4f88f289b17ec429628b307/FV3/io/FV3GFS_io.F90#L651-L658

If I had to guess, I would say that relaxing SSTs back to the climatology is also broken in the frac_grid = .true. case -- I don't see a place where tsfco is updated to the tsfc over ocean grid cells when frac_grid = .true. and this fix doesn't help that. Perhaps that indicates a better fix would be to update these lines instead:

https://github.com/VulcanClimateModeling/fv3gfs-fortran/blob/0db9e15aa92df353b4f88f289b17ec429628b307/FV3/gfsphysics/GFS_layer/GFS_physics_driver.F90#L1184-L1185

by changing tsfco to tsfc, since those get called whether frac_grid = .true. or not?

[spencerkclark]

I suppose it's a little awkward in the frac_grid = .true. case that we would use tsfc as the reference ocean temperature in tsfc3 when tsfc itself is computed as a weighted average of the land, sea-ice, and sea surface temperatures later on in the physics driver:

https://github.com/VulcanClimateModeling/fv3gfs-fortran/blob/0db9e15aa92df353b4f88f289b17ec429628b307/FV3/gfsphysics/GFS_layer/GFS_physics_driver.F90#L1957

Will revert back to 74c4015 for now. I think it's better to leave that possibly broken rather than have a potentially incorrect fix (I also can't test the frac_grid = .true. case without updated orography files for the reason noted above). Maybe something along the lines of #92 (i.e. cycling the ocean temperature rather than the full surface temperature) is the right long-term fix, but the fact that it changes the checksums makes me suspicious.

I'm reasonably confident that in the frac_grid = .false. case 74c4015 does the right thing.

[oliverwm1]

Hey, yeah sorry didn't mean to send you down a rabbit hole. Was just curious if we at least knew if it would work in that case. Agreed it's not a priority to run with frac_grid=True.

[rheacangeo]

What you needed was there but not!!