GEOS-ESM / GEOSgcm_GridComp

Repository containing the physics and IAU code for the GEOS Earth System Model
Apache License 2.0
9 stars 7 forks source link

make_bcs fix for bad HWSDv1.21 data in Argentina "peatland" #944

Closed gmao-rreichle closed 3 months ago

gmao-rreichle commented 4 months ago

In GEOS bcs versions NL5 and v11 (after the introduction of PEATMAP and PEATCLSM), a small region in Argentina (~38W,60W) is wrongly classified as peatland. The reason for the wrong classification are unrealistically high values of organic carbon content (OC) in the HWSDv1.21 input data for soil mapping unit (SMU) #12302.

makebcs uses preprocessed soil properties data that contain merged data from HWSD and STATSGO2 for the US. The SMU in question is spread across two of the SoilProperties*.nc files: H12V06 and H13V06. These two files were patched by replacing the clay, sand, and OC values for the 0-30cm and 30-100cm layers in the SMU in question with values obtained manually from v2 of HWSD.

The present PR enables makebcs to use these revised SoilProperties*.nc data to generate v12 bcs.

PR only touches make_bcs and is thus trivially 0-diff for GCM and LDAS simulations.

PR was successfully 0-diff tested for generation of existing bcs (NL3 for EASE and cube-sphere tile spaces) on 31 May 2024 by @biljanaorescanin

v12 bcs were verified by @gmao-rreichle through direct comparison of bcs files and the SMAP Nature Run v11.3 simulation.

gmao-rreichle commented 4 months ago

@mathomp4 : As a side show in the present PR, we're trying to get "make_bcs.py" to work on SLES15/Milan. My initial approach was to simply remove the #SBATCH constraint (https://github.com/GEOS-ESM/GEOSgcm_GridComp/pull/944/commits/fb06070619e4bb8061df99026102d247eca180fa). But this only works on SLES12. On SLES15, the job that is submitted by make_bcs.py either hangs or crashes unexplicably with library errors. We apparently can get this to work by adding an explicit (non)-constraint #SBATCH --constraint=sky|cas|mil (https://github.com/GEOS-ESM/GEOSgcm_GridComp/pull/944/commits/8fd5fe311a4451859d49da1e2cfd1e5c22a766cf). For SLES15, using just #SBATCH --constraint=mil also works.
That is, it looks like the SLES15 job won't start or run without the explicit #SBATCH constraint directive, which doesn't really make sense to me. Maybe this is something for NCCS to sort out? cc: @biljanaorescanin @weiyuan-jiang

mathomp4 commented 4 months ago

@mathomp4 : As a side show in the present PR, we're trying to get "make_bcs.py" to work on SLES15/Milan. My initial approach was to simply remove the #SBATCH constraint (fb06070). But this only works on SLES12. On SLES15, the job that is submitted by make_bcs.py either hangs or crashes unexplicably with library errors. We apparently can get this to work by adding an explicit (non)-constraint #SBATCH --constraint=sky|cas|mil (8fd5fe3). For SLES15, using just #SBATCH --constraint=mil also works. That is, it looks like the SLES15 job won't start or run without the explicit #SBATCH constraint directive, which doesn't really make sense to me. Maybe this is something for NCCS to sort out? cc: @biljanaorescanin @weiyuan-jiang

You might ping NCCS, but now that I think about it, it's possible an sbatch with no constraint currently only targets sky|cas and not mil. I guess in gcm_setup/gcm_run we always specify the constraint, though that's because we ask the user what they want to run on (makes the math for oserver, etc. easier)

weiyuan-jiang commented 4 months ago

SBATCH --constraint=sky|cas|mil

I don't think this is correct. The executables built on SLES12 cannot be run on SLES15.

mathomp4 commented 4 months ago

SBATCH --constraint=sky|cas|mil

I don't think this is correct. The executables built on SLES12 cannot be run on SLES15.

Oh yeah. That is true. That's the reason I have the BUILT_ON_SLES15 variable in CMake. I use it in gcm_setup and @weiyuan-jiang used it in the LDAS setup as well I think.

This is probably the "right" thing to do.

gmao-rreichle commented 4 months ago

Thanks, @weiyuan-jiang and @mathomp4 for the commit and advice, much appreciated. @biljanaorescanin : When you get a chance, please run just one set of bcs on both SLES12 and SLES 15 to make sure there isn't a syntax error somewhere.

weiyuan-jiang commented 4 months ago

SBATCH --constraint=sky|cas|mil

I don't think this is correct. The executables built on SLES12 cannot be run on SLES15.

Oh yeah. That is true. That's the reason I have the BUILT_ON_SLES15 variable in CMake. I use it in gcm_setup and @weiyuan-jiang used it in the LDAS setup as well I think.

This is probably the "right" thing to do.

@biljanaorescanin said the string is not replaced. Does that mean the trick does not apply to GEOSgcm_GridComp? @mathomp4

biljanaorescanin commented 4 months ago

After last fix everything works now with no errors. I've run few diff sets of bcs on both sles12 and sles15

mathomp4 commented 4 months ago

@biljanaorescanin said the string is not replaced. Does that mean the trick does not apply to GEOSgcm_GridComp? @mathomp4

Ah. You figured it out before I could reply. Yeah, it's configure_file :)

gmao-rreichle commented 4 months ago

@wmputman @sdrabenh @biljanaorescanin : This PR is now ready for merging:

  1. The PR only touches make_bcs and is trivially 0-diff for AGCM and LDAS simulations.
  2. The PR was successfully 0-diff tested for the generation of existing bcs.
  3. The new "v12" bcs have been verified in the offline (land-only) SMAP Nature Run system. In the offline system, the changes are strictly limited to the small region in Argentina where the erroneous soil parameters were replaced with more sensible values. I'm happy to share a few slides if you're interested. In the GCM, a change in one location will obviously impact the rest of the world, but it's fair to assume that the difference between the "v12" and "v11" bcs is not going to change the GCM's climate noticeably.

cc: @rdkoster