Skip to content
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.

Sign up for GitHub

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

Jump to bottom

avoid assigning storage for nothing situation #62

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

avoid assigning storage for nothing situation #62

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c04d1e3
avoid assigning storage for `nothing` situation
Sep 28, 2024
1cfbb81
call function once, avoid sign array
Sep 30, 2024
f28c9ed
move for loop down one level
Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/Denudation/Abstract.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract type DenudationType end
FIXME Computes the denudation for a single time-step, given denudation parameters `param` and a simulation state `state`. `param` should have a `DenudationType` type and `state` should contain the `height` property and `sealevel`.
"""
function denudation(input)
denudation_mass::Array{typeof(1.0u"m/kyr"),2} = Array{typeof(1.0u"m/kyr")}(undef, input.box.grid_size...)
denudation_mass::Union{Array{typeof(1.0u"m/kyr"),2}, Nothing} = Array{typeof(1.0u"m/kyr")}(undef, input.box.grid_size...)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're trying to prevent unneeded memory allocations. Here, what you're saying is: I have this variable denudation_mass which is either an array or nothing, and then you assign a big empty array to it. That doesn't make much sense.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right I can initialise with nothing...


function (state, water_depth, slope)
for idx in CartesianIndices(state.ca)
Expand All @@ -24,7 +24,13 @@ function denudation(input)
end

if water_depth[idx] >= 0
(denudation_mass[idx]) = denudation(input.box, input.denudation, water_depth[idx], slope[idx], input.facies[f])

if denudation(input.box, input.denudation, water_depth[idx], slope[idx], input.facies[f]) !== nothing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're calling this function twice here. In many cases the compiler will optimize this away, but since it is quite a long call, it is more hygienic to write it differently.

(denudation_mass[idx]) = denudation(input.box, input.denudation, water_depth[idx], slope[idx], input.facies[f])
else
denudation_mass = nothing
end

end
end
return denudation_mass
Expand All @@ -46,9 +52,13 @@ end
FIXME
"""
function redistribution(input)
redistribution_mass::Array{typeof(1.0u"m/kyr"),2} = Array{typeof(1.0u"m/kyr")}(undef, input.box.grid_size...)
redistribution_mass::Union{Array{typeof(1.0u"m/kyr"),2}, Nothing} = Array{typeof(1.0u"m/kyr")}(undef, input.box.grid_size...)
function (state, water_depth, denudation_mass)
if redistribution(input.box, input.denudation, denudation_mass, water_depth) !== nothing
redistribution_mass = redistribution(input.box, input.denudation, denudation_mass, water_depth)
else
redistribution_mass = nothing
end
return redistribution_mass
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/Denudation/DissolutionMod.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ function denudation(::Box{BT}, p::Dissolution, water_depth, slope, facies) where
end

function redistribution(box::Box{BT}, p::Dissolution, denudation_mass, water_depth) where {BT<:Boundary}
return denudation_mass .* 0
return nothing
end

end
Expand Down
2 changes: 1 addition & 1 deletion src/Denudation/EmpiricalDenudationMod.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function denudation(::Box, p::EmpiricalDenudation, water_depth, slope, facies)
end

function redistribution(::Box, p::EmpiricalDenudation, denudation_mass, water_depth)
return denudation_mass .* 0
return nothing
end

end
Expand Down
2 changes: 1 addition & 1 deletion src/Denudation/NoDenudationMod.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function denudation(box::Box, p::NoDenudation, water_depth::Any, slope, facies)
end

function redistribution(box::Box, p::NoDenudation, denudation_mass, water_depth)
return denudation_mass .* 0
return nothing
end

end
Expand Down
43 changes: 31 additions & 12 deletions src/Model/WithDenudation.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ end

struct OutputFrame
production::Array{typeof(1.0u"m/Myr"),3}
denudation::Array{typeof(1.0u"m/Myr"),2}
redistribution::Array{typeof(1.0u"m/Myr"),2}
denudation::Union{Array{typeof(1.0u"m/Myr"),2},Nothing}
redistribution::Union{Array{typeof(1.0u"m/Myr"),2},Nothing}
end
# FIXME: deduplicate
mutable struct State
Expand Down Expand Up @@ -138,8 +138,12 @@ function updater(input)

function update(state, Δ::DenudationFrame)
# FIXME: implement
if Δ.denudation !== nothing
state.height .+= Δ.denudation .* input.time.Δt
end
if Δ.redistribution !== nothing
state.height .-= Δ.redistribution .* input.time.Δt
end
state.time += input.time.Δt
end

Expand Down Expand Up @@ -187,24 +191,39 @@ function main(input::Input, output::String)
attr["subsidence_rate"] = input.subsidence_rate |> in_units_of(u"m/Myr")
println("Subsidence rate saved successfully.")

box = input.box
results = map(stack_frames, partition(run_model(input, box), input.time.write_interval))
testframe = first(results)

n_facies = length(input.facies)
ds = create_dataset(fid, "sediment", datatype(Float64),
dataspace(input.box.grid_size..., n_facies, input.time.steps),
chunk=(input.box.grid_size..., n_facies, 1))
denudation = create_dataset(fid, "denudation", datatype(Float64),
dataspace(input.box.grid_size..., input.time.steps),
chunk=(input.box.grid_size..., 1))
redistribution = create_dataset(fid, "redistribution", datatype(Float64),
dataspace(input.box.grid_size..., input.time.steps),
chunk=(input.box.grid_size..., 1))
box = input.box

results = map(stack_frames, partition(run_model(input, box), input.time.write_interval))
denudation = nothing
if testframe.denudation !== nothing
denudation = create_dataset(fid, "denudation", datatype(Float64),
dataspace(input.box.grid_size..., input.time.steps),
chunk=(input.box.grid_size..., 1))
end

redistribution = nothing
if testframe.redistribution !== nothing
redistribution = create_dataset(fid, "redistribution", datatype(Float64),
dataspace(input.box.grid_size..., input.time.steps),
chunk=(input.box.grid_size..., 1))
end

for (step, frame) in enumerate(take(results, n_writes))
ds[:, :, :, step] = frame.production |> in_units_of(u"m/Myr")
denudation[:, :, step] = frame.denudation |> in_units_of(u"m/kyr")
redistribution[:, :, step] = frame.redistribution |> in_units_of(u"m/kyr")

if denudation !== nothing
denudation[:, :, step] = frame.denudation |> in_units_of(u"m/kyr")
end

if redistribution !== nothing
redistribution[:, :, step] = frame.redistribution |> in_units_of(u"m/kyr")
end
end
end
end
Expand Down
Loading