-
Notifications
You must be signed in to change notification settings - Fork 170
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
Set a C-preprocessor switch so that we can block out code specific to the KPP rosenbrock_autoreduce integrator #2689
Open
yantosca
wants to merge
10
commits into
dev/14.6.0
Choose a base branch
from
feature/integrator-specific-handling
base: dev/14.6.0
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NOTE: These modifications were made because the prior method of using "#if KPP_INTEGRATOR == rosenbrock_autoreduce" did not work as expected. We assumed wrongly about how C-preprocssor conditionals work. KPP/carbon/CMakeLists.txt KPP/custom/CMakeLists.txt KPP/Hg/CMakeLists.txt KPP/fullchem/CMakeLists.txt - Added CMake code to define the KPP_INT_AUTOREDUCE and KPP_INT_LSODE switches based on the value of #INTEGRATOR in the *.kpp file. - Also print the KPP integrator name at configure time GeosCore/fullchem_mod.F90 KPP/fullchem/fullchem_AutoReduceFuncs.F90 KPP/stubs/stub_fullchem_AutoReduceFuncs.F90 - Wrap code specific to the rosenbrock_autoreduce integrator in "#ifdef KPP_INT_AUTOREDUCE" conditional blocks - Wrap code specific to the LSODE integrator in "#ifdef KPP_INT_LSODE" conditional blocks CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>
KPP/fullchem/CMakeLists.txt - Changed KPP_INTEGRATOR_NAME (incorrect) to KPP_INT_NAME when testing if the KPP integrator is "lsode" Signed-off-by: Bob Yantosca <[email protected]>
KPP/custom/CMakeLists.txt KPP/fullchem/CMakeLists.txt - Added "target_compile_definitions" statements to make sure that the KPP_INT_AUTOREDUCE and KPP_INT_LSODE variables are also saved to the GEOSChemBuildProperties. This will define them as C-preprocessor switches. Prior to this commit, these variables were defined but the corresponding C-preprocessor switches were not. - Updated comments KPP/carbon/CMakeLists.txt KPP/Hg/CMakeLists.txt - Added comments Signed-off-by: Bob Yantosca <[email protected]>
GeosCore/fullchem_mod.F90 - Set relevant ICNTRL and RCNTRL values within the #ifdef KPP_INT_LSODE C-preprocessor block for the LSODE integrator. These may need to be tweaked further.
GeosCore/fullchem_mod.F90 - Now pass 0.0 and DT instead of TIN and TOUT to the Integrate function. Some KPP integrators like LSODE will update the TIN variable upon output, which could have unintended consequences upstream. - Removed T, TIN, TOUT variables from routine DO_FULLCHEM - Updated and/or removed obsolete comments - Added TODO comments to denote code that could potentially be abstracted out of DO_FULLCHEM and into other subroutines, for clarity Signed-off-by: Bob Yantosca <[email protected]>
GeosCore/fullchem_mod.F90 - Add an #ifdef KPP_INT_BEULER block to set ICNTRL and RCNTRL for beuler KPP/custom/CMakeLists.txt KPP/fullchem/CMakeLists.txt - Add CMake code to define the KPP_INT_BEULER flag to 1 if the integrator name in custom.kpp/fullchem.kpp is "beuler" CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>
This merge brings updates (as of PR #2661) into the feature/integrator-specific-handling branch. This is necessary in order to avoid compilation errors w/ the new HEMCO logfile handling updates. Signed-off-by: Bob Yantosca <[email protected]>
This merge brings updates from GEOS-Chem 14.6.0-alpha.2 into the feature/integrator-specific-handling branch. This is needed in order to block out code specific to the rosenbrock_autoreduce integrator if we are using other integrators like LSODE or BEuler. Signed-off-by: Bob Yantosca <[email protected]>
GeosCore/fullchem_mod.F90 - Removed #ifdef blocks for LSODE and BEuler integrators KPP/custom/CMakeLists.txt KPP/fullchem/CMakeLists.txt - Remove code to define KPP_INT_LSODE and KPP_INT_BEULER Cpp switches CHANGELOG.md - Updated accordingly Signed-off-by: Bob Yantosca <[email protected]>
yantosca
added
category: Feature Request
New feature or request
topic: Chemical Mechanisms
Related to KPP and/or GEOS-Chem chemistry mechanisms
topic: Structural Modifications
Related to GEOS-Chem structural modifications (as opposed to scientific updates)
no-diff-to-benchmark
This update will not change the results of fullchem benchmark simulations
labels
Jan 16, 2025
Integration tests are running |
GEOS-Chem Classic integration tests passed: ==============================================================================
GEOS-Chem Classic: Execution Test Results
CodeDir : 1b20e4a GEOS-Chem & HEMCO updates: Merge PRs #2512, #279 (USTAR in DustDead)
GEOS-Chem : 562c6c32e Remove LSODE and BEULER specific code, we can add them back later
HEMCO : 2423647 Merge PR #279 (Use USTAR from met in DustDEAD extension)
Cloud-J : f8a2b7f Update version number for 8.0.1 release
HETP : 2a99b24 Merge pull request #2 from geoschem/bugfix/initialize_local_variables
Using 24 OpenMP threads
Number of execution tests: 30
Submitted as SLURM job: 837688
==============================================================================
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%% All execution tests passed! %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% |
GCHP integration tests all passed: ==============================================================================
GCHP: Execution Test Results
CodeDir : 3089c38 GEOS-Chem update: Merge PR #2683 (Restore clobbered TMB updates)
MAPL : 9ad63ae Merge PR #37 containing update to vertically flip imports with dimensionless pressure proxy lev coordinates
GMAO_Shared : 4ddb3ec Merge pull request #2 from geoschem/feature/mapl-upgrade
ESMA_cmake : ad5deba Added ecbuild as a submodule of ESMA_cmake
gFTL-shared : 4b82492 Merge branch 'upstream_v1.5.0' into feature/v1.5.0
FMS : 259759d Merge pull request #3 from geoschem/feature/update_gmao_libs
FVdycoreCubed : af42462 Merge PR #8 (Add PLEadv diagnostic for offline advection in GCHP)
geos-chem : 562c6c32e Remove LSODE and BEULER specific code, we can add them back later
HEMCO : 0ae25d2 HEMCO 3.10.1 release
yaFyaml : 19afe50 Merge branch 'upstream_v1.0.4' into feature/v1.0.4
pFlogger : 2c4b724 Merge branch 'upstream_v1.9.1' into feature/v1.9.1
Cloud-J : f8a2b7f Update version number for 8.0.1 release
HETP : 2a99b24 Merge pull request #2 from geoschem/bugfix/initialize_local_variables
Number of execution tests: 12
Submitted as SLURM job: 838169
==============================================================================
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
%%% All execution tests passed! %%%
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% |
lizziel
requested changes
Jan 27, 2025
CHANGELOG.md GeosCore/fullchem_mod.F90 KPP/Hg/CMakeLists.txt KPP/carbon/CMakeLists.txt KPP/custom/CMakeLists.txt KPP/fullchem/CMakeLists.txt KPP/fullchem/fullchem_AutoReduceFuncs.txt KPP/stubs/stub_fullchem_AutoReduceFuncs.txt - Renamed the C-preprocessor switch from KPP_INT_AUTOREDUCE to KPP_INTEGRATOR_AUTOREDUCE for clarity Signed-off-by: Bob Yantosca <[email protected]>
lizziel
approved these changes
Jan 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
category: Feature Request
New feature or request
no-diff-to-benchmark
This update will not change the results of fullchem benchmark simulations
topic: Chemical Mechanisms
Related to KPP and/or GEOS-Chem chemistry mechanisms
topic: Structural Modifications
Related to GEOS-Chem structural modifications (as opposed to scientific updates)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Name and Institution (Required)
Name: Bob Yantosca
Institution: Harvard + GCST
Describe the update
This PR adds code to automatically set a C-preprocessor switch
KPP_INT_AUTOREDUCE
based on the name of the KPP integrator that is being used. This is necessary so that we can block out code that is specific to therosenbrock_autoreduce
integrator, which would result in a compile-time error when we use other integrators.This is a necessary step before we can implement use other KPP integrators (e.g. LSODE, Backwards Euler, etc) with the GEOS-Chem chemistry mechanism.
Expected changes
This is a zero-diff update but should go into 14.6.0. It can be bundled with any other 14.6.0 PR.
Related Github Issue