Transformations: Move common SCC utility routines to utilities

[mlange05]

Another small housekeeping change. This one adds a new sub-package transformations.single_column.util sub-package for common utilities that may be used across the several SCC sub-stages.

A small housekeeping change that moves utility methods from SCCBaseTransformation to the the common transformations.utilities subpackage. This primarily removes the awkward dependency on the SCCBaseTransformation class in other SCC sub-components, which provided this previously.

This change is largely cosmetic (simply copying methods over), except for the get_local_arrays utility, which is now used by the demotion and the annotations sub-component. This is in preparation for a more complex refactoring of the annotation sub-component to support multiple vectorisation and annotation schemes in a future PR.

[github-actions]

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.36%. Comparing base (b7ba5fc) to head (3226c32).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #354   +/-   ##
=======================================
  Coverage   95.35%   95.36%           
=======================================
  Files         173      173           
  Lines       36835    36896   +61     
=======================================
+ Hits        35124    35185   +61     
  Misses       1711     1711           

Flag	Coverage Δ
lint_rules	96.39% <ø> (ø)
loki	95.34% <100.00%> (+<0.01%)	⬆️

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

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

[reuterbal]

Thanks, this looks great and removes the indeed awkward cross-referencing to utilities in the Base class, even from outside the SCC transformations.

We should consider creating dedicated tests for these utilities at some point - not mandating this for the PR but maybe create an issue to track this?

Finally, I have paid a little more attention to the new get_local_arrays utility and noticed that we might have a case-sensitivity problem there. Would be good to resolve that (or prove me wrong) before merging.

[reuterbal]

We could use routine._dummies here as a cheaper alternative because routine.arguments

1. builds routine.symbol_map
2. runs through routine._dummies to pick out the entries from the symbol_map in the right order

...and then we extract only the names again here

[mlange05]

Yes

[reuterbal]

This looks like a potential case-sensitivity issue: v.name is a str, therefore we don't have the benefit of the case-agnostic comparison anymore - and if argument declaration doesn't match the casing of an array use, we might run into issues here.

[mlange05]

Well spotted! I pushed a fix and tests for this now.

[mlange05]

@reuterbal You are, of course, right about the testing. I've added at least rudimentary test coverage for each utility routine. As part of that I spotted an annoying inconsistency, that I also fixed: get_loop_bounds would simply return strings and leave the symbol resolution to the upstream call. I've now moved this, which makes testing nicer and removes redundancy. So, please take another look.

[reuterbal]

Thanks, the changes look fantastic and also very much agreed on the change to the return type of get_loop_bounds! Much appreciate the additional testing!

Unless CI throws up some issues this is good to go!