Add support for assumed size declarations to Fparser #486
Conversation
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/486/index.html |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #486 +/- ##
========================================
Coverage 96.17% 96.18%
========================================
Files 224 224
Lines 40386 40489 +103
========================================
+ Hits 38842 38945 +103
Misses 1544 1544
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
awnawab
force-pushed
the
naan-assumed-size
branch
from
92625cc to
c19139b
Compare
awnawab
force-pushed
the
naan-assumed-size
branch
from
c19139b to
bcae761
Compare
awnawab
requested review from
mlange05 and
reuterbal
and removed request for
mlange05
| implicit none | ||
| real, intent(in) :: a(*) | ||
| real, intent(in) :: b(8,*) | ||
| real, intent(in) :: c(8,0:*) |
There was a problem hiding this comment.
Would you mind adding a d(2,4,3:*) to demonstrate that non-zero lower bounds and multiple preceeding explicit-shape-specs are working as well? It looks to me like the implementation would handle this correctly but worth double-checking.
| @@ -28,6 +29,7 @@ | |||
| ) | |||
| from loki.tools import as_tuple, CaseInsensitiveDict | |||
| from loki.types import BasicType | |||
| from loki.expression import symbols as sym, simplify | |||
There was a problem hiding this comment.
Please add this to the existing import from loki.expression above
| @@ -80,6 +82,35 @@ def transform_subroutine(self, routine, **kwargs): # pylint: disable=arguments- | |||
| if d == ':'] | |||
| vmap[arg] = arg.clone(type=arg.type.clone(shape=new_shape)) | |||
|
|
|||
| elif re.search(r'\*$', str(arg.shape[-1])): | |||
There was a problem hiding this comment.
Do we really need a regex here? Wouldn't the following work?
| elif re.search(r'\*$', str(arg.shape[-1])): | |
| elif str(arg.shape[-1])[-1] == '*': |
There was a problem hiding this comment.
Ah yes that is much cleaner, thanks!
| @@ -80,6 +82,35 @@ def transform_subroutine(self, routine, **kwargs): # pylint: disable=arguments- | |||
| if d == ':'] | |||
| vmap[arg] = arg.clone(type=arg.type.clone(shape=new_shape)) | |||
|
|
|||
| elif re.search(r'\*$', str(arg.shape[-1])): | |||
| # determine extent of explicit shape part of assumed size declaration | |||
| expl_shape_len = [loc for loc, dim in enumerate(arg.shape) if '*' in dim][0] | |||
There was a problem hiding this comment.
The standard explicitly mandates expl_shape_len = len(arg.shape)-1, can't we use that knowledge here?
The rank is equal to one plus the number of explicit-shape-specs.
https://j3-fortran.org/doc/year/10/10-007.pdf#page=114
There was a problem hiding this comment.
Yes great catch, thanks!
| @@ -119,13 +150,15 @@ class ExplicitArgumentArrayShapeTransformation(Transformation): | |||
|
|
|||
| def transform_subroutine(self, routine, **kwargs): # pylint: disable=arguments-differ | |||
|
|
|||
| def assumed(dims): | |||
| return all(d == ':' for d in dims) or re.search(r'\*$', str(dims[-1])) | |||
There was a problem hiding this comment.
Same comment about the need for a regex
This PR adds support for assumed size declarations (😑 ) for Fparser, and also adds the mechanics to the
ArgumentShapeAnalysisto resolve the corresponding explicit shape.