Skip to content

Tweaks and fixes for GA and Codecov configs#58

Merged
danielhollas merged 1 commit into
masterfrom
separate-unittests
Feb 1, 2021
Merged

Tweaks and fixes for GA and Codecov configs#58
danielhollas merged 1 commit into
masterfrom
separate-unittests

Conversation

@danielhollas

@danielhollas danielhollas commented Jan 20, 2021

Copy link
Copy Markdown
Contributor
  • Codecov reports with GFortran 8 and 9 were crashing because we were not setting the correct 'gcov' version.

  • Some caches were not used correctly.
    The GA config is getting a bit unwieldy. :-(

  • GA: Pin Ubuntu version to 18.04:
    Github will soon switch it's 'ubuntu-latest'
    runner to 'ubuntu-20.04' and we're not ready:

    1. Some tests are failing with minor numerical differences on 20.04
      Hopefully this get's solved with Stabilize tests #27.

    2. Some jobs in GFortran CI config now rely on the fact
      that the default GCC/GFortran version is 7. We'll need to make that explicit
      if we want to switch to 20.04. We want to keep the default version = 7
      since that's the one we're using on the PHOTOX clusters.

    3. Given our ancient Linux versions on PHOTOX clusters,
      it probably makes sense to stick to 18.04 for now.
      (and maybe in the future switch one job to 20.04)

  • GA: Separate unit tests from end-to-end tests
    This is just for a convenience, to make it easier
    to inspect Github Action workflows. This will matter more
    as we'll add more unit tests. :-)
    This also makes the Makefile a bit more consistent;
    treating e2e and unit tests on equal footing.

@danielhollas danielhollas self-assigned this Jan 20, 2021
@danielhollas danielhollas added the testing Any changes to Github Actions or testing scripts. label Jan 20, 2021
@danielhollas danielhollas force-pushed the separate-unittests branch 4 times, most recently from ffd8f7c to 5ff5cc8 Compare January 20, 2021 17:27
@codecov

codecov Bot commented Jan 20, 2021

Copy link
Copy Markdown

Codecov Report

Merging #58 (2860c44) into master (4afc0e5) will decrease coverage by 0.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
- Coverage   64.06%   63.95%   -0.12%     
==========================================
  Files          37       37              
  Lines        5661     5674      +13     
==========================================
+ Hits         3627     3629       +2     
- Misses       2034     2045      +11     
Impacted Files Coverage Δ
src/forces.F90 82.63% <0.00%> (-0.60%) ⬇️
src/potentials.F90 18.26% <0.00%> (-0.09%) ⬇️
src/force_tera.F90 0.00% <0.00%> (ø)
src/force_terash.F90 0.00% <0.00%> (ø)
src/remd.F90 83.92% <0.00%> (+0.29%) ⬆️

sudo update-alternatives --install /usr/bin/gcc gcc /usr/bin/gcc-${GCC_V} 100 \
--slave /usr/bin/gfortran gfortran /usr/bin/gfortran-${GCC_V}
--slave /usr/bin/gfortran gfortran /usr/bin/gfortran-${GCC_V} \
--slave /usr/bin/gcov gcov /usr/bin/gcov-${GCC_V}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code coverage for non-default GCC (8 and 9) was failing before this change.

fail-fast: false
matrix:
plumed_v: [2.5.3, 2.6.2, 2.7.0]
gcc_v: [7]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the GGC_V variable was not specified, we weren't reusing the pFUnit cache.

Comment thread src/Makefile
# Used for building Unit Tests
libabin.a: compile_info.o
ar cru libabin.a ${C_OBJS} $(F_OBJS) compile_info.o && ranlib libabin.a
ar rcs libabin.a ${C_OBJS} $(F_OBJS) compile_info.o

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to man ranlib:

The GNU ranlib program is another form of GNU ar; running ranlib is completely equivalent to executing ar -s

@danielhollas danielhollas marked this pull request as ready for review January 20, 2021 20:28
Comment thread .github/workflows/gfortran.yml Outdated
FC: gfortran
FFLAGS: -O0 -fopenmp -Wall --coverage -ffpe-trap=invalid,zero,overflow,denormal
GCC_V: ${{ matrix.gcc_v}}
CODECOV_NAME: ${{format('{0} GCC{1}', github.job, matrix.gcc_v)}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to debug missing Codecov reports, otherwise I cannot distinguish in the Codecov UI which jobs uploaded reports successfully.

with:
path: ~/plumed/${{ env.PLUMED_V }}/install
key: ${{runner.os}}-plumed${{env.PLUMED_V}}-${{hashFiles('dev_scripts/install_plumed.sh')}}
key: ${{runner.os}}-plumed${{env.PLUMED_V}}-gcc${{ env.GCC_V }}-${{hashFiles('dev_scripts/install_plumed.sh')}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clear the cache in case we decide to change the GCC version in the future.

@danielhollas danielhollas force-pushed the separate-unittests branch 4 times, most recently from c52ec27 to 62145aa Compare January 28, 2021 16:30

# TODO: Separate unit test coverage
- name: Upload code coverage to Codecov
run: cd src && bash <(curl -s https://codecov.io/bash) $CODECOV_OPTIONS -U "$CURL_OPTS"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like gcov is working better when running directly on the src/ directory. Before this change some builds were sometimes missing or empty (specifically for GCC-8 and GCC-9 builds).

@danielhollas danielhollas requested a review from suchanj January 28, 2021 16:50
@danielhollas danielhollas changed the title Separate unit tests on GA Tweaks and fixes for GA and Codecov configs Jan 28, 2021
@danielhollas

Copy link
Copy Markdown
Contributor Author

@suchanj this is finally ready for review. 🎉

It took me a long time to stabilize the Codecov behavior, now it seems to do the trick, it seems that the BASH uploader needs to be from within the src/ directory.

The coverage changes are a bit puzzling, but I verified that all of them are correct, i.e. from the code that we do not currently test. I guess these lines were not picked up with gcov-7, and they showed up now that the coverage reports work when compiling with GCC8 and GCC9 (see comments inline).

- Codecov reports from with GFortran 8 and 9 were crashing
because we were not setting the correct 'gcov' version.

- Some caches were not used incorrectly.
  The GA config is getting a bit unwieldy. :-(

- GA: Pin Ubuntu version to 18.04:
  Github will soon switch it's 'ubuntu-latest'
  runner to 'ubuntu-20.04' and we're not ready:
  1. Some tests are failing with minor numerical differences on 20.04
     Hopefully this get's solved with #27.

  2. Some jobs in GFortran CI config now rely on the fact
     that the default GCC/GFortran version is 7. We'll need to make that explicit
     if we want to switch to 20.04. We want to keep the default version = 7
     since that's the one we're using on the PHOTOX clusters.

  3. Given our ancient Linux versions on PHOTOX clusters,
     it probably makes sense to stick to 18.04 for now.
     (and maybe in the future switch one job to 20.04)

- GA: Separate unit tests from end-to-end tests
  This is just for a convenience, to make it easier
  to inspect Github Action workflows. This will matter more
  as we'll add more unit tests. :-)
  This also makes the Makefile a bit more consistent;
  treating e2e and unit tests on equal footing.
FC: gfortran
FFLAGS: -O0 -fopenmp -Wall --coverage -ffpe-trap=invalid,zero,overflow,denormal
GCC_V: ${{ matrix.gcc_v}}
CODECOV_NAME: ${{format('{0} GCC-{1}', github.job, matrix.gcc_v)}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is useful when debuggin builds uploaded from codecov. Otherwise, they are not distiguishable. Here's how it looks now, e.g.

https://codecov.io/gh/PHOTOX/ABIN/commit/2860c44af6aad41e4730f4b9bc75bc126b4f1e36/build

image

@suchanj

suchanj commented Feb 1, 2021

Copy link
Copy Markdown
Contributor

Ok, it is getting harder to keep up :D, but I got the gist of it.

@danielhollas danielhollas merged commit 20acee4 into master Feb 1, 2021
@danielhollas danielhollas deleted the separate-unittests branch February 1, 2021 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Any changes to Github Actions or testing scripts.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants