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

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

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.


## [14.4.1] - 2024-06-28
### Fixed
- Fixed formatting error in `.github/workflows/stale.yml` that caused the Mark Stale Issues action not to run
Expand Down
8 changes: 7 additions & 1 deletion CMakeScripts/GC-ConfigureClassic.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,18 @@ function(configureGCClassic)
#-------------------------------------------------------------------------
# Use Fast-JX rather than Cloud-J?
#-------------------------------------------------------------------------

set(FASTJX OFF CACHE BOOL
"Switch to use legacy FAST-JX in GEOS-Chem"
)
gc_pretty_print(VARIABLE FASTJX IS_BOOLEAN)
if(${FASTJX})
#---------------------------------------------------------------------
# Throw an error unless we are using the Hg mechanism,
# The fullchem & custom mechanisms now use Cloud-J!
if(NOT ${MECH} MATCHES "Hg")
message(FATAL_ERROR "FASTJX can only be used with the Hg mechanism!")
endif()
#---------------------------------------------------------------------
target_compile_definitions(GEOSChemBuildProperties
INTERFACE FASTJX
)
Expand Down
Loading