#1312 ScalarArray code generation

[mo-alistairp]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (60c3bf6) to head (bca6096).

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3101    +/-   ##
========================================
  Coverage   99.95%   99.95%            
========================================
  Files         379      380     +1     
  Lines       53811    53913   +102     
========================================
+ Hits        53789    53891   +102     
  Misses         22       22            

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

🚀 New features to boost your workflow:

• ❄️ 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.

[arporter]

With regards to the change in external/ - what is your process for getting something added there? I currently have a draft PR in MetOffice/lfric_core but I'm unsure if you need that to be committed first to get your integration tests to work

You can just update it as part of this PR. In order to get the integration tests to work, it will also need to be added to stfc/lfric_core. Ideally we'll remove this duplication and perhaps have a git submodule pointing to stfc/lfric_core but we're in a transition stage at the moment.

[mo-alistairp]

With regards to the change in external/ - what is your process for getting something added there? I currently have a draft PR in MetOffice/lfric_core but I'm unsure if you need that to be committed first to get your integration tests to work

You can just update it as part of this PR. In order to get the integration tests to work, it will also need to be added to stfc/lfric_core. Ideally we'll remove this duplication and perhaps have a git submodule pointing to stfc/lfric_core but we're in a transition stage at the moment.

Ok, sure - thanks. Is it sufficient then if I wait until this is merged, then go through the merge process of the MetOffice/lfric_core PR so you can just update your stfc fork to main? Or does the stfc change need to be done sooner, e.g., by me opening another PR in stfc/lfric_core?

I think I misread your comment before, so you need me to open a PR in stfc/lfric_core adding GH_SCALAR_ARRAY into argument_mod?

[mo-alistairp]

With regards to the change in external/ - what is your process for getting something added there? I currently have a draft PR in MetOffice/lfric_core but I'm unsure if you need that to be committed first to get your integration tests to work

You can just update it as part of this PR. In order to get the integration tests to work, it will also need to be added to stfc/lfric_core. Ideally we'll remove this duplication and perhaps have a git submodule pointing to stfc/lfric_core but we're in a transition stage at the moment.

Ok, sure - thanks. Is it sufficient then if I wait until this is merged, then go through the merge process of the MetOffice/lfric_core PR so you can just update your stfc fork to main? Or does the stfc change need to be done sooner, e.g., by me opening another PR in stfc/lfric_core?

I think I misread your comment before, so you need me to open a PR in stfc/lfric_core adding GH_SCALAR_ARRAY into argument_mod?

I just tried to do this and I don't currently have permission to push to stfc/lfric_core

remote: Permission to stfc/lfric_core.git denied to mo-alistairp

[arporter]

I just tried to do this and I don't currently have permission to push to stfc/lfric_core

Ah! You should have an invitation now. @sergisiso and @LonelyCat124 should we protect master in that repo and enforce PRs?

[LonelyCat124]

I just tried to do this and I don't currently have permission to push to stfc/lfric_core

Ah! You should have an invitation now. @sergisiso and @LonelyCat124 should we protect master in that repo and enforce PRs?

Imo yes I think so, using our usual controls would be best since we want to link (loosely) PRs there to PRs in PSyclone anyway

[mo-alistairp]

I've made a PR for the changes to stfc/psyclone at stfc/lfric_core#2 and requested a review

[arporter]

Good work - it's getting very close now.
See comments inline.
I'll run the ITs once stfc/lfric_core is updated.

[mo-alistairp]

Thanks for your comments, I've addressed them and it is now passing CodeCov too. It's now ready for review again

[arporter]

Almost! The modified test needs a bit more work.
I've taken the liberty of updating the lfric_apps hash in the workflow file and have fired off the integration tests.

[arporter]

You've already got a const object but if this is the only thing then I'll do it.

[arporter]

Actually, you also need to update the name and docstring of the test (now that it does both stub and normal).

[arporter]

Two of the integration tests failed but one seems to be #3297 and the other is because stfc/lfric_apps#1 hasn't been merged yet. i.e. neither failure is the fault of this PR.