Skip to content

Commit

Permalink
Remove performance-/precompilation-time harmful @eval (#3556)
Browse files Browse the repository at this point in the history 
* bugfix

* remove evals

* remove other eval

* bugfix

* Update src/Grids/latitude_longitude_grid.jl

Co-authored-by: Gregory L. Wagner <[email protected]>

* remove yet another eval

* bugfix

* Update src/Grids/latitude_longitude_grid.jl

Co-authored-by: Gregory L. Wagner <[email protected]>

* Update src/Grids/latitude_longitude_grid.jl

Co-authored-by: Gregory L. Wagner <[email protected]>

* simplify code

* name change to reconstruction_coefficients function

* bugfix

* last bugfix

* Replace a couple of evals with getglobal

* Mark a few places where eval needs to be removed

* Replace eval within interpolations utils with getglobal

* removed last evals

* correct typo

* correct typos

---------

Co-authored-by: Gregory L. Wagner <[email protected]>
Co-authored-by: Valentin Churavy <[email protected]>
  • Loading branch information
@simone-silvestri @glwagner @vchuravy
3 people authored Jun 24, 2024
1 parent 96fa286 commit 7f2d512
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 97 deletions.
2 changes: 1 addition & 1 deletion src/AbstractOperations/grid_metrics.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function metric_function(loc, metric::AbstractGridMetric)
code = Tuple(interpolation_code(ℓ) for ℓ in loc)
prefix = metric_function_prefix(metric)
metric_function_symbol = Symbol(prefix, code...)
return eval(metric_function_symbol)
return getglobal(@__MODULE__, metric_function_symbol)
end

struct GridMetricOperation{LX, LY, LZ, G, T, M} <: AbstractOperation{LX, LY, LZ, G, T}
Expand Down
39 changes: 20 additions & 19 deletions src/Advection/reconstruction_coefficients.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,25 +209,26 @@ end
@inline function compute_reconstruction_coefficients(grid, FT, scheme; order)

method = scheme == :Centered ? 1 : scheme == :Upwind ? 2 : 3

rect_metrics = (:xᶠᵃᵃ, :xᶜᵃᵃ, :yᵃᶠᵃ, :yᵃᶜᵃ, :zᵃᵃᶠ, :zᵃᵃᶜ)


if grid isa Nothing
for metric in rect_metrics
@eval $(Symbol(:coeff_ , metric)) = nothing
@eval $(Symbol(:smooth_, metric)) = nothing
end
coeff_xᶠᵃᵃ = nothing
coeff_xᶜᵃᵃ = nothing
coeff_yᵃᶠᵃ = nothing
coeff_yᵃᶜᵃ = nothing
coeff_zᵃᵃᶠ = nothing
coeff_zᵃᵃᶜ = nothing
else
metrics = coordinates(grid)
dirsize = (:Nx, :Nx, :Ny, :Ny, :Nz, :Nz)

arch = architecture(grid)
Hx, Hy, Hz = halo_size(grid)
new_grid = with_halo((Hx+1, Hy+1, Hz+1), grid)

for (dir, metric, rect_metric) in zip(dirsize, metrics, rect_metrics)
@eval $(Symbol(:coeff_ , rect_metric)) = calc_reconstruction_coefficients($FT, $new_grid.$metric, $arch, $new_grid.$dir, Val($method); order = $order)
end
metrics = coordinates(grid)

coeff_xᶠᵃᵃ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[1]), arch, new_grid.Nx, Val(method); order)
coeff_xᶜᵃᵃ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[2]), arch, new_grid.Nx, Val(method); order)
coeff_yᵃᶠᵃ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[3]), arch, new_grid.Ny, Val(method); order)
coeff_yᵃᶜᵃ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[4]), arch, new_grid.Ny, Val(method); order)
coeff_zᵃᵃᶠ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[5]), arch, new_grid.Nz, Val(method); order)
coeff_zᵃᵃᶜ = reconstruction_coefficients(FT, getproperty(new_grid, metrics[6]), arch, new_grid.Nz, Val(method); order)
end

return (coeff_xᶠᵃᵃ, coeff_xᶜᵃᵃ, coeff_yᵃᶠᵃ, coeff_yᵃᶜᵃ, coeff_zᵃᵃᶠ, coeff_zᵃᵃᶜ)
Expand All @@ -236,21 +237,21 @@ end
# Fallback for uniform directions
for val in [1, 2, 3]
@eval begin
@inline calc_reconstruction_coefficients(FT, coord::OffsetArray{<:Any, <:Any, <:AbstractRange}, arch, N, ::Val{$val}; order) = nothing
@inline calc_reconstruction_coefficients(FT, coord::AbstractRange, arch, N, ::Val{$val}; order) = nothing
@inline reconstruction_coefficients(FT, coord::OffsetArray{<:Any, <:Any, <:AbstractRange}, arch, N, ::Val{$val}; order) = nothing
@inline reconstruction_coefficients(FT, coord::AbstractRange, arch, N, ::Val{$val}; order) = nothing
end
end

# Stretched reconstruction coefficients for `Centered` schemes
@inline function calc_reconstruction_coefficients(FT, coord, arch, N, ::Val{1}; order)
@inline function reconstruction_coefficients(FT, coord, arch, N, ::Val{1}; order)
cpu_coord = on_architecture(CPU(), coord)
r = ((order + 1) ÷ 2) - 1
s = create_reconstruction_coefficients(FT, r, cpu_coord, arch, N; order)
return s
end

# Stretched reconstruction coefficients for `UpwindBiased` schemes
@inline function calc_reconstruction_coefficients(FT, coord, arch, N, ::Val{2}; order)
@inline function reconstruction_coefficients(FT, coord, arch, N, ::Val{2}; order)
cpu_coord = on_architecture(CPU(), coord)
rleft = ((order + 1) ÷ 2) - 2
rright = ((order + 1) ÷ 2) - 1
Expand All @@ -262,7 +263,7 @@ end
end

# Stretched reconstruction coefficients for `WENO` schemes
@inline function calc_reconstruction_coefficients(FT, coord, arch, N, ::Val{3}; order)
@inline function reconstruction_coefficients(FT, coord, arch, N, ::Val{3}; order)

cpu_coord = on_architecture(CPU(), coord)
s = []
Expand Down
25 changes: 9 additions & 16 deletions src/Grids/latitude_longitude_grid.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,16 +528,6 @@ end

function allocate_metrics(grid::LatitudeLongitudeGrid)
FT = eltype(grid)

# preallocate quantities to ensure correct type and size
grid_metrics = (:Δxᶠᶜ,
:Δxᶜᶠ,
:Δxᶠᶠ,
:Δxᶜᶜ,
:Azᶠᶜ,
:Azᶜᶠ,
:Azᶠᶠ,
:Azᶜᶜ)

arch = grid.architecture

Expand All @@ -546,14 +536,17 @@ function allocate_metrics(grid::LatitudeLongitudeGrid)
metric_size = length(grid.φᵃᶜᵃ)
else
offsets = (grid.Δλᶜᵃᵃ.offsets[1], grid.φᵃᶜᵃ.offsets[1])
metric_size = (length(grid.Δλᶜᵃᵃ) , length(grid.φᵃᶜᵃ))
metric_size = (length(grid.Δλᶜᵃᵃ), length(grid.φᵃᶜᵃ))
end

for metric in grid_metrics
parentM = Symbol(metric, :_parent)
@eval $parentM = zeros($FT, $metric_size...)
@eval $metric = OffsetArray(on_architecture($arch, $parentM), $offsets...)
end
Δxᶠᶜ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Δxᶜᶠ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Δxᶠᶠ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Δxᶜᶜ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Azᶠᶜ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Azᶜᶠ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Azᶠᶠ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)
Azᶜᶜ = OffsetArray(zeros(FT, arch, metric_size...), offsets...)

if grid isa YRegularLLG
Δyᶠᶜ = FT(0)
Expand Down
21 changes: 12 additions & 9 deletions src/Grids/nodes_and_spacings.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ julia> xspacings(grid, Center(), Face(), Center())
"""
@inline xspacings(grid, ℓx, ℓy, ℓz; with_halos=true) = xspacings(grid, ℓx; with_halos)


"""
yspacings(grid, ℓx, ℓy, ℓz; with_halos=true)
Expand Down Expand Up @@ -233,8 +232,12 @@ julia> zspacings(grid, Center(), Center(), Center())
destantiate(::Face) = Face
destantiate(::Center) = Center

function minimum_spacing(dir, grid, ℓx, ℓy, ℓz)
spacing = eval(Symbol(dir, :spacing))
spacing_function(::Val{:x}) = xspacing
spacing_function(::Val{:y}) = yspacing
spacing_function(::Val{:z}) = zspacing

function minimum_spacing(s, grid, ℓx, ℓy, ℓz)
spacing = spacing_function(s)
LX, LY, LZ = map(destantiate, (ℓx, ℓy, ℓz))
Δ = KernelFunctionOperation{LX, LY, LZ}(spacing, grid, ℓx, ℓy, ℓz)

Expand All @@ -258,8 +261,8 @@ julia> minimum_xspacing(grid, Center(), Center(), Center())
0.5
```
"""
minimum_xspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(:x, grid, ℓx, ℓy, ℓz)
minimum_xspacing(grid) = minimum_spacing(:x, grid, Center(), Center(), Center())
minimum_xspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(Val(:x), grid, ℓx, ℓy, ℓz)
minimum_xspacing(grid) = minimum_spacing(Val(:x), grid, Center(), Center(), Center())
"""
minimum_yspacing(grid, ℓx, ℓy, ℓz)
minimum_yspacing(grid) = minimum_yspacing(grid, Center(), Center(), Center())
Expand All @@ -277,8 +280,8 @@ julia> minimum_yspacing(grid, Center(), Center(), Center())
0.25
```
"""
minimum_yspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(:y, grid, ℓx, ℓy, ℓz)
minimum_yspacing(grid) = minimum_spacing(:y, grid, Center(), Center(), Center())
minimum_yspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(Val(:y), grid, ℓx, ℓy, ℓz)
minimum_yspacing(grid) = minimum_spacing(Val(:y), grid, Center(), Center(), Center())

"""
minimum_zspacing(grid, ℓx, ℓy, ℓz)
Expand All @@ -297,7 +300,7 @@ julia> minimum_zspacing(grid, Center(), Center(), Center())
0.125
```
"""
minimum_zspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(:z, grid, ℓx, ℓy, ℓz)
minimum_zspacing(grid) = minimum_spacing(:z, grid, Center(), Center(), Center())
minimum_zspacing(grid, ℓx, ℓy, ℓz) = minimum_spacing(Val(:z), grid, ℓx, ℓy, ℓz)
minimum_zspacing(grid) = minimum_spacing(Val(:z), grid, Center(), Center(), Center())


100 changes: 53 additions & 47 deletions src/MultiRegion/cubed_sphere_grid.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,59 +241,65 @@ function ConformalCubedSphereGrid(arch::AbstractArchitecture=CPU(), FT=Float64;
region_grids,
devices)

fields = (:λᶜᶜᵃ, :φᶜᶜᵃ, :Azᶜᶜᵃ , :λᶠᶠᵃ, :φᶠᶠᵃ, :Azᶠᶠᵃ)
LXs = (:Center, :Center, :Center, :Face, :Face, :Face )
LYs = (:Center, :Center, :Center, :Face, :Face, :Face )

for (field, LX, LY) in zip(fields, LXs, LYs)
expr = quote
$(Symbol(field)) = Field{$(Symbol(LX)), $(Symbol(LY)), Nothing}($(grid))

for region in 1:number_of_regions($(grid))
getregion($(Symbol(field)), region).data .= getregion($(grid), region).$(Symbol(field))
end

if $(horizontal_topology) == FullyConnected
fill_halo_regions!($(Symbol(field)))
end
λᶜᶜᵃ = Field((Center, Center, Nothing), grid)
φᶜᶜᵃ = Field((Center, Center, Nothing), grid)
Azᶜᶜᵃ = Field((Center, Center, Nothing), grid)
λᶠᶠᵃ = Field((Face, Face, Nothing), grid)
φᶠᶠᵃ = Field((Face, Face, Nothing), grid)
Azᶠᶠᵃ = Field((Face, Face, Nothing), grid)

for (field, name) in zip(( λᶜᶜᵃ, φᶜᶜᵃ, Azᶜᶜᵃ, λᶠᶠᵃ, φᶠᶠᵃ, Azᶠᶠᵃ),
(:λᶜᶜᵃ, :φᶜᶜᵃ, :Azᶜᶜᵃ, :λᶠᶠᵃ, :φᶠᶠᵃ, :Azᶠᶠᵃ))

for region in 1:number_of_regions(grid)
getregion(field, region).data .= getproperty(getregion(grid, region), name)
end

for region in 1:number_of_regions($(grid))
getregion($(grid), region).$(Symbol(field)) .= getregion($(Symbol(field)), region).data
end
end # quote
if horizontal_topology == FullyConnected
fill_halo_regions!(field)
end

eval(expr)
for region in 1:number_of_regions(grid)
getproperty(getregion(grid, region), name) .= getregion(field, region).data
end
end

fields₁ = (:Δxᶜᶜᵃ, :Δxᶠᶜᵃ, :Δyᶠᶜᵃ, :λᶠᶜᵃ, :φᶠᶜᵃ, :Azᶠᶜᵃ , :Δxᶠᶠᵃ)
LXs₁ = (:Center, :Face, :Face, :Face, :Face, :Face , :Face )
LYs₁ = (:Center, :Center, :Center, :Center, :Center, :Center, :Face )

fields₂ = (:Δyᶜᶜᵃ, :Δyᶜᶠᵃ, :Δxᶜᶠᵃ, :λᶜᶠᵃ, :φᶜᶠᵃ, :Azᶜᶠᵃ , :Δyᶠᶠᵃ)
LXs₂ = (:Center, :Center, :Center, :Center, :Center, :Center, :Face )
LYs₂ = (:Center, :Face, :Face, :Face, :Face, :Face , :Face )

for (field₁, LX₁, LY₁, field₂, LX₂, LY₂) in zip(fields₁, LXs₁, LYs₁, fields₂, LXs₂, LYs₂)
expr = quote
$(Symbol(field₁)) = Field{$(Symbol(LX₁)), $(Symbol(LY₁)), Nothing}($(grid))
$(Symbol(field₂)) = Field{$(Symbol(LX₂)), $(Symbol(LY₂)), Nothing}($(grid))

for region in 1:number_of_regions($(grid))
getregion($(Symbol(field₁)), region).data .= getregion($(grid), region).$(Symbol(field₁))
getregion($(Symbol(field₂)), region).data .= getregion($(grid), region).$(Symbol(field₂))
end

if $(horizontal_topology) == FullyConnected
fill_halo_regions!(($(Symbol(field₁)), $(Symbol(field₂))); signed = false)
end
Δxᶜᶜᵃ = Field((Center, Center, Nothing), grid)
Δxᶠᶜᵃ = Field((Face, Center, Nothing), grid)
Δyᶠᶜᵃ = Field((Face, Center, Nothing), grid)
λᶠᶜᵃ = Field((Face, Center, Nothing), grid)
φᶠᶜᵃ = Field((Face, Center, Nothing), grid)
Azᶠᶜᵃ = Field((Face, Center, Nothing), grid)
Δxᶠᶠᵃ = Field((Face, Face, Nothing), grid)

fields₁ = ( Δxᶜᶜᵃ, Δxᶠᶜᵃ, Δyᶠᶜᵃ, λᶠᶜᵃ, φᶠᶜᵃ, Azᶠᶜᵃ , Δxᶠᶠᵃ)
names₁ = (:Δxᶜᶜᵃ, :Δxᶠᶜᵃ, :Δyᶠᶜᵃ, :λᶠᶜᵃ, :φᶠᶜᵃ, :Azᶠᶜᵃ , :Δxᶠᶠᵃ)

Δyᶜᶜᵃ = Field((Center, Center, Nothing), grid)
Δyᶜᶠᵃ = Field((Center, Face, Nothing), grid)
Δxᶜᶠᵃ = Field((Center, Face, Nothing), grid)
λᶜᶠᵃ = Field((Center, Face, Nothing), grid)
φᶜᶠᵃ = Field((Center, Face, Nothing), grid)
Azᶜᶠᵃ = Field((Center, Face, Nothing), grid)
Δyᶠᶠᵃ = Field((Face, Face, Nothing), grid)

fields₂ = ( Δyᶜᶜᵃ, Δyᶜᶠᵃ, Δxᶜᶠᵃ, λᶜᶠᵃ, φᶜᶠᵃ, Azᶜᶠᵃ , Δyᶠᶠᵃ)
names₂ = (:Δyᶜᶜᵃ, :Δyᶜᶠᵃ, :Δxᶜᶠᵃ, :λᶜᶠᵃ, :φᶜᶠᵃ, :Azᶜᶠᵃ , :Δyᶠᶠᵃ)

for (field₁, field₂, name₁, name₂) in zip(fields₁, fields₂, names₁, names₂)
for region in 1:number_of_regions(grid)
getregion(field₁, region).data .= getproperty(getregion(grid, region), name₁)
getregion(field₂, region).data .= getproperty(getregion(grid, region), name₂)
end

for region in 1:number_of_regions($(grid))
getregion($(grid), region).$(Symbol(field₁)) .= getregion($(Symbol(field₁)), region).data
getregion($(grid), region).$(Symbol(field₂)) .= getregion($(Symbol(field₂)), region).data
end
end # quote
if horizontal_topology == FullyConnected
fill_halo_regions!(field₁, field₂; signed = false)
end

eval(expr)
for region in 1:number_of_regions(grid)
getproperty(getregion(grid, region), name₁) .= getregion(field₁, region).data
getproperty(getregion(grid, region), name₂) .= getregion(field₂, region).data
end
end

###################################################
Expand Down
2 changes: 1 addition & 1 deletion src/MultiRegion/multi_region_models.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const MultiRegionModel = HydrostaticFreeSurfaceModel{<:Any, <:Any, <:AbstractArc

# Utility to generate the inputs to complex `getregion`s
function getregionalproperties(T, inner=true)
type = eval(T)
type = getglobal(@__MODULE__, T)
names = fieldnames(type)
args = Vector(undef, length(names))
for (n, name) in enumerate(names)
Expand Down
6 changes: 3 additions & 3 deletions src/Operators/interpolation_utils.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ function interpolation_operator(from, to)
global identity_counter += 1
identity = identify_an_identity(identity_counter)

return @eval $identity
return getglobal(@__MODULE__, identity)
else
return eval(Symbol(:ℑ, ℑxsym(x), ℑysym(y), ℑzsym(z), x, y, z))
return getglobal(@__MODULE__, Symbol(:ℑ, ℑxsym(x), ℑysym(y), ℑzsym(z), x, y, z))
end
end

Expand All @@ -77,7 +77,7 @@ operator for fields that have no intrinsic location, like numbers or functions.
function interpolation_operator(::Nothing, to)
global identity_counter += 1
identity = identify_an_identity(identity_counter)
return @eval $identity
return getglobal(@__MODULE__, identity)
end

assumed_field_location(name) = name === :u ? (Face, Center, Center) :
Expand Down
3 changes: 2 additions & 1 deletion src/TimeSteppers/TimeSteppers.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ julia> stepper = TimeStepper(:QuasiAdamsBashforth2, CPU(), grid, tracernames)
"""
function TimeStepper(name::Symbol, args...; kwargs...)
fullname = Symbol(name, :TimeStepper)
return @eval $fullname($args...; $kwargs...)
TS = getglobal(@__MODULE__, fullname)
return TS(args...; kwargs...)
end

# Fallback
Expand Down

0 comments on commit 7f2d512

Please sign in to comment.