KineticPreProcessor / KPP

The KPP kinetic preprocessor is a software tool that assists the computer simulation of chemical kinetic systems
GNU General Public License v3.0
21 stars 11 forks source link

[WIP] Add continuous integration tests ci-tests folder structure #12

Closed yantosca closed 2 years ago

yantosca commented 2 years ago

We have discovered that the examples folder does not contain all of the files in one location that are needed to build and run the examples. To facilitate continuous integration testing, I have created a new folder called ci-tests, that will contain several subfolders, each of which is used to test a single F90 mechanism.

For example, the folder ci-tests/small_f90 contains these files:

atoms -> ../../models/atoms
general.f90 -> ../../drv/general.f90
kpp_lsode.def -> ../../int.modified_WCOPY/kpp_lsode.def
kpp_lsode.f90 -> ../../int.modified_WCOPY/kpp_lsode.f90
Makefile -> Makefile_small_f90
rosenbrock_soa.def -> ../../int.modified_WCOPY/rosenbrock_soa.def
rosenbrock_soa.f90 -> ../../int.modified_WCOPY/rosenbrock_soa.f90
small_f90.dat
small_f90.kpp -> ../../examples/small_f90.kpp
small_strato.def -> ../../models/small_strato.def
small_strato.eqn -> ../../models/small_strato.eqn
small_strato.spc -> ../../models/small_strato.spc

Most of the needed files are just linked to their locations in the repository.

To run the test, use these commands:

$ cd ci-tests/small_f90
$ kpp small_f90.kpp
$ make 
$ ./small_f90.exe

I will try to add tests for the other F90 mechanisms, and maybe down the road, a GEOS-Chem mechanism as well.

RolfSander commented 2 years ago
laestrada commented 2 years ago

@RolfSander to turn on the ci pipeline in your branch you would need to include the files found in the directory .ci-pipelines to your branch and update .ci-pipelines/build-testing.yml to include MECCA as a pipeline triggering branch. eg:

trigger:
  branches:
    include:
      - dev*
      - patch*
      - bugfix*
      - MECCA
    ....

The pipeline itself builds a docker image (.ci-pipelines/Dockerfile) that does the following: installs the necessary software requirements, compiles kpp, and runs some of the toy mechanisms .ci-pipelines/ci-testing-script.sh. If you want to try building the docker image locally, there are some instructions in the Dockerfile for how to do that. You will need to install Docker as a pre-req.

RolfSander commented 2 years ago

Thanks! I will try it...

laestrada commented 2 years ago

@yantosca We should probably adjust the .ci-pipelines/ci-test-script.sh to run the mechanisms in your new ci-tests directory instead of examples

yantosca commented 2 years ago

Thanks for the comments @RolfSander and @laestrada, very helpful:

  1. We would need a .gitignore in every folder, and I can copy that in. .gitignore only ignores code in the given folder.
  2. Maybe what I could do is to add a case to the Makefile.defs for linux_gcc, as a generic configuration for Linux + GNU compilers. That would probably be the easiest.
  3. @laestrada has set up an Azure pipeline so that we could CI tests every time a commit is pushed to Github. We would just need to set up a folder for the MECCA test and then that could be included with the other CI test. Pretty neat actually.
  4. I wasn't sure what the difference between int and int.modified_WCOPY was. The particular example needed files that were in int.modified_WCOPY but int in int. Maybe I could copy the files to the folder instead of linking them. I thought that linking would be better in case there was ever any reason to modify the originals, that we wouldn't have any duplicate versions floating around. Thoughts?
RolfSander commented 2 years ago

Thanks, @yantosca for the replies!

1) I think that ignore rules apply to the current folder as well as to all subfolders. Thus, if we add *.exe to .gitignore in the root directory, the *.exe will be excluded in all directories.

https://linuxize.com/post/gitignore-ignoring-files-in-git/

2) Sounds good. Or you can even define gfortran as the default for everyone in Makefile.defs. This can still be overwritten in the settings for specific systems, if necessary.

4) Ah, I see. The rosenbrock_soa solver only exists in int.modified_WCOPY/. Maybe it just hasn't been converted to Fortran90 array instructions yet. I have never used the second order adjoint, so I was not aware of the missing file in int/.

I also prefer linking instead of copying.

yantosca commented 2 years ago

@RolfSander I wonder if a better solution than to add entries in Makefile.defs for specific systems would be to define a couple of user-defined variables to specify the compiler & flags. It could happen that the Makefile.defs would get very long with lots of potenital users and machines. At least we could do something like KPP_CC, KPP_CC_FLAGS, KPP_FC, KPP_FC_FLAGS. Thoughts.

RolfSander commented 2 years ago

@yantosca I think we can have both. First, we define settings for specific systems in the ifeq ($(SYSTEM),...) blocks. Then, we can have machine- and/or user-specific variables in ifeq ($(HOST),...) and ifeq ($(USER),...) blocks. These can overwrite the machine-specific settings, if necessary.

yantosca commented 2 years ago

Thanks @RolfSander and @laestrada. I did not know about the global gitignore but I can set that up. I will also try to set up GCC as default in Makefile.defs and then let people override that. Will also change the ci-test-script.sh.

yantosca commented 2 years ago

I removed the saprcnov test because the integrator does not use the modern F90 commands (still relies on Blas/Lapack). But I added tests for the sd (Sdirk) and rk (Runge-Kutta) examples, as well as keeping the small_f90 example.

These all passed the CI integration on our Azure pipelines: https://dev.azure.com/KineticPreProcessor/KPP/_build/results?buildId=19&view=results

Also note, for the F90 makefile, I just wrapped this with a

#ifndef COMPILER
#COMPILER = G95
#COMPILER = LAHEY
COMPILER = INTEL
#COMPILER = HPUX
#COMPILER= GFORTRAN
endif

if you then compile with

$ make -f Makefile_small_f90 COMPILER=GFORTRAN

then that will trigger the build with Gfortran instead of ifort. Good enough for running the CI tests!

I'll merge this into dev. Thanks for your help!

yantosca commented 2 years ago

Also if you look at the raw log file you can see the tests actually created output: https://dev.azure.com/KineticPreProcessor/ac23fd44-a68c-4633-a07a-eba20ddd6b28/_apis/build/builds/19/logs/6

RolfSander commented 2 years ago

sdirk.f90 was missing in ci-tests/sd

Somehow I don't understand why the CI test was successful...

RolfSander commented 2 years ago

Thanks for your help again, @laestrada! I have now managed to run CI for my MECCA branch. However, I also encounter the strange MAX_EQN,MAX_SPECIES < 1024 limitation, even when I run docker locally.

Since I definitely need larger values, I don't want to reduce MAX_EQN and MAX_SPECIES in my branch. Instead, I suggest to add the following block to the Dockerfile (next to the other sed commands):

# for CI, MAX_EQN and MAX_SPECIES must be < 1024:
RUN sed -i 's/#define MAX_EQN .*/#define MAX_EQN 1023/g' /kpp/src/gdata.h \
    && sed -i 's/#define MAX_SPECIES .*/#define MAX_SPECIES 1023/g' /kpp/src/gdata.h
laestrada commented 2 years ago

@RolfSander Docker has a default memory size and number of cpus, which you can specify using flags, but azure pipelines limit the memory of the host machine anyway. Your suggestion makes sense to me -- glad you got the ci working.