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

update LONG_NAME attributes to match M21C file specs; conservative regridding of (obs) PRECIP_FILE inputs #1032

Draft
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

gmao-rreichle
Copy link
Contributor

The primary objective of the present PR is to bring the file specs changes from M21C into "develop". The corresponding M21C PR is #637 (merged on 8 Dec 2022).

Additionally, the present PR changes the "regrid_method" for the input (obs) precipitation from PRECIP_FILE in GEOS_SurfaceGridComp.F90 to "conserve". This is of no consequence for FP-like systems that do not use observed precip inputs. The change was made with a view towards future reanalysis and S2S systems.

No changes were made to GEOS_CatchCNCLM45GridComp.F90 because this CatchCN model version is scientifically obsolete and has been disabled. (Proper cleanup of the repo remains pending.)

In M21C, most LONG_NAME attributes for "albedo" were changed to "reflectivity". For consistency, I applied this change in a few more places where it was not applied in M21C (3790723, e31cda3).

Specific comments:

GEOS_SurfaceGridComp.F90

  • It looks like OBIO is not in M21C, and the exports FSWBAND and FSWBANDNA have been replaced with DRBAND and DFBAND. This remains as in develop.
  • The AICE field in the M21C file specs ("slv" collection) uses the OFRACI export. I did my best to transfer this export from M21C to "develop" but I can't promise that I didn't mess up. This will require further inspection and testing.

GEOSsaltwater_GridComp/*.F90

  • There are several LONG_NAME attributes that include the term "skin" temperature. I don't know enough about the ocean and sea ice variables to tell whether the fields should be called "skin temps" or "surface temps". This requires further discussion and input from the @GEOS-ESM/ocean-team

GEOS_SeaiceInterfaceGridComp.F90

  • There are two LONG_NAME attributes of "downwelling"/"downward" longwave fluxes. Elsewhere in the code this was corrected to "absorbed" longwave flux. I don't know enough about sea ice to know if the change applies here as well (although I would be surprised if it did not). This requires further discussion and input from the @GEOS-ESM/ocean-team

@gmao-rreichle gmao-rreichle added documentation Improvements or additions to documentation 0 diff The changes in this pull request have verified to be zero-diff with the target branch. labels Nov 7, 2024
@gmao-rreichle
Copy link
Contributor Author

@biljanaorescanin, @sdrabenh: There's one more issue related to copying the M21C file specs changes into "develop". For M21C, we got NaNs in the all-surface runoff output because of a bug that was introduced with changes to the routing scheme (as currently used in the coupled model). The corresponding M21C PR is #969. However, the fix that we implemented for M21C is unlikely to work for "develop" because the M21C fix effectively disables the routing scheme (which is not needed for M21C). Note also that the relevant code block in "develop" is somewhat different from that of the "R21C" branch.

Having said that, it's not clear to me that the bug still exists in "develop". So first we need to run the GCM for a day or so, using the branch from this PR, and output the "flx" collection from M21C (which includes the all-surface runoff). If this produces reasonable RUNOFFTOT values, no further action is needed. Somewhat more likely, though, this will produce NaNs for RUNOFFTOT. If that is the case, we need to find a fix for the RUNOFFTOT output that does not adversely impact the routing scheme. To find such a fix, we may need to go back to Atanas, who introduced the problematic change in the R21C tag.

@biljanaorescanin
Copy link
Contributor

biljanaorescanin commented Nov 14, 2024

  1. All GEOSldas Regression tests passed.
  2. GCM 1-Day AMIP:
    Has as expected differences in metadata but for flx collection BUT variable RUNOFFTOT is NAN.
    34 : 2015-04-15 14:30:00 0 259920 259920 : nan : RUNOFFTOT

Here are Catch and Surf metadata diff summary:
Comparing NC4 catch_internal_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : CAPAC : 37 <> 28 : VALUES : "vegetation_interception_water_storage" : "interception_reservoir_capac"

Comparing NC4 surf_import_checkpoint using nccmp...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : VARIABLE : DFPARN : ATTRIBUTE : long_name : VALUES : "normalized_surface_downwelling_PAR_diffuse_flux" <> "normalized_surface_downwelling_par_diffuse_flux"
DIFFER : VARIABLE : DRPARN : ATTRIBUTE : long_name : VALUES : "normalized_surface_downwelling_PAR_beam_flux" <> "normalized_surface_downwelling_par_beam_flux"

For history collections:

Comparing tavg1_2d_flx_Nx-.20150414_2130z...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : VARIABLE : RUNOFFTOT : DOES NOT EXIST IN "/discover/nobackup/borescan/par/stock_gcm_amip_1032/scratch/stock_gcm_amip_1032.tavg1_2d_flx_Nx-.20150414_2130z.nc4"
DIFFER : LENGTHS : ATTRIBUTE : long_name : VARIABLE : FRSEAICE : 33 <> 28 : VALUES : "ice_covered_fraction_of_grid_cell" : "ice_covered_fraction_of_tile"
DIFFER : LENGTHS : ATTRIBUTE : standard_name : VARIABLE : FRSEAICE : 33 <> 28 : VALUES : "ice_covered_fraction_of_grid_cell" : "ice_covered_fraction_of_tile"

Comparing tavg1_2d_lfo_Nx-.20150415_0030z...
Failure!
Checking for data differences
Checking for metadata differences
DIFFER : VARIABLE : PRECRAINCU : DOES NOT EXIST IN "/discover/nobackup/borescan/par/stock_gcm_amip_1032/scratch/stock_gcm_amip_1032.tavg1_2d_lfo_Nx-.20150415_0030z.nc4"
DIFFER : VARIABLE : PRECRAINLS : DOES NOT EXIST IN "/discover/nobackup/borescan/par/stock_gcm_amip_1032/scratch/stock_gcm_amip_1032.tavg1_2d_lfo_Nx-.20150415_0030z.nc4"
DIFFER : VARIABLE : PRECCU : DOES NOT EXIST IN "/discover/nobackup/borescan/par/exp_gcm_amip_1032/scratch/exp_gcm_amip_1032.tavg1_2d_lfo_Nx-.20150415_0030z.nc4"
DIFFER : VARIABLE : PRECLS : DOES NOT EXIST IN "/discover/nobackup/borescan/par/exp_gcm_amip_1032/scratch/exp_gcm_amip_1032.tavg1_2d_lfo_Nx-.20150415_0030z.nc4"

LND collection differences are too long to list here since most variable name changes are there. But all changes are in metadata + we now with branch have one more extra variable
13 : 2015-04-15 19:30:00 0 259920 184367 : 0.0000 0.033201 1.0889 : INTRWATR

@gmao-rreichle
Copy link
Contributor Author

@sdrabenh: This PR is now ready for merging as far as the land group is concerned. We still need review and approval from @GEOS-ESM/landice-team, @GEOS-ESM/ocean-team, and @GEOS-ESM/seaice-team. Also, the (all-surface) RUNOFF output is indeed NaN, which requires fixing by @atrayano (see comment above date 7 Nov 2024 6:17pm EST). I think it would be best merge this PR as is (pending the aforementioned review and approvals) and start a new PR that addresses the NaNs in the (all-surface) RUNOFF output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch. documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants