Skip to content

Conversation

@weiyuan-jiang
Copy link
Contributor

@weiyuan-jiang weiyuan-jiang commented Mar 14, 2025

This PR addresses an inconsistency in the land boundary conditions for the coupled atmosphere-ocean model. Specifically, the land properties in the coupled model BCs were inconsistent with those in the matching AGCM BCs owing to a change of the raster file when generating the MOM tripolar ocean BCs. The raster file modifications are necessary for MOM to work (essentially, some land raster grid cells need to be converted to ocean). This PR maintains the modified land/ocean raster file and the resulting modified lat/lon coordinates of the tiles, but the land model parameters of the surviving land tiles are obtained directly from an existing and matching set of BCs for the AGCM (which were constructed without the raster file modifications). In this way, restart files from the AGCM can be used without additional spinup to intitialize the coupled atm-ocean model.

PR is trivially 0-diff for AGCM and LDAS simulations because it only touches make_bcs.
PR successfully 0-diff tested for make_bcs (#1075 (comment)).

cc: @amolod

@weiyuan-jiang weiyuan-jiang added the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label Mar 14, 2025
@weiyuan-jiang
Copy link
Contributor Author

Should we move the line

bin/mk_runofftbl.x -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv}

down to line 63 with the new adjusted tile file?

Does the new tile file have a conflict with the corresponding raster file? Or does the conflict matter ? @gmao-rreichle @zhaobin74

@zhaobin74
Copy link
Contributor

Should we move the line

bin/mk_runofftbl.x -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv}

down to line 63 with the new adjusted tile file?
Does the new tile file have a conflict with the corresponding raster file? Or does the conflict matter ? @gmao-rreichle @zhaobin74

I think runoff table should be made based on the tile file with MOM modifications.

@weiyuan-jiang
Copy link
Contributor Author

Should we move the line

bin/mk_runofftbl.x -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv}

down to line 63 with the new adjusted tile file?
Does the new tile file have a conflict with the corresponding raster file? Or does the conflict matter ? @gmao-rreichle @zhaobin74

I think runoff table should be made based on the tile file with MOM modifications.

Then I expect some inconsistency somewhere. The tiles' lat and lon may be difference between tile file and those used in ".TRN"

@zhaobin74
Copy link
Contributor

zhaobin74 commented Mar 18, 2025

Should we move the line

bin/mk_runofftbl.x -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv}

down to line 63 with the new adjusted tile file?
Does the new tile file have a conflict with the corresponding raster file? Or does the conflict matter ? @gmao-rreichle @zhaobin74

I think runoff table should be made based on the tile file with MOM modifications.

Then I expect some inconsistency somewhere. The tiles' lat and lon may be difference between tile file and those used in ".TRN"

@weiyuan-jiang, I can see a consistency problem between Outlet_latlon.43200x21600 (an input to bin/mk_runofftbl.x) and the raster/tile file the previous step generated. Outlet_latlon.43200x21600 is created without knowing MOM's land sea mask (there is an extra step to move runoff around).

@atrayano, could you help here? Thanks.

@zhaobin74
Copy link
Contributor

@weiyuan-jiang, I have tested the new MOM6 tile land bcs using V12 rc12 at C90. It ran for a couple of days without problems. I also checked the tile file and it is identical to the old one. Since it has been confirmed that land properties are also identical, I think it is ok to proceed. Thanks for the efforts.

@biljanaorescanin
Copy link
Contributor

This PR is trivially zero diff for GCM since all changes are just in Boundary Conditions Package creation.

  1. Regular grids BCs are not affected they are zero diff.
  2. MOM grids are now not changing land tiles anymore but have exactly the same land properties on them which was desired outcome.

@sdrabenh this is now ready for you.

I'm not sure why some CI are failing but in my sand box all looks good during testing. These errors do indicate MOM grid but are in separate repo... any ideas? @mathomp4 @weiyuan-jiang @zhaobin74 @gmao-rreichle

[ 58%] Building Fortran object src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/MOM6_GEOSPlug/mom6_cmake/CMakeFiles/mom6.dir/__/@mom6/src/parameterizations/vertical/MOM_CVMix_KPP.F90.o /__w/GEOSgcm_GridComp/GEOSgcm_GridComp/src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/MOM6_GEOSPlug/@mom6/src/parameterizations/vertical/MOM_CVMix_KPP.F90(1250): error #8284: If the actual argument is scalar, the dummy argument shall be scalar unless the actual argument is of type character or is an element of an array that is not assumed shape, pointer, or polymorphic. [BFSFC] BulkRi_1d = CVmix_kpp_compute_bulk_Richardson( & ------------------^ /__w/GEOSgcm_GridComp/GEOSgcm_GridComp/src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/MOM6_GEOSPlug/@mom6/src/parameterizations/vertical/MOM_CVMix_KPP.F90(1263): error #6285: There is no matching specific subroutine for this generic subroutine call. [CVMIX_KPP_COMPUTE_OBL_DEPTH] call CVMix_kpp_compute_OBL_depth( & -----------^ /__w/GEOSgcm_GridComp/GEOSgcm_GridComp/src/Components/@GEOSgcm_GridComp/GEOSogcm_GridComp/@GEOS_OceanGridComp/MOM6_GEOSPlug/@mom6/src/parameterizations/vertical/MOM_CVMix_KPP.F90(1290): error #8284: If the actual argument is scalar, the dummy argument shall be scalar unless the actual argument is of type character or is an element of an array that is not assumed shape, pointer, or polymorphic. [BFSFC] Vt2_1d(:) = CVmix_kpp_compute_unresolved_shear( & --------------------^

@biljanaorescanin biljanaorescanin removed the Contingent - DNA These changes are contingent on other PRs (DNA=do not approve) label May 2, 2025
@biljanaorescanin biljanaorescanin marked this pull request as ready for review May 2, 2025 14:49
@biljanaorescanin biljanaorescanin requested review from a team as code owners May 2, 2025 14:49
@mathomp4
Copy link
Member

mathomp4 commented May 2, 2025

Ah I see why CI is failing. One moment.

@mathomp4
Copy link
Member

mathomp4 commented May 2, 2025

I've pushed a change to develop and updated this branch. CI should work now.

@gmao-rreichle gmao-rreichle added 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) 0 diff The changes in this pull request have verified to be zero-diff with the target branch. labels May 2, 2025
@sdrabenh sdrabenh merged commit 8185f71 into develop May 2, 2025
15 of 16 checks passed
@sdrabenh sdrabenh deleted the feature/wjiang/adjust_mom_tile branch May 2, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) 0 diff The changes in this pull request have verified to be zero-diff with the target branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants