stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 28 forks source link

different looping limits for different kernels #42

Closed stephenpring closed 3 years ago

stephenpring commented 7 years ago

LFRic ticket https://code.metoffice.gov.uk/trac/lfric/ticket/1058 changes the looping limits for four different kernels.

Some of the kernels loop all over all core and halo cells, or just the core cells or all core and some of the halo cells to a certain depth which is determined by an input parameter.

This ticket is to allow PSyclone to be aware of this issue.

arporter commented 7 years ago

If this is to be a general requirement then the first step is to decide upon what additional kernel meta-data is necessary in order to allow PSyclone to identify those kernels which require loop bounds other than the default. Is this functionality linked in any way to existing kernel meta-data (e.g. stencils)?

rupertford commented 7 years ago

I could be wrong but I suspect this is an example of manually applying redundant computation optimisations to psykal-lite. I think psykal-lite used to compute in level 1 halos by default and it sounds like Stephen has been playing with this. If he has got code with different stencil sizes then he might be getting issues with setting halo's clean when redundant computation is performed.

In PSyclone we do not compute in halo's by default and currently have no transformation to do so. Therefore we end up having halo swaps. We get correct results but in some circumstances there can be a trade off between redundant computation and communication.

Therefore I hope this is not an issue for us.

As a side point I'm not sure how well PSyclone deals with halo's of different depth at the moment.

stephenpring commented 7 years ago

Hi,

Just to give a bit more info. The algorithm is a bit different to the finite element based code and is used to transport stuff on the mesh. What I have at the moment is the halo depth that I use in my code is determined by the sum of dep_pt_stencil_extent and rho_approximation_stencil_extent. If we just call them x and y then halo_depth = x+y.

In the algorithm, one of the looping calls is to loop to a halo depth of x and a stencil of extent y is generated each time. The needs to loop to a depth x and I'm concerned that if it goes any deeper then stencil arrays will go out of bounds as I'm making them length y. I hope that makes sense. It is preventing redundant computations but also more I think preventing stencil arrays going out of bounds.

arporter commented 5 years ago

Hi @stephenpring, is this Issue still pertinent or have things moved on?

stephenpring commented 5 years ago

Hi @arporter yes, this is still relevant. There's quite a bit of code within lfric's psykal_lite_mod which uses different looping limits. Some of the kernels need to loop over just the core cells and some kernels loop over the core and halo cells. Mike Hobson is probably the best LFRic person to talk to as he's familiar with the code.

arporter commented 5 years ago

We discussed this at the LFRic telco on 29/01/2019. Mike Hobson tells me that this should just be an issue for the LFRic infrastructure but we'll keep this issue open in case any PSyclone development is required.

stevemullerworth commented 3 years ago

At some point we are going to need to support Cosmic fields and new issues will be raised. But this particular issue requests a solution that doesn't feel right. Current development is using a different type of LFRic stencil. The issue is no longer referenced in existing psykal-lite code, so I suggest closing it.

rupertford commented 3 years ago

Closing issue as suggested by Lord Mullerworth.