Remove SCC wrapper transformation #124
Conversation
This also includes minor cosmetics in the entry point logic that composes the trafos to be applied.
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/124/index.html |
Codecov Report
@@ Coverage Diff @@
## main #124 +/- ##
==========================================
- Coverage 92.15% 92.10% -0.06%
==========================================
Files 90 89 -1
Lines 16653 16540 -113
==========================================
- Hits 15347 15234 -113
Misses 1306 1306
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| transformation = None | ||
| if mode == 'idem': | ||
| transformation = IdemTransformation() | ||
|
|
||
| if mode == 'sca': | ||
| # Define the target dimension to strip from kernel and caller | ||
| horizontal = scheduler.config.dimensions['horizontal'] | ||
| transformation = ExtractSCATransformation(horizontal=horizontal) |
There was a problem hiding this comment.
Do we not need to wrap this in a tuple? Same for the idem trafo above.
There was a problem hiding this comment.
Good catch! I've added an as_tuple around the transformations in the application loop, which will have the same effect, but makes it a little safer.
| @@ -222,59 +222,6 @@ def test_scc_base_masked_statement(frontend, horizontal): | |||
| assert fgen(kernel_conds[0].else_body) == 'q(jl, jk) = t(jl, jk)' | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize('frontend', available_frontends()) | |||
There was a problem hiding this comment.
Just to make sure we're not loosing test coverage: The deleted tests were essentially redundant in the sense that they tested functionality with the wrapper that's already been checked on the invidividual relevant transformations?
There was a problem hiding this comment.
Yes, I believe so. @awnawab might be better placed to confirm this, but I've glanced over them and they looked fairly identical still.
There was a problem hiding this comment.
Yup, all the tests with 'wrapper' in the name were essentially redundant.
This PR does pretty much what it says on the tin: Remove the old SCC wrapper transformation and instead use the component-wise composition in the
ecphysentry point. In addition, it includes some slight cosmetics improvements and removes the redundant duplicated tests in thetransformationssub-package.I've tested this with the Cy48R1 ec-physics demonstrator and things are fine. Please note that I haven't enabled the vector-section trim or global var offloads yet, because I'm planning to consolidate this with the convert entry point soon, and possibly more these into the config file.