Repository navigation
Unroll array initialisers - #712
Conversation
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/712/index.html |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #712 +/- ##
==========================================
+ Coverage 94.19% 94.21% +0.01%
==========================================
Files 291 291
Lines 48719 48774 +55
==========================================
+ Hits 45893 45950 +57
+ Misses 2826 2824 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2be4783 to
01abdf0
Compare
01abdf0 to
f5d9059
Compare
| if resolved: | ||
| return resolved | ||
| scope_str = f' in routine "{self.scope.name}"' if self.scope is not None else '' | ||
| warning( |
There was a problem hiding this comment.
This warning is not really up-to-date anymore. It will get here either because the RHS is a mixed expression or because the LHS array has bar :. I'd move the warning inside _resolve_literal_list
MichaelSt98
left a comment
MichaelSt98
left a comment
There was a problem hiding this comment.
There are more funky Literal lists in Fortran ...
For example:
arr(1:3) = [(i, i=1,3)]gets transformed to:
arr(1) = ( i, i = 1,3 )which is clearly wrong.
Especially those literal lists/ array initialisers are dangerous regarding performance...
literallist resolution
containing anything other than scalars
MichaelSt98
left a comment
MichaelSt98
left a comment
There was a problem hiding this comment.
Looks great!
The only thing we could think of is making this optional.
mlange05
left a comment
mlange05
left a comment
There was a problem hiding this comment.
Looks good to me - only picked on a minor cosmetic nitpick (optional). Otherwise GTG and very neatly implemented. 👍 
| if l.variable.name.startswith('i_'): | ||
| generated_loop_bodies.extend(FindNodes(ir.Assignment).visit(l.body)) | ||
| literal_assigns = [a for a in assigns | ||
| if hasattr(a.rhs, 'elements') or 'LiteralList' in type(a.rhs).__name__] |
There was a problem hiding this comment.
Why not isinstance(a.rhs, sym.LiteralList)? This is a very odd way of checking type, no?
There was a problem hiding this comment.
That was an LLM-ism I didn't catch, fixed now. Thanks!
|
The latest release of fastjsonschema introduced a bug (see horejsek/python-fastjsonschema#213) which affects python < 3.10 builds, so I've also updated loki's EDIT: a patched version of the package was released, so I've removed the workaround commit. |
eb83512 to
da4352c
Compare
mlange05
added
the
ready to merge
This PR updates the
ResolveVectorNotationTransformationso that array initialisers are unrolled.Some array initialisation statements proved to be very costly in the GPU runs on jupiter. It's hard to determine a priori which ones will bite, so we unroll simple 1D constant bound array initialiasation statements automatically with this PR. A warning is printed for more complex unresolvable ones so that they are easily identifiable and can be addressed in source if necessary.