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.

Sign up for GitHub

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

Jump to bottom

Throw a CMake error if FAST-JX is used for any KPP mechanism except Hg #62

Merged
merged 1 commit into from
Jul 15, 2024
Merged

Throw a CMake error if FAST-JX is used for any KPP mechanism except Hg #62

merged 1 commit into from
Jul 15, 2024

Conversation

yantosca
Copy link
Contributor

@yantosca yantosca commented Jul 12, 2024 •
edited
Loading

Name and Institution (Required)

Name: Bob Yantosca
Institution: Harvard + GCST

Describe the update

This PR adds an error trap that causes CMake to throw an error if the FASTJX compile option is selected for any mechanism except Hg.

Expected changes

Configuring with the -DFASTJX=y option now throws an error message such as:

CMake Error: The source directory "/n/holyscratch01/jacob_lab/ryantosca/tests/nodiff/gc237x/test_fullchem" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.
[holy8a24501 build]$ cmake ../CodeDir -DFASTJX=y
-- The Fortran compiler identification is GNU 10.2.0
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Check for working Fortran compiler: /n/sw/helmod-rocky8/apps/Core/gcc/10.2.0-fasrc01/bin/gfortran - skipped
=================================================================
GCClassic 14.4.1 (superproject wrapper)
Current status: 14.4.1-6-gdb89d20
=================================================================
-- Found NetCDF: /n/sw/helmod-rocky8/apps/MPI/gcc/10.2.0-fasrc01/openmpi/4.1.0-fasrc01/netcdf-fortran/4.5.3-fasrc01/lib/libnetcdff.so  
-- Useful CMake variables:
  + CMAKE_PREFIX_PATH:    /n/sw/helmod-rocky8/apps/MPI/gcc/10.2.0-fasrc01/openmpi/4.1.0-fasrc01/netcdf-c/4.8.0-fasrc01
  ...             /n/sw/helmod-rocky8/apps/MPI/gcc/10.2.0-fasrc01/openmpi/4.1.0-fasrc01/netcdf-fortran/4.5.3-fasrc01
  ...             /n/sw/helmod-rocky8/apps/MPI/gcc/10.2.0-fasrc01/openmpi/4.1.0-fasrc01/netcdf-c/4.8.0-fasrc01
  ...             /n/sw/helmod-rocky8/apps/MPI/gcc/10.2.0-fasrc01/openmpi/4.1.0-fasrc01/netcdf-fortran/4.5.3-fasrc01
  + CMAKE_BUILD_TYPE:     Release
-- Run directory setup:

-- Threading:
  * OMP:          ON  OFF
-- Found OpenMP_Fortran: -fopenmp (found version "4.5") 
-- Found OpenMP: TRUE (found version "4.5")  
-- General settings:
  * MECH:         fullchem  carbon  Hg  custom
  * USE_REAL8:    ON  OFF
  * SANITIZE:     ON  OFF
-- Components:
  * TOMAS:        ON  OFF
  * TOMAS_BINS:   NA  15  40
  * APM:          ON  OFF
  * RRTMG:        ON  OFF
  * GTMM:         ON  OFF
  * HCOSA:        ON  OFF
  * LUO_WETDEP:   ON  OFF
  * FASTJX:       ON  OFF
CMake Error at CMakeScripts/GC-ConfigureClassic.cmake:204 (message):
  FASTJX can only be used with the Hg mechanism!
Call Stack (most recent call first):
  CMakeLists.txt:226 (configureGCClassic)


-- Configuring incomplete, errors occurred!
See also "/n/holyscratch01/jacob_lab/ryantosca/tests/nodiff/gc237x/test_fullchem/build/CMakeFiles/CMakeOutput.log".

Related Github Issue

@yantosca
Throw a CMake error if FAST-JX is used for any mech except Hg
db89d20 
CMakeScripts/GC-ConfigureClassic.cmake
- Add an if block to throw a fatal error if the FASTJX compile option
  is selected for fullchem, custom, or carbon simulations.  Fullchem
  and custom mechanisms use Cloud-J.  We still need to use the FAST-JX
  photolysis scheme with Hg pending input file updates.

CHANGELOG.md
- Updated accordingly

Signed-off-by: Bob Yantosca <yantosca@seas.harvard.edu>
@yantosca yantosca added category: Feature Request New feature or request topic: Build Related to CMake and/or the build sequence no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations labels Jul 12, 2024
@yantosca yantosca added this to the 14.4.2 milestone Jul 12, 2024
@yantosca yantosca self-assigned this Jul 12, 2024
@yantosca yantosca changed the title Throw a CMake error if FAST-JX is used for any mech except Hg Throw a CMake error if FAST-JX is used for any KPP mechanism except Hg Jul 12, 2024
@yantosca
Copy link
Contributor Author

Integration tests are in progress.

@yantosca
Copy link
Contributor Author

All GEOS-Chem integration tests passed:

==============================================================================
GEOS-Chem Classic: Execution Test Results

GCClassic #655527d Merge PR #62 (Throw error if FAST-JX is used w/ non-Hg mechanisms)
GEOS-Chem #03287a67a Merge PR #2372 (Turn off MEGAN extension for non-benchmark sims)
HEMCO     #2192e0e HEMCO 3.9.1 release

Using 24 OpenMP threads
Number of execution tests: 28

Submitted as SLURM job: 39695010
==============================================================================

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%%  All execution tests passed!  %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

All tests were identical to PR #2372 except:

  • TOMAS (parallelization error?)
  • APM (parallelization error?)

@yantosca yantosca requested a review from lizziel July 12, 2024 21:21
lizziel
lizziel requested changes Jul 15, 2024
View reviewed changes
CHANGELOG.md
@@ -6,6 +6,10 @@ This file documents all notable changes to the GEOS-Chem Classic wrapper reposit

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased] - TBD
### Changed
- CMake now throws an error if FAST-JX is used with any other mechanism than Hg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be changed to simply "Discontinue support of FAST-JX for all but mercury simulations". This will make it more clear to users what the change is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a GEOS-Chem PR that goes with this to remove the option for FAST-JX except for mercury.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lizziel. I'm not sure how to implement a GEOS-Chem PR that would prevent use of FAST-JX in the GEOS-Chem repo. There is no good way to compare strings in preprocessor statements (i.e. #if MECH == "Hg" won't work).

What I could do is to remove the FAST-JX CMake compile option and then set FASTJX=1 if -DMECH=Hg is passed at the command line. This would disable FAST_JX for all mechanisms except Hg.

Copy link
Contributor

@lizziel lizziel Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to keep the compile option so that it is easy for Hg developers to switch between the two. What I was thinking was put an error message within a FASTJX ifdef that if not a mercury simulation then fail. We need this for uses of GEOS-Chem outside of the GC-Classic and GCHP super-projects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To further clarify, this would use the Input_Opt logical for Hg simulation, not the compile-time MECH setting.

@lizziel lizziel mentioned this pull request Jul 15, 2024
Closed
@yantosca yantosca merged commit 655527d into dev/no-diff-to-benchmark Jul 15, 2024
5 checks passed
@yantosca Octobox
Copy link
Contributor Author

Thanks @lizziel. I can do that.

@yantosca yantosca mentioned this pull request Jul 15, 2024
Closed
@lizziel lizziel mentioned this pull request Jul 15, 2024
Closed
@yantosca yantosca deleted the dev/disable-fastjx-for-fullchem branch July 26, 2024 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lizziel lizziel lizziel requested changes

Assignees

@yantosca yantosca

Labels
category: Feature Request New feature or request no-diff-to-benchmark This update will not change the results of fullchem benchmark simulations topic: Build Related to CMake and/or the build sequence
Projects
None yet
Milestone
14.4.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@yantosca @lizziel