search

GEOS-ESM / GEOSchem_GridComp

GEOS Earth System Model Chemistry Components
Apache License 2.0
3 stars 4 forks source link

GNU Compiler Issue with GEOS-Chem #21

Closed mathomp4 closed 5 years ago

mathomp4 commented 5 years ago

Well, this is a fun one. Turns out Matt did something stupid yesterday. I decided to get our dev/MAPL-2.0 branch up to date. Unfortunately, I forgot that we really did dev/MAPL-2.0 off of master in regards to Chemistry, but in my zeal I pulled develop into our branch.

In a way it really didn't matter much to us since develop doesn't have changes (yet) to GOCART so things are zero-diff for our testing. But, it turns out develop (or rather GEOS-Chem v12.4.0) (re-)introduced a bug/standards violation such that GCC can't build with it:

[ 69%] Building Fortran object src/Components/@GEOSgcm_GridComp/GEOSagcm_GridComp/GEOSphysics_GridComp/@GEOSchem_GridComp/GEOSCHEMchem_GridComp/CMakeFiles/GEOSCHEMchem_GridComp.dir/gc_column/GIGC/gigc_providerservices_mod.F90.o
/discover/nobackup/mathomp4/SystemTests/builds/AGCM_GITHUBDEVGNU/CURRENT/GEOSgcm/src/Components/@GEOSgcm_GridComp/GEOSagcm_GridComp/GEOSphysics_GridComp/@GEOSchem_GridComp/GEOSCHEMchem_GridComp/gc_column/GIGC/gigc_providerservices_mod.F90:131:56:

   CHARACTER(LEN=15), PARAMETER :: COLLIST(8) = (/ 'NO2', 'O3',   'CH4', 'CO', &
                                                        1
Error: Different CHARACTER lengths (3/2) in array constructor at (1)

(The error is a bit early as there is even a 4-long string which is what the ultimate padding needs to be.)

It turns out the only change to this file between master and develop is the change in this constructor from: https://github.com/GEOS-ESM/GEOSchem_GridComp/blob/cbc9d6c1de857bde56a75d4ef8927f0d6b7d87a3/GEOSCHEMchem_GridComp/gc_column/GIGC/gigc_providerservices_mod.F90#L131-L132 to: https://github.com/GEOS-ESM/GEOSchem_GridComp/blob/2ec6dc8fed5700658c0c1637daace9f2fdd5cb60/GEOSCHEMchem_GridComp/gc_column/GIGC/gigc_providerservices_mod.F90#L131-L132

(There is a similar regression in GEOSCHEMchem_GridCompMod.F90 as well at line 245.)

The fix for this is trivial, and I can make a PR in a minute, but there is a question I have for @GEOS-ESM/chemistry-gatekeepers and @christophkeller: Where to make a PR?

I know you are looking at merging in dev/cakelle2_GCC_GEOS_12.4.0 into develop in #17. So, it could be folded there. But that PR does touch the GridComp but doesn't touch the GIGC file. Plus, #17 still seems in flux so perhaps a fix might not get into develop for a while. If I do put it against develop, then @christophkeller would probably need to rebase his branch to make sure the fixes are in it as well.

mathomp4 commented 5 years ago

Note: I could also look into reverting the last merge onto dev/MAPL-2.0. I'll discuss with @tclune as I'm of a two minds about what might be best for our tracking branch.

ETA: Talked with @tclune and it's actually a good thing we are now tracking develop. Huzzah for unintended consequences.

christophkeller commented 5 years ago

No strong opinion here. Sounds like this should go into 'develop' ASAP, so it makes sense to add it there right away. Since this is such a small fix I am happy to update 'dev/cakelle2_GCC_GEOS_12.4.0' accordingly.

mmanyin commented 5 years ago

Matt - please submit a PR and I will approve the merge into develop. Then Christoph and Elliot will need to update there current branches accordingly.

gmao-esherman commented 5 years ago

Sounds good to me.