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

[DISCUSSION] C-preprocessor switches in GEOS-Chem #168

Closed
lizziel opened this issue Dec 10, 2019 · 20 comments
Closed

[DISCUSSION] C-preprocessor switches in GEOS-Chem #168

lizziel opened this issue Dec 10, 2019 · 20 comments

Comments

@lizziel
Copy link
Contributor

lizziel commented Dec 10, 2019

This thread is for discussion of cpp switches in GEOS-Chem. Some of the existing ones can be removed, consolidated, or renamed. There will be more switches added as we interface with more ESMs. Ideally we would have an agreed upon philosophy about using them, make that philosophy public, and aim to consistently apply it. I look forward to hearing everyone's thoughts!

Please also see comments on the following two relevant PRs: #5, #165

Here are some thoughts to get us started:

In PR5, I put forward this idea, which gives some backstory on why the MODEL_x switches are the way they are.

For clarity at this time I think we should stick with using MODEL_{name} rather than MODEL_. Each instance could be followed by comments in the code on the reasoning behind its use and the potential for generalization in the future. For example, rather than MODEL_ we would have MODEL_WRF with a brief note that it is there because of some feature (or lack of feature) in WRF that may also be applicable to other models interfacing with GEOS-Chem. Over time we may accumulate instances of MODEL_WRF || MODEL_CESM, etc. Eventually we can survey all of these instances and come up with a strategy for generalizing across models.

However, currently we do not have MODEL_CLASSIC or MODEL_GCHP. I am in favor of having both, and following the guidelines I outline above where we periodically consolidate if there is an obvious shared trait. I am also in favor of replacing cpp switches with run-time config options where possible.

@yantosca
Copy link
Contributor

I think that is the best way forward, to use MODEL_CLASSIC, MODEL_GCHP, MODEL_GEOS, MODEL_WRF (and presumably soon MODEL_CESM2). That will let us get rid of the ESMF_ switch.

I also agree we should try to replace cpp switches with runtime options where feasible. But for architecture reasons, we will probably always have to incur some CPP switches.

@jimmielin
Copy link
Contributor

Thanks Lizzie for opening this. I am a proponent of MODEL_CLASSIC and MODEL_GCHP but we should tread carefully on adding these switches, either to include or exclude features. If we do not have someone watching over all the instances it could be very easy to have a slip-up like the tracerinfo.dat issue I found a couple days back. We definitely need to have a convention on when to adopt these switches and use them as a last resort if the code cannot be walled off under a Input_Opt option, or BPCH_TPBC, BPCH_DIAG, etc..

@lizziel
Copy link
Contributor Author

lizziel commented Dec 19, 2019

Here is a list of the CPP switches we currently have in the geos-chem repository:

USE_TIMERS
DEBUG
DEVEL
LINUX_IFORT
LINUX_GFORTRAN
NULL
USE_REAL8
BPCH_DIAG
NC_HAS_COMPRESSION
USE_TEND
MODEL_GEOS
MODEL_WRF
GISS
MODELE
MODEL_
MPI
ESMF_
EXTERNAL_GRID
EXTERNAL_FORCING
SPMD
PILGRIM
MESSY
USE_LINEAR_OVERLAP
USE_APPROX_RANDOM_OVERLAP
USE_MAXIMUM_RANDOM_OVERLAP
SKIP_IF_P_AND_T_ARE_OUT_OF_RANGE
USE_ALL_TAG3_SPECIES
NEW_HENRY_CONSTANTS
EXCHANGE
EXCHANGE_2x25_CH
EXCHANGE_2x25_NA
EXCHANGE_2x25_EU
RRTMG
APM
ISORROPIA_V22
TOMAS
TOMAS12
TOMAS15
TOMAS30
TOMAS40

@yantosca
Copy link
Contributor

yantosca commented Dec 19, 2019

To further clarify, these #ifdefs are manually set by uncommenting lines of Fortran source code. They are typically intended to hardwire a default configuration, but to allow users to choose an alternate configuration if they need it for research:

  1. USE_LINEAR_OVERLAP (FAST-JX)
  2. USE_APPROX_RANDOM_OVERLAP (FAST-JX)
  3. USE_MAXIMUM_RANDOM_OVERLAP (FAST-JX)
  4. SKIP_IF_P_AND_T_ARE_OUT_OF_RANGE (ISORROPIA)
  5. ISORROPIA_V22
  6. USE_ALL_TAG3_SPECIES (tagged_O3_mod)
  7. NEW_HENRY_CONSTANTS (species database -- can be eliminated with new constants in 12.8)

The rest are set at compile time by setting a compiler variable.

@yantosca
Copy link
Contributor

MESSY is used to pick the MESSY regridding in HEMCO.

PILGRIM and SPMD are hardwired into the tpcore modules. They block out MPI-specific code that we don't need in GC Classic.

DEVEL might be able to be removed.

NC_HAS_COMPRESSION is needed to prevent us trying to use netCDF compression in GC-Classic if the user only has netcdf3. It is toggled by NC_NODEFLATE=y. I typically use this to keep the file sizes identical so we can diff them directly.

The EXCHANGE* switches toggle the two-way nesting. This option is currently broken in GC12. Jintai Lin's group is the only one who uses this.

GISS and MODELE might have been added by Lee Murray for compatibility with the ICECAP code, but that is a separate repository.

@msulprizio
Copy link
Contributor

DEVEL might be able to be removed.

As part of the conversion from F77 to F90, I've been replacing #if defined (DEVEL) with IF ( Input_Opt%LPRT) THEN. So that's one less CPP switch to worry about.

@yantosca
Copy link
Contributor

yantosca commented Feb 4, 2020

We can add MODEL_CLASSIC and MODEL_GCHP to 12.8.0. I'll add a separate issue.

@jimmielin
Copy link
Contributor

jimmielin commented Feb 4, 2020

I'd also like to add that MODEL_ was added for generic code that belongs to coupled models, designed to basically replace EXTERNAL_GRID and EXTERNAL_FORCING (which should be removed).

One should take care of not to confuse MODEL_CLASSIC, MODEL_GCHP with others (i.e. MODEL_GEOS, MODEL_CESM, MODEL_WRF), which imply coupling and also carry MODEL_ with them.

There probably needs to be a run through old flags like ESMF_ & make those more specific (really ESMF or GCHP or GCHP/GEOS-5 MAPL?) as we introduce a non-MAPL copy of ESMF into the game with CESM2-HEMCO-GC.

@yantosca
Copy link
Contributor

yantosca commented Feb 4, 2020

@jimmielin: I was thinking that a MODEL_XXX flag would denote code that was specific to a particular model XXX, even if it doesn't use coupling (e.g. MODEL_CLASSIC, MODEL_GCHP). Should we be thinking about a different syntax for these flags? Maybe INTERFACE_CLASSIC and INTERFACE_GCHP?

@jimmielin
Copy link
Contributor

@jimmielin: I was thinking that a MODEL_XXX flag would denote code that was specific to a particular model XXX, even if it doesn't use coupling (e.g. MODEL_CLASSIC, MODEL_GCHP). Should we be thinking about a different syntax for these flags? Maybe INTERFACE_CLASSIC and INTERFACE_GCHP?

I would suggest INTERFACE_CLASSIC and INTERFACE_GCHP, to make the distinction between coupled models with external online meteorology (and so it does not clash with MODEL_). But I can imagine there will be proponents for either.

@lizziel
Copy link
Contributor Author

lizziel commented Feb 4, 2020

I still have the preference of using MODEL_XXX and just stringing together & logic if multiple are used. The benefits I see for this are:

  1. maximizes clarity to people who don't know what all the possible couplings are
  2. makes searching very simple since you don't have to remember all the relevant flags for your particular model
  3. avoids the need for MODEL_ on top of MODEL_XXX

If the cpp logic eventually got cumbersome from the string of MODEL_XXX then they could be simplified at a later stage.

As for MODEL_XXX or INTERFACE_XXX, MODEL_XXX is fewer characters. But I'm not super invested in the prefix as long as it's consistent. What I am invested in is minimizing switches and keeping the names logical, intuitive, and easily searchable.

@yantosca
Copy link
Contributor

yantosca commented Feb 5, 2020

I tend to agree with @lizziel. MODEL_XXX is more intuitive.

@jimmielin
Copy link
Contributor

I agree with @lizziel. The number of highly "generic" model interfaces is not many. The following come to mind and I think it's acceptable to keep MODEL_XXX chained together for those for now.

  • bypassing UCX netCDF inputs - all but MODEL_CLASSIC (does MODEL_GCHP use this?)
  • disable GAMAP / bpch output, tracerinfo.dat stuff -- relevant for everything except MODEL_CLASSIC
  • pressure accept external Ap, Bp - used MODEL_CESM and MODEL_WRF; under MODEL_ for now
  • set grid by center and edges - used MODEL_CESM and MODEL_WRF; under MODEL_WRF for now, I was going to migrate it to MODEL_
  • bypass main.F; under MODEL_ && ESMF_ for now; could do MODEL_CLASSIC.

@jimmielin
Copy link
Contributor

Hi @lizziel may I confirm from #213 that MODEL_CLASSIC and MODEL_GCHP will go in 12.7.1? If yes I will adopt these flags in HEMCO_CESM 3.0 immediately so we don't come back and change the preprocessors later. Thanks!

@lizziel
Copy link
Contributor Author

lizziel commented Feb 18, 2020

Yes, they are already live in the dev/12.7.1 branch.

@msulprizio
Copy link
Contributor

Several CPP switches have been removed as part of the update to .F90 and general cleanup (#92), including:

USE_TIMERS
DEBUG
DEVEL
NULL
USE_TEND

@lizziel
Copy link
Contributor Author

lizziel commented Aug 5, 2020

I am working on cleaning up some of the CPP switches for 13.0 to use MODEL_CLASSIC, MODEL_GCHPCTM, etc.

@jimmielin, what is the full list that WRF-GC is compiled with? I'm looking specifically for if it uses MPI, EXTERNAL_FORCING, EXTERNAL_GRID, or MODEL_.

@fritzt: same question for CESM.

@jimmielin
Copy link
Contributor

Hi @lizziel we use MODEL_ and MODEL_WRF, but we also pass the EXTERNAL_GRID and EXTERNAL_FORCING. But we don't use those specifically so it is OK if you merge these with the others. Thanks!

@fritzt
Copy link
Contributor

fritzt commented Aug 5, 2020

Hi @lizziel !

For CESM-GC, we define a number of GEOS-Chem relevant CPP flags:
EXTERNAL_GRID
EXTERNAL_FORCING
LINUX_IFORT
USE_REAL8
MODEL_
MODEL_CESM
HEMCO_CESM

On top of that, CESM uses some additional CPP flags, which are (most likely):
PLON
PLAT
PLEV
PCNST
PCOLS
PSUBCOLS
N_RAD_CNST
PTRM
PTRN
PTRK
SPMD
MODAL_AERO
MODAL_AERO_3MODE, MODAL_AERO_4MODE or MODAL_AERO_7MODE
CLM40, CLM45 or CLM50
CLUBB_SGS
CLUBB_CAM
NO_LAPACK_ISNAN
CLUBB_REAL_TYPE
HAVE_VPRINTF
HAVE_TIMES
HAVE_GETTIMEOFDAY
HAVE_COMM_F2C
HAVE_NANOTIME
BIT64
HAVE_SLASHPROC
Among those, I believe that only SPMD should appear in GEOS-Chem.

@lizziel
Copy link
Contributor Author

lizziel commented Aug 5, 2020

Excellent. Thanks!

@yantosca yantosca added the never stale Never label this issue as stale label Jan 13, 2021
@msulprizio msulprizio removed the never stale Never label this issue as stale label Mar 3, 2021
@geoschem geoschem locked and limited conversation to collaborators Aug 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants