(Closes #2499) scalarization transformation implementation

[LonelyCat124]

First implementation is here. I have unit tests for the private routines, I need to test it with code examples doing the full transformation properly but in theory its implemented here for now.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.89%. Comparing base (06e0257) to head (94f99eb).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2563   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files         359      360    +1     
  Lines       50997    51089   +92     
=======================================
+ Hits        50942    51034   +92     
  Misses         55       55           

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

[LonelyCat124]

@hiker Quick question on how the ordering of statements goes with the dependency system - if you have an assignment such as a = a + 1 does the a access on the RHS appear before the a on the LHS? I assume so (and that makes sense) but I just wanted to double check as I have a superfluous (and unreachable) if statement if so.

[LonelyCat124]

@sergisiso this is ready for a first look.
I didn't add anything to user or dev guide for now, let me know if you think I should.

I'm waiting for hiker to clarify my question before I remove the code at lines 123 in the transformation, but I expect to remove those lines as I think they're unreachable.

[sergisiso]

Good initial implementation @LonelyCat124, I am just asking if the code can be slightly refactored to be more readable and I am wondering if some logic should leave inside the next_access methods, I am happy to discuss this one.

[LonelyCat124]

@sergisiso I've addressed most of the comments here - I think the next_access one is a bigger issue than just here (and we need to decide if the variable access tooling should handle this or if we just do it in next_access, but I think we can do an implementation for this in the mean time anyway).

I need to still expand the tests for the apply routine and add documentation before another review though I think.

[sergisiso]

@LonelyCat124 I don't mind the order of these (if you put the associated TODOs), but wouldn't want to go very far with some logic that then will need to be deleted.

[LonelyCat124]

I brought this up to master now and made the tests pass, but I'll start reworking the logic here now.

[LonelyCat124]

Ok - integration tests find a couple of issues.

NEMO 5 fails due to an error I'm not sure inside the dependency code, where something isn't created correctly. I'll need to check the processed file to fix that.

NEMO 4 fails due to a failure to resolve a MAXVAL signature, which I need to test myself. I'm a bit confused why we get a signature for MAXVAL but I'll investigate.

[LonelyCat124]

Will run the integration tests again - only NEMO4 though for now.

[LonelyCat124]

Another failure on the integration tests for NEMO4. I've drafted a fix but I'll need to check the original subroutine to check it's fixed, and then create a test to cover it

[LonelyCat124]

Remaining issues seem to be a bug in DefinitionUseChains. I'm going to try to work this out - once I find it am I ok to fix it here or shall i create an issue and separate pull request @sergisiso ?

[LonelyCat124]

Probable bug: What happens if we find an element of a structure that we think can be scalarised (e.g. mystruct%array(i)) - we shouldn't scalarise? Or we scalarise only if we can work out the type of the element of the structure and then we need to come up with a name. @sergisiso what are your thoughts on how this should be handled?

[LonelyCat124]

NEMO passthrough gives the wrong result - I need to investigate.

[LonelyCat124]

NEMO4 now works, NEMO5 gets the wrong answer, I'll try to investigate next week