#1312 ScalarArray code generation

[mo-alistairp]

Adding code generation for ScalarArrays (towards #1312 ). The metadata support was added in #2173.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 99.91%. Comparing base (97dd02d) to head (5d92498).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3101      +/-   ##
==========================================
- Coverage   99.91%   99.91%   -0.01%     
==========================================
  Files         376      377       +1     
  Lines       53529    53627      +98     
==========================================
+ Hits        53484    53581      +97     
- Misses         45       46       +1     

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

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mo-alistairp]

This still isn't working as intended, but is getting closer to correct. As an example, the code gen is now adding the scalar arrays to the generated code, alongside the arrays containing their dimensions. However, it is not adding these dimensions to the generated code (e.g., no dimensions(...) mention). It is also assigning the field as intent(in) right now, which it shouldn't do either.

Furthermore, the stub_gen test is failing because we are currently adding the dimensions array to the psyir layer as an ArgumentInteraface which isn't there as an argument at the call. Need to find a way around this. I'm wondering if we need a dimensions array at all but I'll have to investigate that further.

[mo-alistairp]

I'm unsure how to add the declarations correctly. The ScalarArray needs to have dimension(dims_array(1), dims_array(2), ... ). I've tried creating each of dims_array(n) as individual symbols which get added to the ScalarArray symbols as a reference. However, this doesn't seem to be added to the dimension correctly and adds the dims_array(n) symbols to the reference list.

Design-wise, I'm also unsure whether these ScalarArray declarations should exist in lfric_scalar_args.py or if they should have there own file, e.g., lfric_scalar_array_args.py.

[arporter]

I'm unsure how to add the declarations correctly. The ScalarArray needs to have dimension(dims_array(1), dims_array(2), ... ). I've tried creating each of dims_array(n) as individual symbols which get added to the ScalarArray symbols as a reference. However, this doesn't seem to be added to the dimension correctly and adds the dims_array(n) symbols to the reference list.

You'll need to create a Symbol for dims_array as an integer array that is an argument to the (kernel) subroutine. Then you create an ArrayType for the ScalarArray with dims = [ArrayReference.create(dims_array_sim, [Literal(1)]), ArrayReference.create(dims_array_sym, [Literal(2)])].

[mo-alistairp]

Ah I see, I wasn't aware you could create an array like that - I'll give that a go, thanks!

[mo-alistairp]

This is getting closer to being correct. Currently it's adding the dimension of dims_array wrong (e.g., as dimension(1,1,1,1) instead of dimension(4)). I've added it this way as a temporary fix - when I add dims_array with dimension(4) it errors as it expects array to be of the same rank as dims_array since the dimension symbol inherits from dims_array (I think!). Since I need array to have dimension(dims_array(1), ..., dims_array(4)), that is why it's currently not erroring with dimension(1,1,1,1) as they both have the same length. Not entirely sure how to get around that.

Another smaller problem is that the dimension of array is currently not being added to the declaration - this could be down to how the dimension of array is being added

[mo-alistairp]

I've managed to break some of the other tests too - I suspect this is because of some unintended changes to normal scalar values. I will investigate

[mo-alistairp]

This is definitely a step in the right direction. I have managed to add ArrayReferences to the dimensions array. The current code generation is creating the following -

integer(kind=i_def), intent(inout) :: integer_array integer(kind=i_def), dimension(4), intent(in) :: dims_integer_array integer(kind=i_def), dimension(dims_integer_array(1),dims_integer_array(2),dims_integer_array(3),dims_integer_array(4)), intent(in) :: integer_arraytemp

There are two problems -

• the _array symbol is being added before this code, seemingly not even as a scalar, rather than a ScalarArray. This is why I was previously having trouble getting it to display the dimensions - because it wasn't an array! Currently I am side-stepping this problem by creating a new symbol (ending with temp), but this needs an actual fix
• the dims_array symbol is producing an error as it is not a kernel argument at present, but has an ArgumentInterface - e.g., psyclone.errors.InternalError: PSyclone internal error: ("Symbol dims_rscalar_array_2: DataSymbol<Array<Scalar<INTEGER, Reference[name:'i_def']>, shape=[2]>, Argument(Access.READ)> is not listed as a kernel argument and yet has an ArgumentInterface interface.",).

I need to find out where the symbol is first added to the tree - I think doing so might allow me to fix both issues.

[mo-alistairp]

The current failure is a bit strange, I haven't touched testkern_type explicitly so I'm a bit confused why it's got the wrong number of arguments all of a sudden. I'll keep investigating - I could always reverse my changes which caused the error to see if that fixes it. Irrespectively, I think it makes sense to have a normal scalar value in the test to check that it's not being affected by ScalarArrays being there.

[mo-alistairp]

This is now generating code and the stub for my written tests. The code gen looks sensible, the stub gen needs more work. I haven't managed to fix the error which is prematurely failing the tests.

[mo-alistairp]

The body of the code gen and stub gen looks correct - the only thing to fix is the number of variables appearing in the call of the code gen and the subroutine definition of the stub generation

[mo-alistairp]

I think this is now ready for review. The code gen and stub gen seem to be performing as I would expect. I've added test coverage for everything I've added. Everything is passing the style checks.

[mo-alistairp]

I'm having a little trouble getting testkern_scalar_array_mod via lfric_scalar_codegen_test to compile. Currently, it's erroring with Error: Symbol 'gh_scalar_array' at (1) has no IMPLICIT type; did you mean 'gh_scalar'?

Still trying to find what needs adding to fix this.