Skip to content

Allow kernel functions with fewer indices for KernelFunctionOperation at reduced locations#5659

Open
simone-silvestri wants to merge 4 commits into
mainfrom
ss/allow-reduced-kernel-operations
Open

Allow kernel functions with fewer indices for KernelFunctionOperation at reduced locations#5659
simone-silvestri wants to merge 4 commits into
mainfrom
ss/allow-reduced-kernel-operations

Conversation

@simone-silvestri

Copy link
Copy Markdown
Collaborator

At the moment, KernelFunctionOperations work only with functions that have signature (i, j, k, grid, args...). However, some time, we want to have KernelFunctionOperations on genuinely 2D functions, corresponding to Nothing locations.
For example:

@inline static_column_depthᶠᶜᵃ(i, j, ibg::XFlatAGFIBG) = static_column_depthᶜᶜᵃ(i, j, ibg)
@inline static_column_depthᶜᶠᵃ(i, j, ibg::YFlatAGFIBG) = static_column_depthᶜᶜᵃ(i, j, ibg)
@inline static_column_depthᶠᶠᵃ(i, j, ibg::XFlatAGFIBG) = static_column_depthᶜᶠᵃ(i, j, ibg)
@inline static_column_depthᶠᶠᵃ(i, j, ibg::YFlatAGFIBG) = static_column_depthᶠᶜᵃ(i, j, ibg)

At the moment, we would need to create a helper function that redirects a k index into two-d functions.
This PR makes this step automatic so that it would be possible, for example, to do

KernelFunctionOperation{Center, Center, Nothing}(static_column_depthᶜᶜᵃ, grid)

without requiring extra code

```julia
kernel_function(i, j, grid, arguments...)
```

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest including a full example that illustrates how this works including the KernelFunctionOperation{Center, Center, Nothing} constructor, to be fully explicit

@glwagner glwagner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember considering this at some point in the past, but that there were some issues or barriers to it. Perhaps there are not.

# Three-index kernel functions at reduced locations still work
three_index_kernel_function(i, j, k, grid) = i + j
op = KernelFunctionOperation{Center, Center, Nothing}(three_index_kernel_function, grid)
@test Array(interior(compute!(Field(op))))[:, :, 1] == [i + j for i in 1:size(grid, 1), j in 1:size(grid, 2)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to call compute!(Field(op)), because compute! is already called within Field(op).

The indexing is also messed up, so this can be interior(Field(op), :, :, 1) |> Array or something.

@simone-silvestri

Copy link
Copy Markdown
Collaborator Author

I think the problem here is which method takes precedence, the three index method, the reduced method, or just allow only a reduced method which will cause no ambiguity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants