-
Notifications
You must be signed in to change notification settings - Fork 3
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
Changes from 40 commits
4a92d6e
ca57619
77fd5e1
aa7ef4e
dd042d8
f6c5719
aa7f88c
b8be33f
5dd7c2c
a6876ba
8595100
be564b8
5144f92
8bcfefb
f7538d6
9ae4999
84508de
211ccb4
bd5098d
0e921bf
6850dc2
d7eff3b
1af845a
607885d
00421ae
330ff3f
e112673
c0d7394
25fd24e
1f50575
d9e034b
2001258
e268d79
f5e4777
0aeefc0
2777e96
ad744de
ce4d5bc
68f045b
03ff760
421f465
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,5 @@ | |
*.jl.*.cov | ||
*.jl.mem | ||
docs/build | ||
*.json | ||
Manifest.toml |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
|
||||||
struct OutwardsDispersal{R,W,S<:Stencils.AbstractKernelStencil, M} <: SetNeighborhoodRule{R,W} | ||||||
stencil::S | ||||||
mask_flag::M | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
# 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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideas for more descriptive names, now that everything works