Cleanup work for adding tiice to restart files for runs without fractional grid #213

climbfuji · 2020-12-14T16:03:16Z

Description

When #212 was merged, it was decided that always storing the ice layer temperatures separately from the soil layer temperatures is cleaner and less confusing. Until then, they were only stored separately when the fractional grid was used, but combined into the soil layer temperature array when the binary grid was used.

Solution

The cleanup work includes:

remove the commented lines around adding tiice to the restart data in io/FV3GFS_io.F90
merge the register_restart_field calls for tiice with the calls for the other arrays above in io/FV3GFS_io.F90
in ccpp-physics, remove the code in GFS_surface_composites.F90 that stores tiice in slt

Testing:

This change will require new baselines (because the slt array in the restart and diagnostic files will be changed)

The text was updated successfully, but these errors were encountered:

ShanSunNOAA · 2020-12-14T16:06:01Z

Impressive bookkeeping - thanks for the info!

junwang-noaa · 2021-03-02T20:02:01Z

Some issues come up in regional LAM parallel because the tiice was added to restart sfc file in PR#212. LAM runs on non-fractional grid and does not need to have this 2 layers tiice array. Below is the description below from Eric Rogers.

In the version of chgres_cube we're running in the LAM parallels:

commit 19942e0a15ac48a9c45bf1301dff7c837158e556 (HEAD -> develop, origin/develop, origin/HEAD)
Date: Tue Dec 15 14:44:02 2020 -0500

these are the dimensions in the sfc_data file coming out of chgres_cube:

netcdf sfc_data_new {
dimensions:
xaxis_1 = 1754 ;
yaxis_1 = 1044 ;
zaxis_1 = 4 ;
Time = 1 ;

When I ran the LAMDA with the latest develop branch with has the regional inline post, I see an extra dimension added in the sfc_data file that comes out of the model:

netcdf sfc_data_new {
dimensions:
xaxis_1 = 1754 ;
yaxis_1 = 1044 ;
zaxis_1 = 2 ;
zaxis_2 = 4 ;
Time = 1 ;

This caused an abort in one of the codes Tom wrote that runs after the forecast (to address the boundary imbalance problem in the LAM DA runs), since it read the sfc_data file from the model and expected 4 dimensions. Changing this code to account for the extra dimension was trivial, and I was able to get a successful end-to-end run of the LAMDA with the inline post, but this difference may cause some confusion.

@climbfuji Please keep Eric Rogers included when the issue is resolved so that Eric can check if the tiice issue is gone in LAM.

climbfuji · 2021-03-02T20:34:06Z

Just to make sure we understand each other, tiice should remain in the output for both binary and fractional grid applications. If it causes a problem in one of Tom's codes, then that one will need to be fixed.

junwang-noaa · 2021-03-02T20:38:13Z

Maybe we misunderstood eath other. Can you remind me why tiice is required for restart for non-fractional grid runs?

…

On Tue, Mar 2, 2021 at 3:34 PM Dom Heinzeller ***@***.***> wrote: Just to make sure we understand each other, tiice should remain in the output for both binary and fractional grid applications. If it causes a problem in one of Tom's codes, then that one will need to be fixed. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7D6TPTK3QHDUKZUK4KCCTTBVDU3ANCNFSM4U26VHMA> .

climbfuji · 2021-03-02T20:45:31Z

Putting tiice into tsfc, somewhere buried in the physics has several serious implications.

extremely confusing, no developer knows what is going on, where the data is stored (hidden) and retrieved from
this kind of logic deals with host model limitations (or external code limitations, better to say) and does not belong in physics routines
if kice == number of ice levels == klsm == number of lsm levels, then the array is at least filled properly
if kice < klsm, you need to decide what to put into the layers kice+1:klsm - missing value? zero? yet another thing to get confused about
if kice > klsm, the code breaks - and in fact, kice ==9 for RUC LSM, for example --> putting them back into stc will break RUC lsm and any other potential ice model with kice > klsm

ShanSunNOAA · 2021-03-02T20:48:12Z

tiice was introduced for the fractional grid, where it was for internal ice temp. In the nonfractional case, internal ice temperature borrows the soil temperature slot, as ice and soil won't co-exist in this case. I thought tiice would store its value in the soil temperature slot in the nonfractioanl case prior writing to restart. In other words, tiice should be purely internal for nonfractional case. Will check when hera is back. Thanks, Shan

…

On Tue, Mar 2, 2021 at 1:38 PM Jun Wang ***@***.***> wrote: Maybe we misunderstood eath other. Can you remind me why tiice is required for restart for non-fractional grid runs? On Tue, Mar 2, 2021 at 3:34 PM Dom Heinzeller ***@***.***> wrote: > Just to make sure we understand each other, tiice should remain in the > output for both binary and fractional grid applications. If it causes a > problem in one of Tom's codes, then that one will need to be fixed. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#213 (comment)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AI7D6TPTK3QHDUKZUK4KCCTTBVDU3ANCNFSM4U26VHMA > > . > — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALORMVVCK4TZ4EN5NTDA7XDTBVEEJANCNFSM4U26VHMA> .

climbfuji · 2021-03-02T20:49:30Z

This is all confusing ... no matter what, if kice > klsm it will break.

climbfuji · 2021-03-02T20:51:29Z

tiice is always written to the restart files (and read from it), no matter whether fractional or not. Simply because of the flaws of this design choice that I mentioned above. See current FV3/io/FV3GFS_io.F90.

climbfuji · 2021-03-02T20:53:14Z

fv3atm/io/FV3GFS_io.F90

Line 908 in 4908898

! if (Model%frac_grid) then

junwang-noaa · 2021-03-02T21:08:37Z

So tiice is not saved in tsfc any more even for the situation of non-fractional grid with kice<klsm. Then the downstream jobs may need to know this change and update accordingly.

climbfuji · 2021-03-02T21:19:25Z

So tiice is not saved in tsfc any more even for the situation of non-fractional grid with kice<klsm. Then the downstream jobs may need to know this change and update accordingly.

Yep, seems to be the cleanest solution.

SMoorthi-emc · 2021-03-02T21:22:25Z

I thought I modified to write tiice separately to avoid these issues.

…

Sent from my iPhone

On Mar 2, 2021, at 3:49 PM, Dom Heinzeller ***@***.***> wrote: This is all confusing ... no matter what, if kice > klsm it will break. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

SMoorthi-emc · 2021-03-02T21:23:19Z

I thought that is what I did. Moorthi

…

Sent from my iPhone

On Mar 2, 2021, at 3:51 PM, Dom Heinzeller ***@***.***> wrote: tiice is always written to the restart files (and read from it), no matter whether fractional or not. Simply because of the flaws of this design choice that I mentioned above. See current FV3/io/FV3GFS_io.F90. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

yangfanglin · 2021-03-02T21:34:27Z

For the GSM/GFS, tsfc has always been used to represent skin temperatures over all surface types/grids, including land, ice and water. Has this been changed ? There are many downstream users relying on this product. On Tue, Mar 2, 2021 at 4:23 PM SMoorthi-emc <notifications@github.com> wrote:

…

I thought that is what I did. Moorthi Sent from my iPhone > On Mar 2, 2021, at 3:51 PM, Dom Heinzeller ***@***.***> wrote: > > tiice is always written to the restart files (and read from it), no matter whether fractional or not. Simply because of the flaws of this design choice that I mentioned above. See current FV3/io/FV3GFS_io.F90. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub, or unsubscribe. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKY5N2PCZX5KGOFD7CHL2GTTBVJNPANCNFSM4U26VHMA> .

-- *Fanglin Yang, Ph.D.* *Chief, Model Physics Group* *Modeling and Data Assimilation Branch* *NOAA/NWS/NCEP Environmental Modeling Center* *https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/ <https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/>*

SMoorthi-emc · 2021-03-02T21:39:10Z

Tsfc has nothing to do with tiice. They are different. Tiice is only in restart (I can’t see the code now - I will confirm later) Moorthi

…

Sent from my iPhone

On Mar 2, 2021, at 4:34 PM, Fanglin Yang ***@***.***> wrote: For the GSM/GFS, tsfc has always been used to represent skin temperatures over all surface types/grids, including land, ice and water. Has this been changed ? There are many downstream users relying on this product. On Tue, Mar 2, 2021 at 4:23 PM SMoorthi-emc ***@***.***> wrote: > I thought that is what I did. > Moorthi > > Sent from my iPhone > > > On Mar 2, 2021, at 3:51 PM, Dom Heinzeller ***@***.***> > wrote: > > > > tiice is always written to the restart files (and read from it), no > matter whether fractional or not. Simply because of the flaws of this > design choice that I mentioned above. See current FV3/io/FV3GFS_io.F90. > > > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub, or unsubscribe. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#213 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AKY5N2PCZX5KGOFD7CHL2GTTBVJNPANCNFSM4U26VHMA> > . > -- *Fanglin Yang, Ph.D.* *Chief, Model Physics Group* *Modeling and Data Assimilation Branch* *NOAA/NWS/NCEP Environmental Modeling Center* *https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/ <https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/>* — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

climbfuji · 2021-03-02T21:40:29Z

I thought I modified to write tiice separately to avoid these issues.
…

Yes, that is what you did and what should remain. The cleanup work here was just a follow-up, not a reversal but got misinterpreted.

@yangfanglin this is about tiice and stc, not tsfc.

junwang-noaa · 2021-03-02T21:54:45Z

I think the impact is that model used to output internal ice temp in stc (soil temp) on ice points, but now the internal ice temp is saved in its own array, I believe the stc does not contain the ice temp any more(restart non-reproducible for non-fractional grid uncoupled with tiice) . At this time, we also need to update the GFS_diagnostics.F90 to allow it output to model history files for non-fractional grid runs (e.g. LAM). The tsfc does not change for non-fractional grid, but is the composite temp for fractional grid.

…

On Tue, Mar 2, 2021 at 4:40 PM Dom Heinzeller ***@***.***> wrote: I thought I modified to write tiice separately to avoid these issues. … <#m_-3481226871809676753_> Yes, that is what you did and what should remain. The cleanup work here was just a follow-up, not a reversal but got misinterpreted. @yangfanglin <https://github.com/yangfanglin> this is about tiice and stc, not tsfc. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7D6TP7CYGRBQUNS4572VLTBVLN5ANCNFSM4U26VHMA> .

SMoorthi-emc · 2021-03-02T22:01:36Z

There is tiice in non fractional grid also. It may also be in stc for non fractional case. I did not add tiice to history partly because I don’t know enough about it plus also did not want change history Moorthi

…

Sent from my iPhone

On Mar 2, 2021, at 4:55 PM, Jun Wang ***@***.***> wrote: I think the impact is that model used to output internal ice temp in stc (soil temp) on ice points, but now the internal ice temp is saved in its own array, I believe the stc does not contain the ice temp any more(restart non-reproducible for non-fractional grid uncoupled with tiice) . At this time, we also need to update the GFS_diagnostics.F90 to allow it output to model history files for non-fractional grid runs (e.g. LAM). The tsfc does not change for non-fractional grid, but is the composite temp for fractional grid. On Tue, Mar 2, 2021 at 4:40 PM Dom Heinzeller ***@***.***> wrote: > I thought I modified to write tiice separately to avoid these issues. > … <#m_-3481226871809676753_> > > Yes, that is what you did and what should remain. The cleanup work here > was just a follow-up, not a reversal but got misinterpreted. > > @yangfanglin <https://github.com/yangfanglin> this is about tiice and > stc, not tsfc. > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#213 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AI7D6TP7CYGRBQUNS4572VLTBVLN5ANCNFSM4U26VHMA> > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

- All `#ifdef OPENMP` are replaced with `#ifdef _OPENMP` to follow standard compiler macro for OpenMP. - CCPP code generation is moved inside of `FV3/`. This will be redundant when the Data Atmosphere component comes in. Updates to the python script was performed to remove the hard-wired `FV3/` path name in the file. - `cmake/GNU.cmake` and `cmake/Intel.cmake` hard-wire global compiler definitions with `add_definitions`. These are now applied with `target_compile_definitions`. - CMakeLists.txt is passed through cmake-norm checker. - `INSTALL_INTERFACE` is added, so that this component can be installed and linked in downstream applications e.g. JEDI. This may be incomplete, so will need more work.

DeniseWorthen · 2022-09-12T15:57:21Z

This is a very old issue. I will close. Please re-create the issue if required.

…3/SAS/MYNN fix) (#865) * Host side changes for h2o photochemistry scheme * A fix for the issue to run C3 or SAS convection with the prognostic area fraction closure, and MYNN PBL: tendency_of_vertically_diffused_tracer_concentration from MYNN PBL --------- Co-authored-by: Dustin Swales <dustin.swales@noaa.gov> Co-authored-by: Lisa Bengtsson <Lisa.Bengtsson@noaa.gov>

climbfuji added the enhancement New feature or request label Dec 14, 2020

climbfuji mentioned this issue Dec 14, 2020

Final-final GFS v16 updates / restart reproducibility bugfixes #212

Merged

climbfuji assigned climbfuji and ShanSunNOAA Dec 14, 2020

DeniseWorthen closed this as completed Sep 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup work for adding tiice to restart files for runs without fractional grid #213

Cleanup work for adding tiice to restart files for runs without fractional grid #213

climbfuji commented Dec 14, 2020

ShanSunNOAA commented Dec 14, 2020

junwang-noaa commented Mar 2, 2021

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

ShanSunNOAA commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

climbfuji commented Mar 2, 2021

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021

climbfuji commented Mar 2, 2021

SMoorthi-emc commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

yangfanglin commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

DeniseWorthen commented Sep 12, 2022

Cleanup work for adding tiice to restart files for runs without fractional grid #213

Cleanup work for adding tiice to restart files for runs without fractional grid #213

Comments

climbfuji commented Dec 14, 2020

Description

Solution

Testing:

ShanSunNOAA commented Dec 14, 2020

junwang-noaa commented Mar 2, 2021

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

ShanSunNOAA commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

climbfuji commented Mar 2, 2021

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021

climbfuji commented Mar 2, 2021

SMoorthi-emc commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

yangfanglin commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

climbfuji commented Mar 2, 2021

junwang-noaa commented Mar 2, 2021 via email

SMoorthi-emc commented Mar 2, 2021 via email

DeniseWorthen commented Sep 12, 2022