Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure sfcsub.F of global_cycle is linked to the version used by the forecast model. #424

Closed
GeorgeGayno-NOAA opened this issue Mar 18, 2021 · 37 comments · Fixed by #636
Closed
Assignees
Labels
maintenance Basic upkeep

Comments

@GeorgeGayno-NOAA
Copy link
Collaborator

The sfcsub.F module is used by global_cycle AND the forecast model. Both programs should use the same version. Need a way to ensure they are always linked up.

Forecast model version: https://github.com/NCAR/ccpp-physics/blob/master/physics/sfcsub.F

Will need to coordinate with @junwang-noaa

@GeorgeGayno-NOAA GeorgeGayno-NOAA added the maintenance Basic upkeep label Mar 18, 2021
@GeorgeGayno-NOAA GeorgeGayno-NOAA self-assigned this Mar 18, 2021
@junwang-noaa
Copy link
Contributor

@climbfuji @DusanJovic-NOAA do you know if there is a way to point to a single file from other repository?

@climbfuji
Copy link
Contributor

I found this on the web, haven't tried it:

First clone the repo with the -n option, which suppresses the default checkout of all files, and the --depth 1 option, which means it only gets the most recent revision of each file

git clone -n git://path/to/the_repo.git --depth 1
Then check out just the file you want like so:

cd the_repo
git checkout HEAD name_of_file

@edwardhartnett
Copy link
Collaborator

This will not work in general, because it will break the github distribution of tarballs. In other words, git will be the only way the software can be built, which is unusual and unfair.

Is there some library we can put this code in? Some library that both of these applications can link to?

@climbfuji
Copy link
Contributor

This will not work in general, because it will break the github distribution of tarballs. In other words, git will be the only way the software can be built, which is unusual and unfair.

Is there some library we can put this code in? Some library that both of these applications can link to?

It's a single file ... of course we could put it into a separate repository, possibly together with gcycle.F90, if we think this isn't too much overhead. The benefit of using the same repository is big, but it will create a testing dependency between ccpp-physics and UFS_UTILS. Not sure if this is what we want, especially if UFS_UTILS requires changes that then trigger a whole range of updates in all models using ccpp-physics.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

This will not work in general, because it will break the github distribution of tarballs. In other words, git will be the only way the software can be built, which is unusual and unfair.
Is there some library we can put this code in? Some library that both of these applications can link to?

It's a single file ... of course we could put it into a separate repository, possibly together with gcycle.F90, if we think this isn't too much overhead. The benefit of using the same repository is big, but it will create a testing dependency between ccpp-physics and UFS_UTILS. Not sure if this is what we want, especially if UFS_UTILS requires changes that then trigger a whole range of updates in all models using ccpp-physics.

I doubt there will be many changes to sfcsub.F that are initiated from UFS_UTILS. The global_cycle program is simply a stand-alone wrapper that runs sfcsub.F. I am more worried about keeping up with changes to sfcsub.F in CCPP.

@climbfuji
Copy link
Contributor

This will not work in general, because it will break the github distribution of tarballs. In other words, git will be the only way the software can be built, which is unusual and unfair.
Is there some library we can put this code in? Some library that both of these applications can link to?

It's a single file ... of course we could put it into a separate repository, possibly together with gcycle.F90, if we think this isn't too much overhead. The benefit of using the same repository is big, but it will create a testing dependency between ccpp-physics and UFS_UTILS. Not sure if this is what we want, especially if UFS_UTILS requires changes that then trigger a whole range of updates in all models using ccpp-physics.

I doubt there will be many changes to sfcsub.F that are initiated from UFS_UTILS. The global_cycle program is simply a stand-alone wrapper that runs sfcsub.F. I am more worried about keeping up with changes to sfcsub.F in CCPP.

This file hasn't seen many changes in CCPP either ... maybe twice a year or so in the last few years?

GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 25, 2021
Add submodule for ccpp repo. Just tinkering right now.

Fixes ufs-community#424
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 25, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 25, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 25, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 1, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 14, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 21, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 21, 2021
@GeorgeGayno-NOAA
Copy link
Collaborator Author

@SMoorthi-emc I compiled a version of global_cycle that pointed to the CCPP version of sfcsub.F. I then ran it on a canned C768 case. The sea ice records were different between my control and test. I see you made ice-related changes to the CCPP version about a year ago. Can you look at my case and let me know if the results make sense. There are differences in areal ice coverage at the low end of concentration (15%).

@SMoorthi-emc
Copy link
Contributor

Hi George,
The version in CCPP allows for different minimum ice concentration for sea ice and lake ice. See the namelist parameters "min_seaice" and min_lakeice" I also probably made some other changes that I don't remember anymore. If you set "min_lakeice=min_seaice=0.15" then it should just like the original version. Actually, I have an updated version of sfcsub.F as the current one in CCPP/master has some issues that I fixed. However the repositories are changing so rapidly that I don't know when I will get a chance to push my changes. If you can point me to the global_cycle version on Hera, I can try to see what the differences are.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

@SMoorthi-emc Thanks for your help.

I placed the version of global_cycle with the ccpp version of sfcsub.F on Hera:
/scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.ccpp/UFS_UTILS.sfcsub
The global_cycle code is under ./sorc/global_cycle.fd. The sfcsub.F routine is under ./ccpp-physics/physics.

The current version of global_cycle with its own copy of sfcsub.F is here:
/scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.control/UFS_UTILS

I ran a canned C768 test case using both versions. Only tiles 3 and 6 are different. And the difference is related to the minimum ice value. The control files are here: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.control/output. The ccpp files are here: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.ccpp/output. Difference files for tile 3 and 6 are also included as is the log file (consistency.log).

You will note differences in the ice coverage at the concentration minimum. (The ccpp version has less ice).

Is there an ice check that was changed from (> min_seaice) or (>= min_seaice)?

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Apr 22, 2021 via email

@GeorgeGayno-NOAA
Copy link
Collaborator Author

Hi George, I am not sure how to visualize your output .nc files. Grads does not open them and dbrowser is very slow. I see that the global_cycle version of sfcsub has been significantly changed from the model version over time. I am trying to look at the code differences. Moorthi

On Thu, Apr 22, 2021 at 11:40 AM GeorgeGayno-NOAA @.> wrote: @SMoorthi-emc https://github.com/SMoorthi-emc Thanks for your help. I placed the version of global_cycle with the ccpp version of sfcsub.F on Hera: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.ccpp/UFS_UTILS.sfcsub The global_cycle code is under ./sorc/global_cycle.fd. The sfcsub.F routine is under ./ccpp-physics/physics. The current version of global_cycle with its own copy of sfcsub.F is here: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.control/UFS_UTILS I ran a canned C768 test case using both versions. Only tiles 3 and 6 are different. And the difference is related to the minimum ice value. The control files are here: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.control/output. The ccpp files are here: /scratch2/NCEPDEV/stmp1/George.Gayno/sfcsub.test/global-cycle.ccpp/output. Difference files for tile 3 and 6 are also included as is the log file (consistency.log). You will note differences in the ice coverage at the concentration minimum. (The ccpp version has less ice). Is there an ice check that was changed from (> min_seaice) or (>= min_seaice)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#424 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALLVRYVC74AUBKTAATDZDT3TKA7NJANCNFSM4ZNGKJIA .
-- Dr. Shrinivas Moorthi Research Meteorologist Modeling and Data Assimilation Branch Environmental Modeling Center / National Centers for Environmental Prediction 5830 University Research Court - (W/NP23), College Park MD 20740 USA Tel: (301)683-3718 e-mail: @.
Phone: (301) 683-3718 Fax: (301) 683-3718

I can try to create some Grads control files.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

@SMoorthi-emc I left a Grads control file - tile3.ctl - in each directory. It does not plot a map background. Just the 2-d data. So do 'set mproj off'.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Apr 22, 2021 via email

@SMoorthi-emc
Copy link
Contributor

Hi George,
The differences are probably becasue of the following differences in the code:
(ccpp)
" if (slifcs(i) >= 1.99_kind_io8) then
if (sicfcs(i) > crit) then
tem1 = 1.0_kind_io8 / sicfcs(i)
tsffcs(i) = (sicanl(i)*tsffcs(i)
& + (sicfcs(i)-sicanl(i))tgice) * tem1
sitfcs(i) = (tsffcs(i)-tgice
(1.0-sicfcs(i))) * tem1
sicfcs(i) = sicanl(i)
else
tsffcs(i) = tsfanl(i)
! tsffcs(i) = tgice
! sihfcs(i) = sihnew
sihfcs(i) = 0.0_kind_io8
sicfcs(i) = 0.0_kind_io8
slifcs(i) = 0.0_kind_io8
endif
endif"

and global_cycle:
" if (slifcs(i).ge.2.) then
if (sicfcs(i).gt.crit) then
tsffcs(i) = (sicanl(i)*tsffcs(i)
& + (sicfcs(i)-sicanl(i))tgice)/sicfcs(i)
sitfcs(i) = (tsffcs(i)-tgice
(1.0-sicfcs(i))) / sicfcs(i)
else
tsffcs(i) = tsfanl(i)
! tsffcs(i) = tgice
sihfcs(i) = sihnew
endif
endif
sicfcs(i) = sicanl(i)"

I think my changes in CCPP may not be completely right. I need to think about it a little more.
Thanks
Moorthi

@SMoorthi-emc
Copy link
Contributor

Actually, it gets even more messier with fractional grid and flake model for lakes.

@SMoorthi-emc
Copy link
Contributor

You may want to consider postponing this unification as the code is even originally incorrect when ice concentration is < 1.0
(this is the first factional grid that was ignored in the original sfcsub). Depending on just slmsk is incorrect.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

You may want to consider postponing this unification as the code is even originally incorrect when ice concentration is < 1.0
(this is the first factional grid that was ignored in the original sfcsub). Depending on just slmsk is incorrect.

Ok. Thanks for taking a look. I will hold off. Let me know if you want help with testing/debugging.

GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 23, 2021
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 26, 2021
@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Mar 19, 2022 via email

@GeorgeGayno-NOAA
Copy link
Collaborator Author

Ran the consistency tests using bdcdb38 on WCOSS-Dell. The C768 test (that mimics GFS OPS) failed in regions with ice (tiles 1, 2, 4 and 5 were bit identical to the baseline). Differences were small. But are they significant?

Tile 3

Variable Group Count       Sum   AbsSum         Min          Max      Range        Mean      StdDev
tsea     /      9340   113.615  113.634 -0.00945343     0.248625   0.258078   0.0121643   0.0527147
hice     /       477 -0.578968 0.578968 -0.00294555 -8.92851e-05 0.00285627 -0.00121377 0.000515948
tisfc    /     10527   757.433  757.559  -0.0630229       1.6575    1.72052   0.0719514    0.332016

Tile 6

Variable Group Count        Sum     AbsSum          Min         Max       Range         Mean      StdDev
tsea     /     36232   -4.22314    4.75771     -1.55269    0.142046     1.69473 -0.000116558   0.0112281
hice     /         9 0.00153423 0.00182039 -7.16645e-05 0.000585212 0.000656876   0.00017047 0.000203502
tisfc    /     39411   -28.1543    31.7181     -10.3512    0.946975     11.2982 -0.000714376   0.0717717

@SMoorthi-emc Are these differences expected? I can create difference plots if that would help.

@SMoorthi-emc
Copy link
Contributor

George,
Can you explain a bit about this test? Is it on full grid? Are you setting min_ice=0.15 for both ocean and lakes?
Also, can you point me to your source on dell for both opr and new so that I can actually look at the differences in the code?
Thanks
Moorthi

@GeorgeGayno-NOAA
Copy link
Collaborator Author

George, Can you explain a bit about this test? Is it on full grid? Are you setting min_ice=0.15 for both ocean and lakes? Also, can you point me to your source on dell for both opr and new so that I can actually look at the differences in the code? Thanks Moorthi

To see the source code, you can clone my fork:

git clone https://github.com/GeorgeGayno-NOAA/UFS_UTILS.git

Then checkout the feature/sfcsub_ccpp branch.

Code differences are here: https://github.com/ufs-community/UFS_UTILS/pull/636/files

@GeorgeGayno-NOAA
Copy link
Collaborator Author

George, Can you explain a bit about this test? Is it on full grid? Are you setting min_ice=0.15 for both ocean and lakes? Also, can you point me to your source on dell for both opr and new so that I can actually look at the differences in the code? Thanks Moorthi

The test is a global C768 grid. The data directory is on Venus: /gpfs/dell1/stmp/George.Gayno/reg-tests.sfcsub/global-cycle/test1.

The fort.35/36/37 namelists were saved. Sea ice is being applied - FNACNA is pointing to this ice file: /gpfs/dell2/emc/modeling/noscrub/George.Gayno/ufs_utils.git/reg_tests/global_cycle/input_data/gdas.t00z.seaice.5min.blend.grb.

I believe I am using a min ice of 15%.

GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 23, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 23, 2022
@SMoorthi-emc
Copy link
Contributor

I looked at the differences in the code. There are some minor differences in how ice is handled. So I assume there will be some differences where sea-ice/lake_ice are involved. Since the model is working fine, I have to assume the version in the model also is fine. It would be instructive if the differences and the total fields are plotted. I am neither that familiar with plotting with netcdf nor with plotting on tiles.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

I looked at the differences in the code. There are some minor differences in how ice is handled. So I assume there will be some differences where sea-ice/lake_ice are involved. Since the model is working fine, I have to assume the version in the model also is fine. It would be instructive if the differences and the total fields are plotted. I am neither that familiar with plotting with netcdf nor with plotting on tiles.

I will make some plots.

@GeorgeGayno-NOAA
Copy link
Collaborator Author

Ran the consistency tests using bdcdb38 on WCOSS-Dell. The C768 test (that mimics GFS OPS) failed in regions with ice (tiles 1, 2, 4 and 5 were bit identical to the baseline). Differences were small. But are they significant?

Tile 3

Variable Group Count       Sum   AbsSum         Min          Max      Range        Mean      StdDev
tsea     /      9340   113.615  113.634 -0.00945343     0.248625   0.258078   0.0121643   0.0527147
hice     /       477 -0.578968 0.578968 -0.00294555 -8.92851e-05 0.00285627 -0.00121377 0.000515948
tisfc    /     10527   757.433  757.559  -0.0630229       1.6575    1.72052   0.0719514    0.332016

Tile 6

Variable Group Count        Sum     AbsSum          Min         Max       Range         Mean      StdDev
tsea     /     36232   -4.22314    4.75771     -1.55269    0.142046     1.69473 -0.000116558   0.0112281
hice     /         9 0.00153423 0.00182039 -7.16645e-05 0.000585212 0.000656876   0.00017047 0.000203502
tisfc    /     39411   -28.1543    31.7181     -10.3512    0.946975     11.2982 -0.000714376   0.0717717

@SMoorthi-emc Are these differences expected? I can create difference plots if that would help.

Some printout from the C768 consistency test (branch and develop). These TILE 6 points have the largest ice temperature differences (TISFC). Note that all these points had ice concentrations (SICFCS) at the minimum - 15% - before and after the call to sfccycle. At ice points, skin temperature (TSEA) is a linear combination of the ice temperature and the open water temperature (271.2) based on the ice concentration. If ice concentration does not change, I would not expect TSEA or TISFC to change. But that is what is happening with 'develop' (TSEA and TISFC become equal). The branch, which uses the CCPP version of sfcsub.F, does not change TSEA and TISFC. That makes sense to me. Does the CCPP version have a bug fix?

'develop'

These points at tile 6

                     SLIFCS             SICFCS            TSEA            TISFC
(258,201) before  2.00000000000000  0.150000000000000  269.373310985963  259.022073239755
(216,238) before  2.00000000000000  0.150000000000000  270.051486406558  263.543242710385
(324,633) before  2.00000000000000  0.150000000000000  271.347340912173  272.182272747823

                    SLIFCS             SICFCS            TSEA            TISFC 
(258,201) after   2.00000000000000  0.150000000000000  269.373310985963  269.373310985963
(216,238) after   2.00000000000000  0.150000000000000  270.051486406558  270.051486406558
(324,633) after   2.00000000000000  0.150000000000000  271.347340912173  271.347340912173

branch at db54641

                     SLIFCS             SICFCS            TSEA            TISFC
(258,201) before  2.00000000000000  0.150000000000000  269.373310985963  259.022073239755
(216,238) before  2.00000000000000  0.150000000000000  270.051486406558  263.543242710385
(324,633) before  2.00000000000000  0.150000000000000  271.347340912173  272.182272747823

                     SLIFCS             SICFCS            TSEA           TISFC
(258,201) after   2.00000000000000  0.150000000000000  269.373310985963  259.022073239755
(216,238) after   2.00000000000000  0.150000000000000  270.051486406558  263.543242710385
(324,633) after   2.00000000000000  0.150000000000000  271.347340912173  272.182272747823

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Mar 25, 2022 via email

GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Mar 28, 2022
@GeorgeGayno-NOAA
Copy link
Collaborator Author

Some graphics from the tests described here: #424 (comment)

Note how the biggest differences in TSEA and TISFC only occur at (a very few) points where the sea ice is at the 15% threshold. As shown by the printout of several points in the previous comment. The CCPP version of sfcsub.F seems to have corrected a problem at the 15% ice threshold.

tile6 tisfc

tile6 tisfc mask

tile6 tsea

tile6 tsea mask

tile3 tisfc

tile3 tisfc mask

tile3 tsea

tile3 tsea mask

@GeorgeGayno-NOAA
Copy link
Collaborator Author

I did make several updates in the code related to the ice. It happened as I was trying to take into account the fractional grid. I do not specifically recall the details as this was done some time ago. Moorthi

Maybe you fixed something, or the 'develop' version of global_cycle was not calling sfcsub.F properly. Anyway, I think the branch which uses the CCPP version is correct. Here is another look at one point from tile 6 before and after the call to sfcsub.F.

SLIFCS SICFCS TSEA TISFC
(258,201) before 2.00000000000000 0.150000000000000 269.373310985963 259.022073239755
(258,201) after (develop) 2.00000000000000 0.150000000000000 269.373310985963 269.373310985963
(258,201) after (CCPP) 2.00000000000000 0.150000000000000 269.373310985963 259.022073239755

If the ice fraction does not change, I don't see why TISFC or TSEA should change. And since TSEA is a blend of the ice skin temperature and the open water temperature (271.2), TISFC and TSEA should not be equal at ice points.

GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 25, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 25, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue Apr 25, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 4, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 4, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 6, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 13, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 20, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 24, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 25, 2022
GeorgeGayno-NOAA added a commit to GeorgeGayno-NOAA/UFS_UTILS that referenced this issue May 25, 2022
GeorgeGayno-NOAA added a commit that referenced this issue May 25, 2022
Ensure the global_cycle program is using the same sfcsub.F
module as the forecast model.

Fixes #424.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Basic upkeep
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants