Bring over main branch changes from geoschem/KPP

[laestrada]

@yantosca This PR contains the code updates from the main branch of geoschem/KPP. I looked over the changes and didn't see any code that looked too specific to GC -- the assumption being that any GC specific code we would need to develop some logic to turn those features on or off. However, I think you may be a better judge in that regard than I am.

[laestrada]

@RolfSander On point 1, I think your approach for Makefile.defs is an improvement over the current method. Although I suggest having either a separate directory (Makefile_defs/) with the various system specific makefiles in place, or system specific boolean logic in a single Makefile.defs.

On your comments:

• I defer to @msl3v on whether the scanner warnings are intended for production, since I believe he made that change.
• Noted. I have added the Mecca branch to the readme
• Agreed. I have ticked back the version number

[RolfSander]

Since there are only 4 active lines in Makefile.defs, I agree that we can put the code for all systems into just one file. Maybe even inside the Makefile itself, e.g.:

SYSTEM := $(shell uname)
HOST := $(shell hostname)
ifeq ($(SYSTEM),Linux)
  CC=gcc 
  CC_FLAGS=-O -Wno-implicit-function-declaration
  FLEX=flex
  FLEX_LIB_DIR=/usr/lib
endif
ifeq ($(HOST),mycomputer)
  CC=cc 
  CC_FLAGS=-O
  FLEX=flex
  FLEX_LIB_DIR=/usr/lib64
endif

[msl3v]

Are there any specific behaviors associated with the FAMILIES functionality that I need to address? The code for families shouldn't be executed unless it is enabled in *.kpp. I'm also not aware of any scanner warnings. Please point me to specifics if I'm wrong.

[RolfSander]

Regarding the scanner warnings: If we don't know who made these changes, I suggest we simply keep ScanError as it was in version 2.2.3.

However, we may want to make the error message a bit more user-friendly. Currently, you only get the equation number. We could print out the whole equation.

[RolfSander]

Thanks for implementing the adaptive Makefile.defs and the updated max values!

I have a question about the branches. We have a "dev" and a "main" branch. In case we want to follow the "git-flow" workflow (see https://nvie.com/files/Git-branching-model.pdf for details), all the development should go into "dev". Only when preparing a new KPP release, the code should be (after testing) merged into "main". So maybe the recent changes should go into "dev", not into "main". What do you think?

[laestrada]

I agree that we should follow the standard git-flow, but seeing as the changes in this branch are directly from the main branch of the old repository (geoschem/KPP), I considered the changes here to represent a previously released version. However, I'm happy to reconsider this. @RolfSander @yantosca Thoughts?

Additionally, I found that increasing MAX_EQN or MAX_SPECIES any higher than 1023 results in a seg fault when running our CI build, which currently runs examples/small_f90.kpp

[yantosca]

A couple of thoughts:

(1) I think we had used a FLEX_HOME variable to specify the path to flex. This could be used if it's not found in /usr/lib or /usr/lib64, i.e. if it's from a Spack library stack.

(2) The "dev" branch was updates that @LiamBindle and I had made to make KPP compile with CMake (while preserving the GNU Make capability). That is something that could be brought in at a later stage after validation (to make sure it doesn't break anything w/r/t backwards compatibility).

[RolfSander]

The main branch of the old repository represented a released version, but that version hasn't been tested on either MISTRA or MECCA yet. Anyway, I think the easiest solution is if you continue the merge into "main", and we'll do the checks with MISTRA and MECCA later. Next, to get back on track, it will be necessary to merge "main" into "dev".

For our future workflow, I'd prefer the git-flow method going via the dev branch.

[RolfSander]

I think the dev branch that Bob refers to is now the branch called gc_dev. Right? We can probably call it a "feature branch" and bring it in at a later stage, as Bob suggests.

[RolfSander]

My high values of MAX_EQN and MAX_SPECIES can be quite a challenge for machines that don't have enough memory. Let's stick to 1023 for now...

[laestrada]

Yes, the dev branch from geoschem/KPP is now the gc_dev branch in this repository. Your plan sounds good to me, thanks for your thoughts on this.