-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make Nv a type parameter #1757
Make Nv a type parameter #1757
Conversation
93804c7
to
2236720
Compare
Note to self: import |
Perhaps this closes #11. I think we don’t actually need |
9105bd0
to
ff3b484
Compare
ff3b484
to
2b51d76
Compare
I know that there's variation in CI timings, but this nevertheless looks interesting:
(these are CPU benchmarks) |
2b51d76
to
1b84475
Compare
GPU target pipeline was run here: https://buildkite.com/clima/climaatmos-target-gpu-simulations/builds/308. So, only 1-2% faster on the GPU, perhaps within the noise. After all, this PR is was not intended to introduce a performance improvement, it was only intended to allow us to rebuild DataLayouts inside broadcast expressions with MArrays (#1746). |
I don't think/consider the type parameters of our DataLayouts to be public-facing, so I'd consider these changes to be non-breaking. However, ClimaAtmos has some functions that broke due to this PR. The functions are: '''
array2field(array, space)
Wraps `array` in a `ClimaCore` `Field` that is defined over `space`. Can be used
to simplify the process of getting and setting values in an `RRTMGPModel`; e.g.
'''
array2field(model.center_temperature, center_space) .=
center_temperature_field
face_flux_field .= array2field(model.face_flux, face_space)
'''
The dimensions of `array` are assumed to be `([number of vertical nodes], number
of horizontal nodes)`. Also, `array` must represent a `Field` of scalars, so
that the struct type of the resulting `Field` is the same as the element type of
`array`. If this restriction were removed, one would also need to pass the
desired `Field` struct type as an argument to `array2field`, which would then
need to permute the dimensions of `array` to match the target `DataLayout`.
'''
array2field(array, space) =
Fields.Field(array2data(array, Spaces.local_geometry_data(space)), space)
array2data(
array::AbstractArray{T, 1},
::DataLayouts.IF{<:Any, Ni},
) where {T, Ni} = DataLayouts.IF{T, Ni}(reshape(array, Ni, 1))
array2data(
array::AbstractArray{T, 1},
::DataLayouts.IFH{<:Any, Ni},
) where {T, Ni} = DataLayouts.IFH{T, Ni}(reshape(array, Ni, 1, :))
array2data(
array::AbstractArray{T, 1},
::DataLayouts.IJF{<:Any, Nij},
) where {T, Nij} = DataLayouts.IJF{T, Nij}(reshape(array, Nij, Nij, 1))
array2data(
array::AbstractArray{T, 1},
::DataLayouts.IJFH{<:Any, Nij},
) where {T, Nij} = DataLayouts.IJFH{T, Nij}(reshape(array, Nij, Nij, 1, :))
array2data(
array::AbstractArray{T, 2},
::DataLayouts.VF{<:Any, Nv},
) where {T, Nv} = DataLayouts.VF{T, Nv}(reshape(array, size(array, 1), 1))
array2data(
array::AbstractArray{T, 2},
::DataLayouts.VIFH{<:Any, Nv, Ni},
) where {T, Nv, Ni} =
DataLayouts.VIFH{T, Nv, Ni}(reshape(array, size(array, 1), Ni, 1, :))
array2data(
array::AbstractArray{T, 2},
::DataLayouts.VIJFH{<:Any, Nv, Nij},
) where {T, Nv, Nij} = DataLayouts.VIJFH{T, Nv, Nij}(
reshape(array, size(array, 1), Nij, Nij, 1, :),
) Arguably, these should belong in ClimaCore. |
This PR adds
Nv
, the number of vertical levels, to the type space in all datalayouts with vertical indices.This is a step towards #11, and is required for #1746.