Skip to content

Commit ef0a74e

Browse files
dennisYatuninclaude
andcommitted
Fix downstream ClimaAtmos CI failures
- Compact kernel parameters: adapt every DataLayout view to a CompactDeviceView at kernel launch, storing an Int32 offset per restricted dimension instead of a SubArray (72-88 vs 128-160 bytes per broadcast argument), so that large EDMF broadcasts fit the 4 KiB parameter limit of sm_60 GPUs; full arrays pass through unchanged. - Combine DataScopes with pairwise recursion instead of unrolled helpers: broadcasts with many arguments (EDMF tendencies) make inference hit its recursion limiter inside UnrolledUtilities' methods, which the DataLayouts recursion_relation lift cannot cover, so every scope in the launch path widens to Any and dispatches at runtime at every slice. Interleaved Larcform1 measurements: median step allocations drop from 17118 (10.19 MB) to 3280 (1.18 MB), below main's 5298 (1.12 MB), with median step time 9.6 ms -> 8.2 ms. - QuasiMonotoneLimiter: apply the limit through one scalar view per component of ρq, indexing DataLayouts instead of reshaping slab parent arrays, since Base.reshape of a device-array SubArray is uncompilable in kernels under Julia 1.11. - Fix the GPU MPI DSS exchange buffer index, which overlapped items whenever Nv > 1 and Nf > 1, corrupting every distributed GPU weighted_dss! of extruded multi-component fields. - Restore two behaviors from main that downstream code relies on: zero-size fields are hidden from DataLayout propertynames (restart comparisons recurse into Tensor bases otherwise), and array2field derives its parent shape from any layout (PointSpace included). - Support the old universal CartesianIndex{5} indexing convention on layouts, slabs, and columns, which registered downstream packages like ClimaCoreTempestRemap still use. - Dev-install lib/ClimaCoreMakie in the performance pipeline, since the registered version still destructures the old 5-tuple layout size. - Remove the unused needs_projection methods for AutoBroadcasters. - unit_loops.jl: seed the RNG, and compare single-threaded reductions against a hand-written pairwise reference, since the @simd blocks in Base's mapreduce reassociate contiguous values unless bounds checks are enabled. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 331a6dd commit ef0a74e

15 files changed

Lines changed: 554 additions & 142 deletions

File tree

.buildkite/perf/pipeline.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ steps:
2626
# Instantiate and dev install ClimaCore
2727
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.instantiate(;verbose=true)'
2828
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.develop(path=".")'
29+
# The registered ClimaCoreMakie still destructures the old 5-tuple layout
30+
# size in plot_triangles, which crashes the AMIP postprocessing plots, so
31+
# dev-install the migrated in-repo version alongside ClimaCore.
32+
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.develop(path="lib/ClimaCoreMakie")'
2933
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.add("MPI")'
3034
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.precompile()'
3135
- julia --project=$COUPLER_PATH/experiments/AMIP/ -e 'using Pkg; Pkg.status()'

ext/cuda/data_layouts.jl

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,122 @@ import ClimaCore: DataLayouts
66
include("scopes.jl")
77
include("loops.jl")
88
include("data_layouts_threadblock.jl")
9+
10+
# Kernel parameters are limited to 4 KiB of memory before compute capability
11+
# 7.0, and a SubArray of a 5-D CuDeviceArray uses 128-160 of those bytes per
12+
# broadcast argument (64 for the parent array, 48-80 for the index ranges, and
13+
# 16 for precomputed linear-indexing fields), so broadcasts over a few dozen
14+
# field views cannot be launched as kernels. Since every extent of the array in
15+
# a DataLayout is either available from the layout's type or identical to the
16+
# corresponding extent of the parent array, the index ranges can be replaced
17+
# with an Int32 offset for every restricted dimension, plus an Int32 extent for
18+
# every restricted dimension whose extent is not available from the type. The
19+
# type parameters are the extent of each dimension `E` (with 0 for extents that
20+
# are only available at runtime), the restricted dimensions `R`, and the tuple
21+
# lengths `K = length(R)` and `D = count(d -> iszero(E[d]), R)`.
22+
struct CompactDeviceView{T, N, E, R, K, D, A <: AbstractArray{T, N}} <:
23+
AbstractArray{T, N}
24+
parent::A
25+
offsets::NTuple{K, Int32}
26+
dynamic_extents::NTuple{D, Int32}
27+
end
28+
29+
Base.parent(view::CompactDeviceView) = view.parent
30+
31+
DataLayouts.DataScope(
32+
::Type{<:CompactDeviceView{<:Any, <:Any, <:Any, <:Any, <:Any, <:Any, A}},
33+
) where {A} = DataLayouts.DataScope(A)
34+
35+
@inline function Base.size(
36+
view::CompactDeviceView{<:Any, N, E, R},
37+
) where {N, E, R}
38+
return ntuple(Val(N)) do d
39+
iszero(E[d]) || return Int(E[d])
40+
d in R || return size(parent(view), d)
41+
return Int(view.dynamic_extents[count(r -> r <= d && iszero(E[r]), R)])
42+
end
43+
end
44+
45+
# Index into the parent array that corresponds to an index into the view,
46+
# shifted by the stored offset along each restricted dimension
47+
@inline parent_index(
48+
view::CompactDeviceView{<:Any, N, <:Any, R},
49+
index,
50+
) where {N, R} =
51+
ntuple(Val(N)) do d
52+
position = findfirst(==(d), R)
53+
isnothing(position) ? index[d] : index[d] + Int(view.offsets[position])
54+
end
55+
56+
Base.@propagate_inbounds function Base.getindex(
57+
view::CompactDeviceView{<:Any, N},
58+
index::Vararg{Integer, N},
59+
) where {N}
60+
@boundscheck checkbounds(view, index...)
61+
return @inbounds parent(view)[parent_index(view, index)...]
62+
end
63+
64+
Base.@propagate_inbounds function Base.setindex!(
65+
view::CompactDeviceView{<:Any, N},
66+
value,
67+
index::Vararg{Integer, N},
68+
) where {N}
69+
@boundscheck checkbounds(view, index...)
70+
@inbounds parent(view)[parent_index(view, index)...] = value
71+
return view
72+
end
73+
74+
# Index types generated by stable_view for unrestricted and restricted
75+
# dimensions of the parent array in a DataLayout
76+
const ViewDimIndex = Union{
77+
Base.Slice{<:Base.OneTo{<:Integer}},
78+
Base.OneTo{<:Integer},
79+
UnitRange{<:Integer},
80+
}
81+
82+
# Extent of every parent array dimension that is available from a DataLayout's
83+
# type, with 0 for dimensions whose extents are only available at runtime
84+
@inline inferred_parent_extents(data, array) = DataLayouts.add_f_dim(
85+
map(extent -> something(extent, 0), DataLayouts.inferred_size(data)),
86+
DataLayouts.num_basetypes(eltype(array), eltype(data)),
87+
Val(DataLayouts.f_dim(data)),
88+
)
89+
90+
compact_device_view(array, data) = array
91+
92+
@inline function compact_device_view(
93+
array::SubArray{<:Any, N, <:CUDA.CuDeviceArray, <:NTuple{N, ViewDimIndex}},
94+
data::DataLayouts.DataLayout,
95+
) where {N}
96+
extents = inferred_parent_extents(data, array)
97+
length(extents) == N ||
98+
throw(DimensionMismatch("DataLayout extents do not match its array"))
99+
indices = parentindices(array)
100+
restricted =
101+
filter(d -> indices[d] isa UnitRange, ntuple(identity, Val(N)))
102+
dynamic = filter(d -> iszero(extents[d]), restricted)
103+
view = CompactDeviceView{
104+
eltype(array),
105+
N,
106+
extents,
107+
restricted,
108+
length(restricted),
109+
length(dynamic),
110+
typeof(parent(array)),
111+
}(
112+
parent(array),
113+
map(d -> Int32(first(indices[d]) - 1), restricted),
114+
map(d -> Int32(length(indices[d])), dynamic),
115+
)
116+
# Validates every extent assumption at launch time, including that any
117+
# Base.OneTo indices span their full parent dimensions
118+
size(view) == size(array) ||
119+
throw(DimensionMismatch("DataLayout extents do not match its array"))
120+
return view
121+
end
122+
123+
Adapt.adapt_structure(to::CUDA.KernelAdaptor, data::DataLayouts.DataLayout) =
124+
DataLayouts.rebuild(
125+
data,
126+
compact_device_view(Adapt.adapt(to, parent(data)), data),
127+
)

ext/cuda/topologies_dss.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ function Topologies.fill_send_buffer!(
252252
# Avoid indexing with a colon, which would allocate in the kernel.
253253
(h, p) = (send_buf_idx[send_index, 1], send_buf_idx[send_index, 2])
254254
item = perimeter_data[v, p, 1, h]
255-
buffer_index = v + (send_index - 1) * Nv * Nf
255+
buffer_index = (v - 1) * Nf + 1 + (send_index - 1) * Nv * Nf
256256
DataLayouts.set_struct!(send_data, item, buffer_index, Val(1))
257257
end
258258
nothing
@@ -279,7 +279,7 @@ function Topologies.load_from_recv_buffer!(
279279
(v, recv_index) = CartesianIndices((Nv, nrecv))[gidx].I
280280
# Avoid indexing with a colon, which would allocate in the kernel.
281281
(h, p) = (recv_buf_idx[recv_index, 1], recv_buf_idx[recv_index, 2])
282-
buffer_index = v + (recv_index - 1) * Nv * Nf
282+
buffer_index = (v - 1) * Nf + 1 + (recv_index - 1) * Nv * Nf
283283
item_view = DataLayouts.view_struct(recv_data, T, buffer_index, Val(1))
284284
parent_view = parent(view(perimeter_data, v, p, 1, h))
285285
for f in 1:Nf

src/DataLayouts/DataLayouts.jl

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,13 @@ function replace_basetype(data::DataLayout, ::Type{B}) where {B}
274274
return similar_layout(data, T, B)
275275
end
276276

277-
@inline Base.propertynames(data::DataLayout) = fieldnames(eltype(data))
277+
# Hide zero-size fields (e.g. the singleton bases of Tensors) from property
278+
# iteration, so that generic code which recursively walks propertynames only
279+
# encounters properties that contain data. Zero-size fields are still
280+
# accessible through getproperty, which returns a view with an empty F axis.
281+
@inline function Base.propertynames(::DataLayout{T}) where {T}
282+
filter(name -> sizeof(fieldtype(T, name)) > 0, fieldnames(T))
283+
end
278284

279285
# Wrap the field index in a Val as soon as it is available, resolving field
280286
# views through specialization rather than constant propagation. Making the Val
@@ -532,10 +538,11 @@ include("deprecated.jl")
532538

533539
# Drop the recursion limits of this module's Core.kwcall methods and recursive
534540
# DataScope functions, so that kwarg functions like fill! and column_reduce! can
535-
# be composed and slice_subscope can repeatedly partition a scope. The default
536-
# limit makes the compiler widen argument types, leading to dynamic dispatch.
541+
# be composed, multiple scopes can be combined, and is_subscope/slice_subscope
542+
# can repeatedly partition a scope. The default limit makes the compiler widen
543+
# argument types, leading to dynamic dispatch and runtime allocations.
537544
@static if hasfield(Method, :recursion_relation)
538-
for f in (Core.kwcall, slice_subscope, is_subscope), method in methods(f)
545+
for f in (Core.kwcall, DataScope, is_subscope, slice_subscope), method in methods(f)
539546
method.module === (@__MODULE__) || continue
540547
method.recursion_relation = Returns(true)
541548
end

src/DataLayouts/deprecated.jl

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,26 @@ const IJHF{S, Nij} = VIJHF{S, 1, Nij, Nij, nothing}
3131
# These were exported on main, and packages like ClimaCoreTempestRemap access
3232
# them through `using ClimaCore.DataLayouts`.
3333
export AbstractData, IJFH, IJHF
34+
35+
# Old: getindex/setindex! with a 5-D "universal index" CartesianIndex(i, j, f,
36+
# v, h), whose f component was ignored because the entire value was read or
37+
# written. New: Cartesian indices have one coordinate per array dimension, in
38+
# the order (v, i, j, h). Packages like ClimaCoreTempestRemap still index full
39+
# layouts, slabs, and columns with universal indices (e.g.,
40+
# `slab(data, h)[CartesianIndex(i, j, 1, 1, 1)]`), so the methods below
41+
# translate 5-D indices from the old convention to the new one. They override
42+
# the generic AbstractArray interpretation of a `CartesianIndex{5}` as a 4-D
43+
# index with a trailing singleton coordinate, so new code must not add trailing
44+
# coordinates when indexing into 4-dimensional layouts. Remove these methods
45+
# together with the aliases above.
46+
@inline universal_index_shim(I::CartesianIndex{5}) =
47+
CartesianIndex(I[4], I[1], I[2], I[5])
48+
@propagate_inbounds Base.getindex(data::VIJHWithF, I::CartesianIndex{5}) =
49+
getindex(data, universal_index_shim(I))
50+
@propagate_inbounds Base.setindex!(data::VIJHWithF, value, I::CartesianIndex{5}) =
51+
setindex!(data, value, universal_index_shim(I))
52+
53+
# Old: every component of a universal index was ignored for 0-dimensional data.
54+
@propagate_inbounds Base.getindex(data::DataF, I::CartesianIndex{5}) = data[]
55+
@propagate_inbounds Base.setindex!(data::DataF, value, I::CartesianIndex{5}) =
56+
setindex!(data, value)

src/DataLayouts/scopes.jl

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ DataScope(::Type{<:Base.ReshapedArray{<:Any, <:Any, A}}) where {A} = DataScope(A
4646
# Infer parent types of other AbstractArrays (constant-folding not guaranteed).
4747
DataScope(::Type{A}) where {A <: AbstractArray} = DataScope(return_type(parent, Tuple{A}))
4848

49-
DataScope(arg1, arg2, args...) =
50-
unrolled_reduce(unrolled_map(DataScope, (arg1, arg2, args...))) do scope1, scope2
51-
is_subscope(scope1, scope2) ? scope1 :
52-
is_subscope(scope2, scope1) ? scope2 :
53-
throw(ArgumentError(non_overlapping_scopes_string(scope1, scope2)))
54-
end
49+
DataScope(scope1::DataScope, scope2::DataScope) =
50+
is_subscope(scope1, scope2) ? scope1 :
51+
is_subscope(scope2, scope1) ? scope2 :
52+
throw(ArgumentError(non_overlapping_scopes_string(scope1, scope2)))
5553
@generated non_overlapping_scopes_string(::S1, ::S2) where {S1, S2} =
5654
"$S1 and $S2 do not overlap, so they cannot be put in the same DataScope"
5755

56+
DataScope(arg1, arg2, args...) = DataScope(DataScope(arg1), DataScope(arg2, args...))
57+
5858
"""
5959
partition(scope)
6060
@@ -194,11 +194,11 @@ struct ThisThreadPool <: DataScope end
194194
# _sym_to_tpid (0 = :interactive, 1 = :default); fall back to the public API if the
195195
# internals change.
196196
@static if isdefined(Threads, :_nthreads_in_pool)
197-
@inline default_pool_size() = Int(Threads._nthreads_in_pool(Int8(1)))
198-
@inline interactive_pool_size() = Int(Threads._nthreads_in_pool(Int8(0)))
197+
default_pool_size() = Int(Threads._nthreads_in_pool(Int8(1)))
198+
interactive_pool_size() = Int(Threads._nthreads_in_pool(Int8(0)))
199199
else
200-
@inline default_pool_size() = Threads.threadpoolsize(:default)
201-
@inline interactive_pool_size() = Threads.threadpoolsize(:interactive)
200+
default_pool_size() = Threads.threadpoolsize(:default)
201+
interactive_pool_size() = Threads.threadpoolsize(:interactive)
202202
end
203203

204204
# Threads.threading_run compiles faster than an equivalent static Threads.@threads loop
@@ -219,8 +219,8 @@ end
219219

220220
# Task-local storage marks ClimaCore-launched threads, distinguishing them from external
221221
# threaded loops; storage is nothing until first set, so storage-less threads are external.
222-
@inline running_in_threaded_loop() = !iszero(ccall(:jl_in_threaded_region, Cint, ()))
223-
@inline function running_in_external_threaded_loop()
222+
running_in_threaded_loop() = !iszero(ccall(:jl_in_threaded_region, Cint, ()))
223+
function running_in_external_threaded_loop()
224224
running_in_threaded_loop() || return false
225225
storage = current_task().storage
226226
return isnothing(storage) ||

src/Fields/Fields.jl

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -584,10 +584,7 @@ The struct type of the resulting `Field` is set to the array's element type.
584584
"""
585585
function array2field(array, space)
586586
data = Spaces.local_geometry_data(space)
587-
(; Nv, Ni, Nj, Nh, F) = DataLayouts.shape_params(data)
588-
Nh_dynamic = isnothing(Nh) ? DataLayouts.nelems(data) : Nh
589-
array_size =
590-
DataLayouts.add_f_dim((Nv, Ni, Nj, Nh_dynamic), 1, Val(F))
587+
array_size = DataLayouts.add_f_dim(size(data), 1, Val(DataLayouts.f_dim(data)))
591588
parent_array = reshape(array, array_size)
592589
return Field(DataLayouts.rebuild(data, parent_array, eltype(array)), space)
593590
end

src/Fields/field_iterator.jl

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ function flattened_property_chains!(prop_chains, f::Field, pc = ())
3838
else
3939
for pn in propertynames(f)
4040
p = getproperty(f, pn)
41-
# Skip properties of singleton types, which do not contain data
42-
sizeof(eltype(p)) == 0 && continue
4341
flattened_property_chains!(prop_chains, p, (pc..., pn))
4442
end
4543
end

src/Geometry/auto_broadcaster_methods.jl

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,6 @@ mul_with_projection(x::AutoBroadcaster, y, lg) =
2020
mul_with_projection(x, y::AutoBroadcaster, lg) =
2121
nested_broadcast(y -> mul_with_projection(x, y, lg), y)
2222

23-
needs_projection(
24-
::Type{X},
25-
::Type{Y},
26-
) where {X <: AutoBroadcaster, Y <: AutoBroadcaster} =
27-
needs_projection(eltype(X), eltype(Y))
28-
needs_projection(::Type{X}, ::Type{Y}) where {X <: AutoBroadcaster, Y} =
29-
needs_projection(eltype(X), Y)
30-
needs_projection(::Type{X}, ::Type{Y}) where {X, Y <: AutoBroadcaster} =
31-
needs_projection(X, eltype(Y))
32-
3323
mul_return_type(
3424
::Type{X},
3525
::Type{Y},

src/Limiters/Limiters.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
module Limiters
22

33
import ..DataLayouts, ..Topologies, ..Spaces, ..Fields
4+
using UnrolledUtilities
45
import ..DebugOnly: call_post_op_callback, post_op_callback
56
import ClimaCore: slab
67

0 commit comments

Comments
 (0)