-
Notifications
You must be signed in to change notification settings - Fork 194
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
Reductions with FieldTimeSeries
on an ImmersedBoundaryGrid
are very slow
#3750
Comments
I thought this might be an issue with using Oceananigans
arch = CPU()
L = 1
H = 1
underlying_grid = LatitudeLongitudeGrid(
arch;
topology = (Bounded, Bounded, Bounded),
size = (512, 512, 64),
latitude = (-L/2, L/2),
longitude = (-L/2, L/2),
z = (-H, 0),
halo = (4, 4, 4)
)
h = L/2
w = L/5
mount(x, y) = h * exp(-x^2 / 2w^2) * exp(-y^2 / 2w^2)
bottom(x, y) = -H + mount(x, y)
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom(bottom))
model = HydrostaticFreeSurfaceModel(; grid)
u = model.velocities.u then julia> @time minimum(u)
0.063563 seconds (344 allocations: 31.789 KiB)
0.0
julia> @time minimum(u.data)
0.013262 seconds (3 allocations: 1.391 KiB)
0.0 Only ~6x slower. So maybe the I haven't looked into the code much but wanted to at least open this issue. |
Reductions on FieldTimeSeries are performed individually for each element by constructing two Fields and reducing one into another. Probably, the construction of the individual field is what is causing the loss in performance? |
Just a thought that we probably want reductions on field time series to be performant anyways. So it's better that we call data summary because then more people will be annoyed that they are slow => more pressure to fix it 😆 |
PS there are more minimal ways to create a |
But why are there memory allocations when calling The problem is actually computing reductions of windowed immersed fields. julia> @time minimum(u2)
0.031018 seconds (366 allocations: 33.102 KiB, 6.05% compilation time)
0.0 |
Here's an illustration: using Oceananigans
Nx, Ny, Nz = 100, 100, 100
latitude = longitude = z = (0, 1)
underlying_grid = LatitudeLongitudeGrid(size=(Nx, Ny, Nz); latitude, longitude, z)
grid = ImmersedBoundaryGrid(underlying_grid, GridFittedBottom((λ, φ) -> 0.5))
ci = CenterField(grid)
ciw = view(ci, 1:Nx, 1:Ny, 1:Nz)
cu = CenterField(underlying_grid)
cuw = view(cu, 1:Nx, 1:Ny, 1:Nz)
for n = 1:10
@time minimum(ci)
@time minimum(ciw)
@time minimum(cu)
@time minimum(cuw)
end Note there is such a concept as "stubborn compilation" so we sometimes have to invoke functions a few times to get them to compile... Now I get: julia> @time minimum(ci)
0.000888 seconds (331 allocations: 33.148 KiB)
0.0
julia> @time minimum(ciw)
1.611260 seconds (7.27 M allocations: 7.968 GiB, 37.23% gc time)
0.0
julia> @time minimum(cu)
0.001069 seconds (387 allocations: 21.586 KiB)
0.0
julia> @time minimum(cuw)
0.001060 seconds (686 allocations: 33.258 KiB)
0.0 |
This also makes
data_summary
andBase.show
very slow since showing aFieldTimeSeries
prints its min, mean, and max. So it's harder to work withFieldTimeSeries
interactively. Seems fine when not on aImmersedBoundaryGrid
.I'm guessing it's slower because it's masking out the immersed values but I don't know if we expect it to be ~2000x slower than without an immersed boundary. It's those memory allocations...
A quick quality-of-life fix could be to not call
data_summary
when showing aFieldTimeSeries
.MWE
Reduction over the
FieldTimeSeries
:Reduction over the underlying data:
or almost 2000x faster.
Environment
Oceananigans.jl
main
branch withThe text was updated successfully, but these errors were encountered: