extend hoist variables functionality

[MichaelSt98]

to account for possibly uninitialised variables used as dimensions (if wanted via flag)

This

subroutine driver(NLON, NB, FIELD1)
    use kernel_mod, only: kernel
    implicit none
    INTEGER, INTENT(IN) :: NLON, NB
    integer :: b
    integer, intent(inout) :: field1(nlon, nb)
    integer :: local_nlon
    local_nlon = nlon
    do b=1,nb
        call KERNEL(local_nlon, field1(:,b))
    end do
end subroutine driver

would be transformed to

SUBROUTINE driver (NLON, NB, field1)
  USE kernel_mod, ONLY: kernel
  implicit none
  INTEGER, INTENT(IN) :: NLON, NB
  INTEGER :: b
  INTEGER, INTENT(INOUT) :: field1(nlon, nb)
  INTEGER :: local_nlon
  INTEGER :: kernel_tmp1(local_nlon)
  local_nlon = nlon
  DO b=1,nb
    CALL KERNEL(local_nlon, field1(:, b), kernel_tmp1)
  END DO
END SUBROUTINE driver

which would be a problem as local_nlon is not initialised.

Setting remap_dimensions=True the transformation will produce

SUBROUTINE driver (NLON, NB, field1)
  USE kernel_mod, ONLY: kernel
  implicit none
  INTEGER, INTENT(IN) :: NLON, NB
  INTEGER :: b
  INTEGER, INTENT(INOUT) :: field1(nlon, nb)
  INTEGER :: local_nlon
  INTEGER :: kernel_tmp1(nlon)
  local_nlon = nlon
  DO b=1,nb
    CALL KERNEL(local_nlon, field1(:, b), kernel_tmp1)
  END DO
END SUBROUTINE driver

[github-actions]

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/357/index.html

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.40%. Comparing base (1f87624) to head (11cffba).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #357   +/-   ##
=======================================
  Coverage   95.40%   95.40%           
=======================================
  Files         178      178           
  Lines       37267    37293   +26     
=======================================
+ Hits        35555    35581   +26     
  Misses       1712     1712           

Flag	Coverage Δ
lint_rules	96.39% <ø> (ø)
loki	95.39% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[reuterbal]

Thanks, this looks great! I left two remarks, please have a look if that makes sense or point out what I'm missing there. Otherwise this is fantastic.

[reuterbal]

Just to point out how to do this in a "single line", or single list comprehension at least. For larger kernels like CLOUDSC with hundreds of assignments I expect you might actually be able to measure a small advantage with below change.

Suggested change

	dim_map = {}
	assignments = FindNodes(Assignment).visit(routine.body)
	for assignment in assignments:
	dim_map[assignment.lhs] = assignment.rhs
	dim_map = {
	assignment.lhs: assignment.rhs
	for assignment in FindNodes(Assignment).visit(routine.body)
	}

Also, this map might become quite big, so we might want to build a list of candidates first and then create the mapping? Suggestion (untested) would be something like this:

Suggested change

	dim_map = {}
	assignments = FindNodes(Assignment).visit(routine.body)
	for assignment in assignments:
	dim_map[assignment.lhs] = assignment.rhs
	dim_vars = [
	dim_var
	for var in variables
	for dim_var in FindVariables().visit(var.dimensions)
	if isinstance(var, sym.Array)
	]
	dim_map = {
	assignment.lhs: assignment.rhs
	for assignment in FindNodes(Assignment).visit(routine.body)
	if assignment.lhs in dim_vars
	}

[reuterbal]

Is there a situation where we wouldn't want to do this? It seems to me like we might not need to make this an option and instead always do this.

[MichaelSt98]

Good question ... I can't think of a situation we don't want to do that but on the other hand I think having a switch is not a bad idea?!
Anyway, I switched the default but if you insist we can remove this flag.

[reuterbal]

No, flipping the default should be enough. Thanks!

[reuterbal]

Thanks, looks great and gtg from me!
I'll rebase the branch and will merge once green

[reuterbal]

No, flipping the default should be enough. Thanks!

[mlange05]

Looks good to me; clean and nicely tested. I left a general comment for later development, but no action needed and GTG from me.

[mlange05]

[no action]: Just a general note: we keep hitting this remapping problem in many use cases - so eventually we might want a general utility that can do this. But for this use case this looks great.