stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 29 forks source link

(closes #717) Remove unused code #2725

Closed sergisiso closed 1 month ago

sergisiso commented 1 month ago

717 was the same than #2687 but there were some TODOs associated to it. Looking at them it turns out that they where related to the array shape querying ulility functionality in fparser2, but these are now unused (other than in its own tests) as we use the ArrayMixin utility methods.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.86%. Comparing base (6aad1d4) to head (99a1c1b). Report is 10 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2725 +/- ## ========================================== - Coverage 99.86% 99.86% -0.01% ========================================== Files 354 354 Lines 49112 48893 -219 ========================================== - Hits 49048 48829 -219 Misses 64 64 ```

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

sergisiso commented 1 month ago

This is ready for review. Initially I just wanted to delete some unused test, but then realised that the WHERE fix also solves the last passthrough file in NEMOv4 🥳 and it has a TODO to this issue.

After removing it from the excluded file another issue hit that file: the fact that we cannot distinguish between imported calls with arguments and arrays. This was somewhat fixed in ACCKernels by listing all the NEMO FUNCTIONS, so I generalised that into utils so it can be used by all transformations.

It passed the integration tests with expected performance.

One for @arporter (since it touches the NEMO ACC Kernels) or @LonelyCat124

sergisiso commented 1 month ago

@arporter This is ready for another review

arporter commented 1 month ago

Integration tests were green.