Routing Outlets in Boundary Conditions

[biljanaorescanin]

Boundary conditions produce river routing file with coupled grid runs. ( *.TRN)
Current problem: This file right from the package on develop now would have to be manually adjusted if we wanted to use it with GCM coupled runs. All routing outlets should end up in oceans but that is not the case with develop version.

This PR brings preprocessing tools used to create new “Outlet” file used during boundary conditions package runs to produce more correct river outlet information for coupled runs. Everyting will now end up in the ocean.

Once we conform this information we will add code changes to point and use “v2” of this Outlet file.

This PR is zero diff for AGCM but not zero diff for boundary conditions package.

This PR is related to issue: #681

@gmao-rreichle @sanAkel

[zyj8881357]

Boundary conditions produce river routing file with coupled grid runs. ( *.TRN) Current problem: This file right from the package on develop now would have to be manually adjusted if we wanted to use it with GCM coupled runs. All routing outlets should end up in oceans but that is not the case with develop version.

This PR brings preprocessing tools used to create new “Outlet” file used during boundary conditions package runs to produce more correct river outlet information for coupled runs. Everyting will now end up in the ocean.

Once we conform this information we will add code changes to point and use “v2” of this Outlet file.

This PR is zero diff for AGCM but not zero diff for boundary conditions package.

This PR is related to issue: #681

@gmao-rreichle @sanAkel

This version is only for 5 degree ocean grid.

[gmao-rreichle]

@zyj8881357, @biljanaorescanin, thanks for putting this together. Here are a couple of initial thoughts after a very quick glance:

1. We'll need a README file or some other form of documentation. With a couple of exceptions, all f90 files are programs, and the only documentation is in the file names, with no comments or instructions at all inside the files. This isn't sufficient for someone else to understand how to use the package to generate the outlet files.
2. Re. your comment that this only works for the 5-deg ocean grid: I might not understand this correctly, but if the package is specific to a given ocean model grid and resolution, and presumably a very coarse resolution that is for testing purposes only, there aren't yet outlet files that would work with S2Sv3 or what's in development. Is this correct?
3. Re. the previous item: My understanding is that the package first obtains the definition of the catchments on the raster grid and then determines the connection from each catchment to the ocean. This step should be independent of the ocean model resolution. The second step would then be to determine lat/lon values for the outlets that work with a particular ocean model grid and resolution. I suspect the existing f90 programs and ncl tools can be easily assigned to one of these two steps, and this separation is just a matter of documentation. If the package doesn't separate easily into sub-packages for the (ocean) resolution-independent step and the resolution-dependent steps, it should be modified so the package becomes modular in this way.
4. Do we really want to produce (ocean) resolution-specific outlet files here, or would it be easier to stick with the current approach and have this package only produce a single outlet file with outlet lat/lon values that match the catchment raster file. The moving of the outlet lat/lon points into suitable ocean grid points (ie, the resolution-dependent step) could then be coded within the make_bcs package, where the *.TRN file is produced. The latter approach has the advantage of having the ocean model resolution information at hand as part of make_bcs processing. If the ocean model resolution is addressed within the present preproc package, it'd be harder to maintain consistency between the preproc package and make_bcs.

Not sure if this really makes sense, but it's a start for discussing how to move the PR forward.

[zyj8881357]

@zyj8881357, @biljanaorescanin, thanks for putting this together. Here are a couple of initial thoughts after a very quick glance:

1. We'll need a README file or some other form of documentation.  With a couple of exceptions, all f90 files are programs, and the only documentation is in the file names, with no comments or instructions at all inside the files.  This isn't sufficient for someone else to understand how to use the package to generate the outlet files.

2. Re. your comment that this only works for the 5-deg ocean grid:  I might not understand this correctly, but if the package is specific to a given ocean model grid and resolution, and presumably a very coarse resolution that is for testing purposes only, there aren't yet outlet files that would work with S2Sv3 or what's in development.  Is this correct?

3. Re. the previous item: My understanding is that the package first obtains the definition of the catchments on the raster grid and then determines the connection from each catchment to the ocean.  This step should be independent of the ocean model resolution.  The second step would then be to determine lat/lon values for the outlets that work with a particular ocean model grid and resolution.  I suspect the existing f90 programs and ncl tools can be easily assigned to one of these two steps, and this separation is just a matter of documentation.  If the package doesn't separate easily into sub-packages for the (ocean) resolution-independent step and the resolution-dependent steps, it should be modified so the package becomes modular in this way.

4. Do we really want to produce (ocean) resolution-specific outlet files here, or would it be easier to stick with the current approach and have this package only produce a single outlet file with outlet lat/lon values that match the catchment raster file.  The moving of the outlet lat/lon points into suitable ocean grid points (ie, the resolution-dependent step) could then be coded within the make_bcs package, where the *.TRN file is produced.  The latter approach has the advantage of having the ocean model resolution information at hand as part of make_bcs processing.  If the ocean model resolution is addressed within the present preproc package, it'd be harder to maintain consistency between the preproc package and make_bcs.

Not sure if this really makes sense, but it's a start for discussing how to move the PR forward.

@gmao-rreichle @biljanaorescanin @sanAkel
All the comments from Rolf have been addressed.

1. A readme.txt has been added.
2. Now the pre-processing package (GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/preproc/routing) only contains code to create the outlet locations in land. The moving from land to ocean is done by the mk_runofftbl.f90 in the mk_bcs package. The new mk_runofftbl.f90 (GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/makebcs/mk_runofftbl.f90) is also included in this branch.
3. The ocean resolution is not fixed or hard coded in the script. The new mk_runofftbl.f90 reads the ocean resolution that inputs to mk_bcs. It should work for any ocean resolution.

But the outlet locations have not been checked by Biljana or Santha. It should be tested before merging to the develop branch.

[gmao-rreichle]

@zyj8881357, @biljanaorescanin: The software infrastructure of this package needs fixing up as follows:

1. Use Cmake build instead of "build" bash script.
2. Can we avoid adding the custom netcdf modules (rwncMod.F90, ncdioMod.F90) and use whatever is available in the GEOS environment?
3. The "run.sh" script for Discover should be converted to python.

The above list is just a start, and I may be wrong on some or all of it. I'll defer to the SI team on what needs changing and how.
The SI team should be able to assist with these fixes.

@rdkoster @tclune @weiyuan-jiang

[sanAkel]

@zyj8881357, @biljanaorescanin: The software infrastructure of this package needs fixing up as follows:

1. Use Cmake build instead of "build" bash script.

2. Can we avoid adding the custom netcdf modules (rwncMod.F90, ncdioMod.F90) and use whatever is available in the GEOS environment?

3. The "run.sh" script for Discover should be converted to python.

The above list is just a start, and I may be wrong on some or all of it. I'll defer to the SI team on what needs changing and how.

The SI team should be able to assist with these fixes.

@rdkoster @tclune @weiyuan-jiang

If you guys are stuck, let me know. Above seem straightforward to me.

[zyj8881357]

@biljanaorescanin @gmao-rreichle I have added the version information in mk_runofftble.f90

I slightly changed the version links as follows:

bcs version --> Outlet lat/lon file version
v12 --> (new) v2 (produced with Yujin's pre-processing routines)
v11 --> (old) v1 (produced with Randy's old file)
otherwise --> n/a (produced with Randy's old file, but do not move outlet locations to ocean)

[biljanaorescanin]

I've added v12; MOM6 created with v2 Outlet file to centralized location.
As expected only trn/TRN files differ to previous files.
For now I will host links until Santha confirms all works for him.
@sanAkel this should work for you directly from gcm_setup since we added v12 to it.
/discover/nobackup/projects/gmao/bcs_shared/fvInput/ExtData/esm/tiles/v12/geometry

Should I run MOM5 options as well? Do we have users who need v12 MOM5?

[gmao-rreichle]

@zyj8881357:

otherwise --> n/a (produced with Randy's old file, but do not move outlet locations to ocean)

We should not produce routing files for any bcs other than v11 and v12. Some of the existing older bcs have custom-made routing files (manual intervention) that we cannot and do not want to replicate in make_bcs. Routing files generated with make_bcs will be in conflict with the existing custom-made routing files. That is, they should not be generated by make_bcs in the first place.

[zyj8881357]

Hi Rolf, I have realized the code does not work and know the solution same as your suggestion. I push the code just for testing purposes. Please do the check it after I make a comment in the github. Thanks so much!

…

On Tue, Jul 2, 2024 at 5:00 PM Rolf Reichle ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In GEOSagcm_GridComp/GEOSphysics_GridComp/GEOSsurface_GridComp/Utils/Raster/makebcs/make_bcs_cube.py <#827 (comment)> : > setenv OMP_NUM_THREADS 1 - if ({SKIPLAND} != True) bin/mkCatchParam.x -x {NX} -y {NY} -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv} + if ({SKIPLAND} != True) then + bin/mkCatchParam.x -x {NX} -y {NY} -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter -v {lbcsv} + bin/mk_runofftbl.x -g CF{NC}x6C{SGNAME}_{OCEAN_VERSION}{DATENAME}{IMO}x{POLENAME}{JMO}-Pfafstetter @zyj8881357 <https://github.com/zyj8881357> : Thanks for moving the runoff config info into the land bcs version case statement within rmTinyCatchParaMod.F90. Have you tried running the code? I wonder if it can work. I would have expected that we need to pass {lbcsv} into mk_runofftbl.x via a command line argument and then use call init_bcs_config (trim(LBCSV)) ! get bcs details from version string from within the mk_runofftbl program. I might be wrong, but at a quick glance I'm not sure the version info makes it to where it's needed. — Reply to this email directly, view it on GitHub <#827 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHZKGWQCPVTAODOQDV5FSKLZKMIITAVCNFSM6AAAAAA5H2QGWCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNJUHA3DMNBSGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[zyj8881357]

@gmao-rreichle I have fixed the problem. The program should work now.

[gmao-rreichle]

@zyj8881357 , @biljanaorescanin :
The code now looks like it should work. The documentation needed cleaning up. Please double-check my latest commit.

@biljanaorescanin, please generate select v12 bcs to see if the results match those in the bcs_shared dir. If that works out, I think the PR is ready for Scott.

@biljanaorescanin: The [..]/bcs_shared/make_bcs_inputs/land/route/ directory may still need some cleanup. I don't understand why the there are "v1" files in the [..]/bcs_shared/make_bcs_inputs/land/route/ directory and then links to these files in the ./v1 subdirectory. Ideally, the files should all be inside the v1 subdirectory only. In this case, I don't think backward compatibility with older make_bcs versions matters. The older make_bcs versions don't produce sensible routing TRN files anyway.

[zyj8881357]

.

@gmao-rreichle The edit looks fine in your latest commit. Thank you!

[biljanaorescanin]

@biljanaorescanin, please generate select v12 bcs to see if the results match those in the bcs_shared dir. If that works out, I think the PR is ready for Scott.

I've tested both v11 and v12 for MOM6 C90, C12 and C180 all are zero diff to what we had before.
PR is trivially zero diff for GCM, since all changes are boundary conditions and BVS preprocessing related no further testing is needed.

@sdrabenh this is ready for you . @zhaobin74 has tested for v12 MOM6 C90 coupled run.

[zhaobin74]

Testing of coupled MOM6 at C180 with V12 BCS(and new outlets) also works.