Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kernels looping over halo cells and stencils #2781

Open
sergisiso opened this issue Nov 14, 2024 · 4 comments
Open

Kernels looping over halo cells and stencils #2781

sergisiso opened this issue Nov 14, 2024 · 4 comments
Assignees
Labels
LFRic Issue relates to the LFRic domain

Comments

@sergisiso
Copy link
Collaborator

sergisiso commented Nov 14, 2024

there is an issue that I hadn't anticipated with removing two of the extended mesh PSyKAl-lite kernels.

Both of those kernels loop over halo cells, and also use stencils. Given the kernel metadata, PSyclone produces the correct psy-layer, specifying that fields to be read by the stencil need to have a halo exchange to a depth of halo_depth + stencil_depth, where halo_depth is the depth of cells to loop the kernel over.

However these two kernels seem to only use parts of the stencil that are parallel to the halos, and not perpendicular -- thus in the PSyKAl-lite implementations, the halo exchange is only to a depth of halo_depth. We therefore trigger errors when trying to perform a halo exchange up to a depth of halo_depth + stencil_depth

Originally posted by @tommbendall in #2745 (comment)

@tommbendall tommbendall added the LFRic Issue relates to the LFRic domain label Nov 14, 2024
@arporter
Copy link
Member

@tommbendall when you say it triggers errors: is that because we're asking for too deep a halo? Should we do MAX(halo_depth, stencil_depth) or would that just be a sticking plaster?

@arporter arporter self-assigned this Nov 14, 2024
@tommbendall
Copy link
Collaborator

Thanks for posting this @sergisiso.

Maybe one solution here is to implement a new type of stencil (on the LFRic infrastructure side) that be need a new type of stencil that is explicitly parallel to the halos.

A hack might be to change the halo exchange to happen on:
MIN(field%get_halo_depth(), halo_depth + stencil_depth)

(and as I'm typing I've just seen Andy type the same thing!)

@arporter
Copy link
Member

We could have a rule that for kernels that operate on halo cells and have a stencil, any halo exchange will always be to the maximum depth? (We already have machinery for handling the special case of maximum depth.)

@tommbendall
Copy link
Collaborator

I realised that I didn't answer your question above. Yes the failures were because halo_depth + stencil_depth > field%get_halo_depth(), so we were trying force a halo exchange to a depth that didn't exist.

Maybe our halo exchanges should be performed to a depth MAX(field%get_halo_depth(), halo_depth).

I was just discussing this with @thomasmelvin and the challenge is when we actually want to fail and when we don't. Could we potentially print out a warning if field%get_halo_depth() < halo_depth + stencil_depth? There currently isn't a situation where we need to perform a halo exchange to depth halo_depth + stencil_depth but what should happen if someone tries to do that?

However if we halo exchange to MAX(field%get_halo_depth(), halo_depth), then we would get correctly get failures if halo_depth > field%get_halo_depth(), so maybe this is a good option?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LFRic Issue relates to the LFRic domain
Projects
None yet
Development

No branches or pull requests

3 participants