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.

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

Fixing boundary checks on masked grids #114

Merged
merged 41 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4a92d6e
Trying masked OutwardsDispersal
nicolasmerino41 Jun 13, 2024
ca57619
Update outwards.jl
nicolasmerino41 Jun 13, 2024
77fd5e1
Update index.md
nicolasmerino41 Jun 13, 2024
aa7ef4e
Update outwards
nicolasmerino41 Jun 13, 2024
dd042d8
Update outwards.jl
nicolasmerino41 Jun 13, 2024
f6c5719
Update outwards.jl
nicolasmerino41 Jun 13, 2024
aa7f88c
Update outwards.jl
nicolasmerino41 Jun 13, 2024
b8be33f
Update src/kernel/outwards.jl
nicolasmerino41 Jun 13, 2024
5dd7c2c
Update src/kernel/outwards.jl
nicolasmerino41 Jun 13, 2024
a6876ba
Update outwards.jl
nicolasmerino41 Jun 13, 2024
8595100
Irrelevant update
nicolasmerino41 Jun 13, 2024
be564b8
Update outwards.jl
nicolasmerino41 Jun 13, 2024
5144f92
Update outwards.jl
nicolasmerino41 Jun 13, 2024
8bcfefb
Update outwards.jl
nicolasmerino41 Jun 13, 2024
f7538d6
Update outwards.jl
nicolasmerino41 Jun 13, 2024
9ae4999
Updated OutwardsDispersal struct
nicolasmerino41 Jun 13, 2024
84508de
Update src/kernel/outwards.jl
nicolasmerino41 Jun 14, 2024
211ccb4
Update outwards and testing
nicolasmerino41 Jun 14, 2024
bd5098d
Small typo fix
nicolasmerino41 Jun 14, 2024
0e921bf
Update src/kernel/outwards.jl
nicolasmerino41 Jun 17, 2024
6850dc2
If/else block for mask identification
nicolasmerino41 Jun 17, 2024
d7eff3b
If/else mask identification
nicolasmerino41 Jun 17, 2024
1af845a
remove target
nicolasmerino41 Jun 17, 2024
607885d
Update outwards.jl
nicolasmerino41 Jun 17, 2024
00421ae
Back to old version
nicolasmerino41 Jun 17, 2024
330ff3f
Update outwards.jl
nicolasmerino41 Jun 17, 2024
e112673
Update outwards.jl
nicolasmerino41 Jun 17, 2024
c0d7394
Update outwards.jl
nicolasmerino41 Jun 17, 2024
25fd24e
Update outwards.jl
nicolasmerino41 Jun 17, 2024
1f50575
Update outwards.jl
nicolasmerino41 Jun 17, 2024
d9e034b
Update outwards.jl
nicolasmerino41 Jun 17, 2024
2001258
Update outwards.jl
nicolasmerino41 Jun 17, 2024
e268d79
Update outwards.jl
nicolasmerino41 Jun 17, 2024
f5e4777
Update outwards.jl
nicolasmerino41 Jun 17, 2024
0aeefc0
Performance fix
nicolasmerino41 Jun 17, 2024
2777e96
Update outwards.jl
nicolasmerino41 Jun 17, 2024
ad744de
Update outwards.jl
nicolasmerino41 Jun 17, 2024
ce4d5bc
Irrelevant
nicolasmerino41 Jun 18, 2024
68f045b
Back to slow bound-checking
nicolasmerino41 Jun 18, 2024
03ff760
Update doc
nicolasmerino41 Jun 18, 2024
421f465
Renaming
nicolasmerino41 Jun 19, 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
*.jl.*.cov
*.jl.mem
docs/build
*.json
Manifest.toml
2 changes: 2 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ DynamicGrids = "a5dba43e-3abc-5203-bfc5-584ca68d3f5b"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
Reexport = "189a3867-3050-52da-a836-e630ba90ab69"
Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
Stencils = "264155e8-78a8-466a-aa59-c9b28c34d21a"

[compat]
Distributions = "0.21, 0.22, 0.23, 0.24, 0.25"
DocStringExtensions = "0.8, 0.9"
DynamicGrids = "0.21"
Reexport = "0.2, 1.0"
Stencils = "0.3"
julia = "1"

[extras]
Expand Down
1 change: 1 addition & 0 deletions src/Dispersal.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ using DynamicGrids.ModelParameters
using DynamicGrids.Setfield
using DynamicGrids.Stencils
using DynamicGrids.StaticArrays
using Stencils

using DynamicGrids.ModelParameters: params

Expand Down
39 changes: 33 additions & 6 deletions src/kernel/outwards.jl
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,50 @@ is occupied.
Default is 1.0.
- `distancemethod`: [`DistanceMethod`](@ref) object for calculating distance between cells.
The default is [`CentroidToCentroid`](@ref).
- `mask_flag`: The default setting is `NoMask()`. Use `Mask()` to indicate that the grid is
masked, enabling the rule to perform boundary checking at mask edges. Not using
`Mask()` on a masked grid may result in the loss of individuals at the edges, but it comes
at a performance cost.

Pass grid name `Symbol`s to `R` and `W` type parameters to use specific grids.
"""
struct OutwardsDispersal{R,W,S<:Stencils.AbstractKernelStencil} <: SetNeighborhoodRule{R,W}

struct Mask end
struct NoMask end
Copy link
Member

@rafaqz rafaqz Jun 19, 2024

Choose a reason for hiding this comment

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

Suggested change
struct Mask end
struct NoMask end
struct AvoidMasked end
struct LossToMasked end

Ideas for more descriptive names, now that everything works


struct OutwardsDispersal{R,W,S<:Stencils.AbstractKernelStencil, M} <: SetNeighborhoodRule{R,W}
stencil::S
mask_flag::M
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mask_flag::M
maskbehavior::M

end

# Constructors for OutwardsDispersal
function OutwardsDispersal{R,W}(stencil::S; mask_flag::Union{Mask, NoMask}=NoMask()) where {R,W,S<:Stencils.AbstractKernelStencil}
OutwardsDispersal{R,W,S,typeof(mask_flag)}(stencil, mask_flag)
end
function OutwardsDispersal{R,W}(; kw...) where {R,W}
OutwardsDispersal{R,W}(DispersalKernel(; kw...))

function OutwardsDispersal{R,W}(; mask_flag::Union{Mask, NoMask}=NoMask(), kw...) where {R,W}
stencil = DispersalKernel(; kw...)
OutwardsDispersal{R,W,typeof(stencil),typeof(mask_flag)}(stencil, mask_flag)
end

@inline function applyrule!(data, rule::OutwardsDispersal{R,W}, N, I) where {R,W}
N == zero(N) && return nothing

# Check if the current cell is masked, skip if it is
mask_data = if rule.mask_flag === NoMask() nothing else DynamicGrids.mask(data) end
nicolasmerino41 marked this conversation as resolved.
Show resolved Hide resolved
if !isnothing(mask_data) && !mask_data[I...]
return nothing
end

sum = zero(N)
for (offset, k) in zip(offsets(rule), kernel(rule))
@inbounds propagules = N * k
@inbounds add!(data[W], propagules, I .+ offset...)
sum += propagules
target = I .+ offset
(target_mod, inbounds) = DynamicGrids.inbounds(data, target)
Copy link
Member

Choose a reason for hiding this comment

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

Ok this check is likely the extra cost.

We can reorganise the if block below so that this only runs when we are checking the mask. Then we should be back to the original performance without the mask check.

if inbounds && (isnothing(mask_data) || mask_data[target_mod...])
@inbounds propagules = N * k
@inbounds add!(data[W], propagules, target_mod...)
sum += propagules
end
end
@inbounds sub!(data[W], sum, I...)
return nothing
Expand Down
44 changes: 44 additions & 0 deletions test/integration.jl
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,47 @@ end
end

end

@testset "OutwardsDispersal Test" begin
# Define a mask
mask_data = [i == 1 || i == 10 || j == 1 || j == 10 ? false : true for i in 1:10, j in 1:10]

# Create OutwardsDispersal with a mask
outdisp_with_mask = OutwardsDispersal(
formulation=ExponentialKernel(λ=0.0125),
distancemethod=AreaToArea(30),
mask_flag=Mask()
)

# Create OutwardsDispersal without a mask, NoMask is default
outdisp_without_mask = OutwardsDispersal(
formulation=ExponentialKernel(λ=0.0125),
distancemethod=AreaToArea(30)
)

# Create a grid with empty borders matching the mask
init = map(x -> x ? 100.0 : 0.0, mask_data)

# Create ruleset and outputs
rule_with_mask = Ruleset(outdisp_with_mask; boundary=Reflect())
rule_without_mask = Ruleset(outdisp_without_mask; boundary=Reflect())

# Run the simulation with a mask
output_with_mask = ArrayOutput(init; tspan=1:1000, mask=mask_data)
a = sim!(output_with_mask, rule_with_mask)

@test sum(a[1]) ≈ sum(a[1000]) # Floating error should be smaller than 1.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@test sum(a[1]) sum(a[1000]) # Floating error should be smaller than 1.0
@test sum(a[1]) sum(a[1000]) # Floating error should be close to zero


# Run the simulation without a mask to check default works fine
output_without_mask = ArrayOutput(init; tspan=1:1000)
b = sim!(output_without_mask, rule_without_mask)

@test sum(b[1]) ≈ sum(b[1000]) # Floating error should be smaller than 1.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@test sum(b[1]) sum(b[1000]) # Floating error should be smaller than 1.0
@test sum(b[1]) sum(b[1000]) # Floating error should be close to zero


# Run the simulation with a mask but outdisp_without_mask
output_without_mask = ArrayOutput(init; tspan=1:1000, mask=mask_data)
b = sim!(output_without_mask, rule_without_mask)

@test sum(b[1]) > sum(b[1000]) #= Floating error should be larger than 1.0
because this does not identify the mask properly =#
end