Implement CI on GH Actions

[codecov[bot]]

Codecov Report

Merging #441 (c016204) into master (9372a30) will decrease coverage by 0.02%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
- Coverage   88.73%   88.71%   -0.03%     
==========================================
  Files          17       17              
  Lines        2264     2268       +4     
==========================================
+ Hits         2009     2012       +3     
- Misses        255      256       +1     

Flag	Coverage Δ
build	62.84% <ø> (+0.34%)	:arrow_up:
tests	87.78% <ø> (-0.12%)	:arrow_down:
unittests	72.13% <ø> (+36.40%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/outputFunctions.f90	88.75% <0.00%> (-0.56%)	:arrow_down:
src/dataStructures.f90	95.07% <0.00%> (+0.03%)	:arrow_up:
src/solverFunctions.f90	91.35% <0.00%> (+0.10%)	:arrow_up:
src/argparse.f90	65.00% <0.00%> (+0.25%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9372a30...828fb2c. Read the comment docs.

[spco]

@rs028 - I think the lion's share of the work here is done. Before merging, I will want to replace all remaining references to Travis (such as in the docs, and the name of the travis directory, as well as removing .travis.yml). What thoughts do you have on the naming? Obviously in the docs I should refer to GitHub Actions when talking about the specific provider. I'd suggest we rename the travis directory to ci or similar - what do you think?

I will also rename .github/workflows/test.yml to a more appropriate filename - perhaps ci.yml.

Finally, I will squash this all into one commit (it's so messy!) before merging.

[rs028]

Nice! It looks like the output logs are more tidy as well. What are those warnings about SHA?

The only thing that worries me a bit about this setup is whether we are then too dependent on the specific github infrastructure. Should we, in the future, be force to move to another system for whatever reason. Is it something to worry? I honestly didn't have time to study how Actions work.

Okay regarding the renaming and removing refs to Travis (I think we need to disable it in the repo settings as well). Does the description on how to set up new tests in the manual require changing?

[spco]

I'll hopefully fix the SHA warning - it's a known thing according to my research.

Sure, I think it's right to consider whether we're going to get locked in. I don't think it's a worry for now, because most of these tend to have pretty similar syntax that takes a few hours' work to translate across. So I don't think it would be huge to move away from Actions if the need arose. The most robust setup would probably be a Jenkins setup that can be hosted on any of the providers with minimal modification, but I don't know enough about Jenkins to do that for less effort. We also have the Travis setup in the git history for reference if we ever had to move back there or to a system closer to that than Actions. Either way, I would say there's the small chance of lock-in, but even if it became an issue, it wouldn't be difficult to move away - our setup is not complex!

I'll take a look at the manual and attempt to fix anything I see that is affected.

[spco]

I'm actually going to rename travis/ to test/ as it contains all the test stuff, and nothing in there is CI-specific (it can all be run locally as well, not just on a CI system).

[spco]

Ok @rs028 - I think this is ready to go. I will squash into a single merge commit I think, as so much of this history is messy.

All that remains (for another PR) is the editing of the table in the manual, and the regenerating of the manual PDF.

Are you happy for me to merge this? I can then also remove the existing link to Travis in Settings.

[rs028]

Awesome! Great job.

If I understand this is testing compilation of AtChem with gfortran v8 and v9 on linux and mac? I have two questions: the different compilers are not used to compile CVODE and the other dependencies, is that right? Also looking at the output of the checks I noticed a LAPACK warning that it is not functional and won't be used (line 333 of build (ubuntu-latest, 8)). What does it mean?

I will do the changes to the manual in a separate PR.

[spco]

Yes, v8 and 9 on both mac and linux.

We do use these versions to install cvode (this is sent to install_cvode.sh with the $(which gfortran-${{ matrix.fortran }}) on lines 33/38 of ci.yml.

But we don't use them for openlibm/numdiff - but I'm not sure whether they even require a Fortran compiler, so I think we are fine either way.

I think the Lapack warning can be ignored for CI - it's unable to find the Lapack libraries so doesn't use them. Given that the linkage to Lapack is only within CVODE, it shouldn't make a difference to us - CVODE will expose exactly the same API to AtChem2 regardless, but it would be more performant using Lapack rather than its own built-in solvers.

So I can't see any situation where that warning would make any difference to whether the pipeline pass/fails, apart from a small chance of rounding differences due to implementation differences.

[rs028]

I understand. But does it mean LAPACK is not really a requirement?

[spco]

Yes - CVODE can function perfectly happily without it, but using Lapack allows you to use optimised solvers etc.

[rs028]

So it is a bit like openlibm. It is not really needed but gives better results? BTW, yes I am happy to merge :)

[spco]

Yes. In fact, I'm not sure whether with the calls we use from CVODE whether Lapack is ever even used if available. I would need to look into it to understand that - it's been too long since we wrote this!

[rs028]

All right. Probably the best place to investigate this is #257. I will make a note there.