Run unit tests with OpenMP in CI

[dabele]

Changes and Information

Add a job to the main CI pipeline that runs the unit tests with OpenMP enabled.

simplified build artifact names in the CI pipeline, i.e. leave out default values.

The job runs with 4 threads. Github runners have 2 CPUs, I wasn't able to find information on Hyperthreading, I used 4 just in case. This is not very high parallelization, we may want to add a custom runner (maybe the HPDA cluster or something) to increase the chance of finding bugs.

The unit tests are repeated 10 times, this was enough to reliably catch the segfault in #857, which happens ~50% of the time. That bug is quite easy to reproduce, we may want to add more repetitions in the future to catch rarer bugs.

So far I didn't add new unit tests just for OpenMP, the current tests catch race conditions in the currently parallelized parts. I don't know if there is a way to ensure 100% coverage of possible current and future race conditions. I think good general test coverage and maybe running testing loops a few more times than strictly necessary (e.g. do more simulation steps) is as good as it gets.

Multithreading issues are usually random, so it's unclear what the failure will be. On my local machine, one test went into an infinite loop sometimes (due to the bug in #857), so I added a timeout to the test. Otherwise the test would keep spinning for 6 hours (the github limit). The tests might also just work as expected even if there is a race condition. To increase the chance of catching a race condition, I added a reminder to the pull request template to run unit tests with OpenMP locally.

Closes #786

Merge Request - Guideline Checklist

Please check our git workflow. Use the draft feature if the Pull Request is not yet ready to review.

Checks by code author

• Every addressed issue is linked (use the "Closes #ISSUE" keyword below)
• New code adheres to coding guidelines
• No large data files have been added (files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)
• Tests are added for new functionality and a local test run was successful
• Appropriate documentation for new functionality has been added (Doxygen in the code and Markdown files if necessary)
• Proper attention to licenses, especially no new third-party software with conflicting license has been added
• (For ABM development) Checked benchmark results

Checks by code reviewer(s)

• Corresponding issue(s) is/are linked and addressed
• Code is clean of development artifacts (no deactivated or commented code lines, no debugging printouts, etc.)
• Appropriate unit tests have been added, CI passes and code coverage is acceptable (did not decrease)
• No large data files added in the whole history of commits(files should in sum not exceed 100 KB, avoid PDFs, Word docs, etc.)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0368ad2) 95.46% compared to head (f8a37a6) 95.46%.
Report is 18 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #870   +/-   ##
=======================================
  Coverage   95.46%   95.46%           
=======================================
  Files         118      118           
  Lines        9289     9289           
=======================================
  Hits         8868     8868           
  Misses        421      421           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dabele]

merged main branch, openmp unittest is passing now as expected as the race condition has been fixed in #830

[xsaschako]

good changes, looks good
actually i think github upgraded the runners to be 4 cored for public repositories so you may want to change it to 8 , see https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners

[dabele]

good changes, looks good actually i think github upgraded the runners to be 4 cored for public repositories so you may want to change it to 8 , see https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners

Since we already have 4 threads enabled and don't know about hyperthreading this should be fine for now, 4 are probably enough to catch issues.