ecmwf-ifs / loki

Freely programmable source-to-source translation for Fortran
https://sites.ecmwf.int/docs/loki/
Apache License 2.0
29 stars 12 forks source link

JIT compilation updates and compatibility with f90wrap v0.2.14 #315

Closed reuterbal closed 4 months ago

reuterbal commented 4 months ago

The latest release of f90wrap includes module names in the name of generated interface routines (introduced in https://github.com/jameskermode/f90wrap/pull/215). This lets them grow quickly beyond the Fortran length limit of 63 characters for names, and gfortran (not even 13.2) does not allow to lift that limitation.

Therefore, I have shorted the module and routine names in the test base where the character limit was exceeded. In the process, I also introduced the use of tempdir fixtures more widely to ensure clean-up of test files is handled gracefully and the littering of the source directory is reduced.

github-actions[bot] commented 4 months ago

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/315/index.html

reuterbal commented 4 months ago

This turned into quite a rabbit hole. Something in the original set of changes seems to have slightly skewed timings in the test execution, presumably due to the increased use of test-local temporary directories. The symptom was then random segfaults when garbage collection was triggered.

The subsequent investigation unearthed a few glaring holes in our JIT infrastructure for tests:

I'm afraid, the end result is now a fairly sizeable diff. Sorry!

codecov[bot] commented 4 months ago

Codecov Report

Attention: Patch coverage is 98.65229% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 95.11%. Comparing base (5db17e5) to head (b4decea).

Files Patch % Lines
loki/expression/tests/test_expression.py 33.33% 2 Missing :warning:
loki/transformations/transpile/tests/test_sdfg.py 91.30% 2 Missing :warning:
loki/transformations/tests/test_parametrise.py 96.15% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #315 +/- ## ========================================== + Coverage 95.09% 95.11% +0.01% ========================================== Files 165 165 Lines 35301 35065 -236 ========================================== - Hits 33571 33351 -220 + Misses 1730 1714 -16 ``` | [Flag](https://app.codecov.io/gh/ecmwf-ifs/loki/pull/315/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf-ifs) | Coverage Δ | | |---|---|---| | [lint_rules](https://app.codecov.io/gh/ecmwf-ifs/loki/pull/315/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf-ifs) | `96.39% <ø> (ø)` | | | [loki](https://app.codecov.io/gh/ecmwf-ifs/loki/pull/315/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf-ifs) | `95.08% <98.65%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ecmwf-ifs#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.