about lfric_apps psy layer and stencils

[Alan J Hewitt (alanjhewitt)]

Andy Porter has developed some new functionality to allow psyclone to work out the psy layer stuff for kernels with stencil support. Previously, these have required additional developments in psykal_lite_physics_mod

stfc/PSyclone#3572

I have been testing this development with an lfric_apps branch, where I have removed all of the relevant psykal_lite code and have modified the kernel and algorithm so that they should work together.

MetOffice/lfric_apps#750

With a lot of help from Ben West, I managed to get my testing branch and Andy Porters psyclone branch working together with the command line build.

I ran into the problem below, where the number of arguments to the jules_exp_kernel_type has increased from 108 to 113.
There are five stencil fields being passed, so it is at least consistent with this.

15:57:53 PSyclone - global optimisation /data/users/alan.j.hewitt/branches/Github/lfric_apps/applications/lfric_atm/working/build_lfric_atm/algorithm/rad_tile_alg_mod.x90
Generation Error: error: expected '113' arguments for the call to kernel 'jules_exp_kernel_type' from invoke 'invoke_2_jules_exp_kernel_type' in the algorithm layer but found '108'. Expected '108' standard arguments, '5' stencil arguments, '0' qr_arguments and '0' halo-depth arguments.
Transformed invoke 'invoke_0' ...
Found 0 ['setval_c'] calls
InvokeSchedule[invoke='invoke_0', dm=True]
0: Loop[type='dof', field_space='any_space_1', it_space='dof', upper_bound='nannexed']

The ten missing arguments from the algorithm layer are:
    u_w3_stencil / v_w3_stencil / tile_stencil / sea_u_w3_stencil / sea_v_w3_stencil ( maybe analogous to the map_ arguments )
    u_w3_stencil_size / v_w3_stencil_size / tile_stencil_size / sea_u_w3_stencil_size / sea_v_w3_stencil_size

1. Because these were not previously present in the algorithm layer, I was wondering if these fields should not be counted as expected arguments (i.e. a further change to the psyclone branch is required).
2. Alternatively, I do need to add the five extra arguments to the algorithm layer (but I need help figuring this out).

I dont really understand what is happening in psyclone to generate the extra arguments that appear in a kernel layer that are not passed from the algorithm layer, but its not usually a problem as for most algorithm/kernel pairs I understand the pattern. Its probably time for me to take a deeper dive into what is happening in the psy layer.

Sorry for an even wordier question than even I usually write.

Kind regards,

Alan

[Alan J Hewitt (alanjhewitt)]

Adding Andy Porter (@arporter)

[Andy Porter (arporter)]

Thanks Alan J Hewitt (@alanjhewitt). I suggest you take a look at:

https://psyclone.readthedocs.io/en/latest/user_guide/lfric.html#stencil-extent

and check that the necessary arguments are being provided in the Algorithm layer. I'll have a more detailed look after lunch...

[Andy Porter (arporter)]

Note that the PSyclone complaint is about the number of arguments provided to the kernel in the invoke(...) call - it's confusing but you have to remember that this is not the same as the number of arguments that the kernel subroutine itself accepts.

[iboutle]

There are multiple things that probably need changing to get this right:

• the stencil arguments - I believe these are right in the kernel metadata and kernel argument list, but the algorithm argument list has not be kept up-to-date with previous Psyclone changes - namely now the "stencil_depth" argument must appear after each argument with a stencil, rather than just once at the end of the argument list as it does currently.
• The number of cells in the local domain:
  • For a normal domain kernel, seg_len is automatically appended as the 2nd argument in the kernel argument list, but does not need to appear in the algorithm argument list, so you can remove ncells from that
  • The kernel also requires seg_len_halo, i.e. the number of points including those in the stencil halo being passed in. I'm not sure in how Andy Porter (@arporter) has resolved the stencil/domain combination whether this will also get automatically appended to the kernel argument list (and therefore is also not needed in the algorithm), or whether it will need passing through from the algorithm as a scalar input to the kernel (in which case kernel metadata will need adding for it)

[Andy Porter (arporter)]

• The kernel also requires seg_len_halo, i.e. the number of points including those in the stencil halo being passed in. I'm not sure in how Andy Porter (Andy Porter (@arporter)) has resolved the stencil/domain combination whether this will also get automatically appended to the kernel argument list (and therefore is also not needed in the algorithm), or whether it will need passing through from the algorithm as a scalar input to the kernel (in which case kernel metadata will need adding for it)

Thanks iboutle. This is a complication I hadn't expected. Why does this kernel need to know about the halo?

[iboutle]

So it's needed in 2 places:

• Firstly to dimension the stencil (dofmap) arrays, e.g. integer(kind=i_def), dimension(ndf_w3,maxval(u_w3_stencil_size),seg_len_halo), intent(in) :: u_w3_stencil, i.e. these contain the number of points in the domain plus the number of points in the stencil around the domain
• Secondly, I need it locally in the kernel for looping into the stencil points, i.e. I need to loop over all points in the domain, plus those in the stencil around it

[Alan J Hewitt (alanjhewitt)]

Hi iboutle and Andy Porter (@arporter)

I got further through the build after adding the extend arguments imediately after each stencil argument.
MetOffice/lfric_apps@a6e0082

Rather oddly, it now gives an error message that suggests that mid-build it has reverted back to the default version of psyclone instead of the branch version, as this message should not show up from Andy's psyclone branch

       0: CodedKern process_urban_code(urbwrr,urbhwr,urbhgt,urbztm,urbdisp,urbemisw,urbemisr,urbemisc) [module_inline=False]

14:56:47 PSyclone - global optimisation /data/users/alan.j.hewitt/branches/Github/lfric_apps/applications/lfric_atm/working/build_lfric_atm/algorithm/jules_timestep_alg_mod.x90
Parse Error: In the LFRic API, kernels that operate on the domain are not permitted to have arguments with a stencil access but found: 'arg_type(gh_field, gh_real, gh_read, w3, stencil(region))'
Transformed invoke 'invoke_0' ...
Found 6 ['setval_c'] calls
InvokeSchedule[invoke='invoke_0', dm=True]
0: Loop[type='dof', field_space='any_space_1', it_space='dof', upper_bound='dof_halo(Literal[value:'1', Scalar<INTEGER, UNDEFINED>])']

Note that the error is no longer complaining about 'jules_exp_kernel_type'

I will check with Benjamin Went (@MetBenjaminWent) if such behaviour is possible and if so how i could prevent it.

[Alan J Hewitt (alanjhewitt)]

ok, this is because i had another copy of psyclone checked out in my home directory from a totally different debugging problem in July. now deleted.

[alan.j.hewitt@cazldf00000H PSyclone]$ which psyclone
~/.local/bin/psyclone

[Andy Porter (arporter)]

Thanks iboutle. The rule is that a kernel should not know/care about halos (while acknowledging that PSyclone still has explicit support for such things that was added in haste and will be removed in future). Obviously, a stencil will result in reads from the halo but the necessary information should be in the stencil dofmaps of those cells on the edge of the domain. Therefore, I would have expected the stencil dofmaps to be dimensioned by the number of owned cells only. Can the kernel be made to work in this way?

[iboutle]

I don't think so... the kernel is an interface to external code (Jules), so all it's really doing is copying the LFRic data into the arrays which are used by Jules. For the arrays which contain halo information, these need to be dimensioned with the total number of points required (domain+halo) and then have all of this data copied into them from the LFRic arrays (which is just done as a loop over the number of points in domain+halo).

[Andy Porter (arporter)]

OK. This looks like being a bigger Issue then. Bringing Hacka Fett (@christophermaynard) into this conversation as we need to decide what to do about this.

[iboutle]

I think for the purposes of Alan J Hewitt (@alanjhewitt)'s testing, this means that "seg_len" is the only argument (along with nlayers) that will be auto-populated at the top of the kernel argument list, and he will need to add "seg_len_halo" as a scalar integer argument to the kernel metadata and algorithm argument list (ncells_halo) to get it working.

[Alan J Hewitt (alanjhewitt)]

Good news. I have successfully built and run my lfric_apps branch with Andy's pscylone branch at revision 7c9f78f (aka September 1st commit).

I took the configuration.nml file from task run_lfric_atm_scm_gal9_dice2-BiP2x2-50000x50000_azspice_gnu_fast-debug-32bit and put it here applications/lfric_atm/example/configuration.nml

I also made a new directory spec in the same location and copied the following six files from applications/lfric_atm/working/scratch/socrates/data/spectra/ga9 there:
mcica_data sp_lw_cloud9 sp_lw_ga9 sp_lw_ga9_k sp_sw_cloud9 sp_sw_ga9 sp_sw_ga9_k

I ran into problems with the build step - mistakes i made with the environment setup. however with help from Benjamin Went (@MetBenjaminWent) I was able to get this to build successfully.

The run step worked once I had local copies of the above files in place.

[Alan J Hewitt (alanjhewitt)]

Closing this discussion.

Future developers could get in touch if they would like to repeat my hacky method of testing a psyclone branch with an lfric_apps branch.
I have also made notes in this (not for review) pull request.
MetOffice/lfric_apps#750

Thanks for help everyone.

Alan