Skip to content

support for active_cells_map in kernels #3920

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

Merged
merged 97 commits into from
Apr 29, 2025
Merged

Conversation

simone-silvestri
Copy link
Collaborator

I am not sure how hacky this is or if it will work throughout the board, but this is supposed to achieve what was discussed in #3918:

for example, the current script:

using Oceananigans
grid = RectilinearGrid(size = (3, 3, 3), extent = (1, 1, 1))

using Oceananigans.Utils
using KernelAbstractions: @kernel, @index

@kernel function _test_indices(a)
    i, j, k = @index(Global, NTuple)
    @show a[i, j, k]
end

a = [(i, j, k) for i in 1:3, j in 1:3, k in 1:3]

returns

julia> launch!(CPU(), grid, :xyz, _test_indices, a)
a[i, j, k] = (1, 1, 1)
a[i, j, k] = (2, 1, 1)
a[i, j, k] = (3, 1, 1)
a[i, j, k] = (1, 2, 1)
a[i, j, k] = (2, 2, 1)
a[i, j, k] = (3, 2, 1)
a[i, j, k] = (1, 3, 1)
a[i, j, k] = (2, 3, 1)
a[i, j, k] = (3, 3, 1)
a[i, j, k] = (1, 1, 2)
a[i, j, k] = (2, 1, 2)
a[i, j, k] = (3, 1, 2)
a[i, j, k] = (1, 2, 2)
a[i, j, k] = (2, 2, 2)
a[i, j, k] = (3, 2, 2)
a[i, j, k] = (1, 3, 2)
a[i, j, k] = (2, 3, 2)
a[i, j, k] = (3, 3, 2)
a[i, j, k] = (1, 1, 3)
a[i, j, k] = (2, 1, 3)
a[i, j, k] = (3, 1, 3)
a[i, j, k] = (1, 2, 3)
a[i, j, k] = (2, 2, 3)
a[i, j, k] = (3, 2, 3)
a[i, j, k] = (1, 3, 3)
a[i, j, k] = (2, 3, 3)
a[i, j, k] = (3, 3, 3)

julia> launch!(CPU(), grid, :xyz, _test_indices, a; active_cells_map = [(1, 2, 3), (3, 2, 1)])
a[i, j, k] = (1, 2, 3)
a[i, j, k] = (3, 2, 1)

@simone-silvestri
Copy link
Collaborator Author

simone-silvestri commented Nov 12, 2024

Probably this is very hacky, a better way to go about it would be to add a map to the Kernel

@simone-silvestri simone-silvestri added the benchmark performance runs preconfigured benchamarks and spits out timing label Apr 25, 2025
@simone-silvestri
Copy link
Collaborator Author

This PR might be able to be merged, I am just not sure why the OceananigansAMDGPUExt extension is not precompiling

@simone-silvestri
Copy link
Collaborator Author

I guess we can merge this because it seems to work. It complicates kernel_launching.jl, but it simplifies the rest of the code

@glwagner
Copy link
Member

how about benchmarks

@simone-silvestri
Copy link
Collaborator Author

Should not play a role in performance, we can check the immersed benchmarks, which are the only ones affected.

@simone-silvestri
Copy link
Collaborator Author

In the diff_immersed_output.txt there seems to be something wrong in the data scraped by the main build, it looks like it is taking the previous main before the performance improvements of using the reciprocals, maybe updating the branch might fix this issue. However, it seems that this PR does not affect performance looking at the results on this branch and (the correct ones) on main. I ll update the branch to confirm

@simone-silvestri simone-silvestri merged commit 2109de4 into main Apr 29, 2025
59 checks passed
@simone-silvestri simone-silvestri deleted the ss/active-index-macro branch April 29, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark performance runs preconfigured benchamarks and spits out timing cleanup 🧹 Paying off technical debt 🚨 DO NOT MERGE 🚨 IN BIG BOLD RED CAPS WITH FLASHING SIRENS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants