Skip to content
1 change: 1 addition & 0 deletions examples/example_config_files/ivc_inactivelayer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ grid:
boundaries:
left: inf
right: inf
spacing_surface_refinement: [1e-4, 1e-4, 1e-4] #m
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's here semantically, I would also add this to the World that we create from it and save it to sim.world.

Not necessarily for this PR, but we could think about also allowing to put things like the refinement limits, max/min tick distance etc. in here, so that they do not have to be passed at runtime to the calculate_potential! methods.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

medium: vacuum
detectors:
- semiconductor:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ grid:
r:
to: 9
boundaries: inf
y:
phi:
from: 0
to: 0
boundaries: periodic
Expand Down
2 changes: 1 addition & 1 deletion examples/example_config_files/test_detector.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ grid:
r:
to: 9
boundaries: inf
y:
phi:
from: 0
to: 180
boundaries: periodic
Expand Down
73 changes: 73 additions & 0 deletions src/PotentialCalculation/Refinement.jl
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,76 @@ end

_extend_refinement_limits(rl::Real) = (rl, rl, rl )
_extend_refinement_limits(rl::Tuple{<:Real,<:Real,<:Real}) = rl

@inline function has_surface_points(slice::AbstractArray{PointType})::Bool
return any(is_in_inactive_layer, slice)
end

function _refine_axis_surface( ax::DiscreteAxis{T, <:Any, <:Any, ClosedInterval{T}}, surface_intervals::AbstractVector{Bool}, min_spacing::T;
extra_before::Int = 5, # intervals to refine before first surface interval
extra_after::Int = 5 # intervals to refine after last surface interval
) where {T}

old_ticks = ax.ticks
n_int = length(surface_intervals)

# Find first and last surface intervals
first_surface = findfirst(surface_intervals)
last_surface = findlast(surface_intervals)

(isnothing(first_surface) || isnothing(last_surface)) && return ax

# Create flags for all intervals to refine
refine_flags = copy(surface_intervals)

# Add extra intervals before first surface interval
start_idx = max(first_surface - extra_before, 1)
refine_flags[start_idx:first_surface-1] .= true

# Add extra intervals after last surface interval
end_idx = min(last_surface + extra_after, n_int)
refine_flags[last_surface+1:end_idx] .= true

# Merge consecutive intervals to refine
merged = Vector{UnitRange{Int}}()
i = 1
while i <= n_int
if refine_flags[i]
j = i
while j < n_int && refine_flags[j+1]
j += 1
end
push!(merged, i:j)
i = j + 1
else
i += 1
end
end

# Compute number of points to add per interval
ns = zeros(Int, n_int)
for r in merged
for i in r
Δ = old_ticks[i+1] - old_ticks[i]
if Δ > min_spacing
ns[i] = ceil(Int, Δ/min_spacing) - 1
end
end
end

# Subdivide intervals
sub_widths = [(old_ticks[i+1] - old_ticks[i]) / (ns[i]+1) for i in 1:n_int]
ticks = Vector{T}(undef, length(old_ticks) + sum(ns))
i_tick = 1
for j in 1:n_int
ticks[i_tick] = old_ticks[j]
for k in 1:ns[j]
i_tick += 1
ticks[i_tick] = old_ticks[j] + k*sub_widths[j]
end
i_tick += 1
end
ticks[end] = old_ticks[end]
Comment on lines +236 to +246
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any scenario in which some values in ticks might remain undef?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don’t think so, the loop structure guarantees that all entries are written, this just follows the same pattern as _refine_axis function


return typeof(ax)(ax.interval, ticks)
end
51 changes: 32 additions & 19 deletions src/PotentialCalculation/SuccessiveOverRelaxation/CPU_innerloop.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
pwΔmp2r, pwΔmp2l,
pwΔmp3r, pwΔmp3l,
) where {T, S}


# pww1r = pcs.geom_weights[3][1, in1]
pww1r = geom_weights_3[1, in1]
pww1l = geom_weights_3[2, in1]
pwΔmp1 = geom_weights_3[3, in1]
Δ1_ext_inv_l = geom_weights_3[4, in1]
Δ1_ext_inv_r = geom_weights_3[4, in1 + 1]
Δ1_ext_inv_r = in1 < size(geom_weights_3, 2) ? geom_weights_3[4, in1 + 1] : geom_weights_3[4, in1]

# ϵ_ijk: i: 1.RB-Dim. | j: 2.RB-Dim. | k: 3.RB-Dim.
ϵ_lll, ϵ_llr, ϵ_lrl, ϵ_lrr, ϵ_rll, ϵ_rlr, ϵ_rrl, ϵ_rrr = get_ϵ_of_oktant(
Expand Down Expand Up @@ -168,36 +167,50 @@ end
ϵ_r::AbstractArray{T, 3}, ::Type{Cylindrical},
i1, in1, i2, in2, i3, in3
) where {T}
# Cylindrical: inner loop over z (3rd dimension)
i1_safe = clamp(i1, 1, size(ϵ_r, 3))
in1_safe = clamp(in1, 1, size(ϵ_r, 3))
i2_safe = clamp(i2, 1, size(ϵ_r, 2))
in2_safe = clamp(in2, 1, size(ϵ_r, 2))
i3_safe = clamp(i3, 1, size(ϵ_r, 1))
in3_safe = clamp(in3, 1, size(ϵ_r, 1))
# ϵ_r is not transformed into an red-black-4D-array.
# The inner loop (over i1) is along the z-Dimension (Cylindrical Case),
# which is the 3rd dimension for Cylindrical coordinates: (r, φ, z)
return @inbounds begin
ϵ_r[ in3, in2, in1 ],
ϵ_r[ i3, in2, in1 ],
ϵ_r[ in3, i2, in1 ],
ϵ_r[ i3, i2, in1 ],
ϵ_r[ in3, in2, i1 ],
ϵ_r[ i3, in2, i1 ],
ϵ_r[ in3, i2, i1 ],
ϵ_r[ i3, i2, i1 ]
ϵ_r[in3_safe, in2_safe, in1_safe],
ϵ_r[i3_safe, in2_safe, in1_safe],
ϵ_r[in3_safe, i2_safe, in1_safe],
ϵ_r[i3_safe, i2_safe, in1_safe],
ϵ_r[in3_safe, in2_safe, i1_safe],
ϵ_r[i3_safe, in2_safe, i1_safe],
ϵ_r[in3_safe, i2_safe, i1_safe],
ϵ_r[i3_safe, i2_safe, i1_safe]
Comment on lines +170 to +188
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed now? Was it needed before already (is this a bug fix)?
Or is this something required now with the things you have added in this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This bit of the code was giving a bound error when adding more grid points around the surface layer. When refining the grid near the surface, some index would fall just outside the valid array range. Previously this did not show up because the grid configurations we used did not hit these boundary cases, but with the refinement introduced in this PR it can occur and leads to a BoundsError. So the clamping would act as a safeguard to keep the indices within the range. In that sense this is effectively a bug fix for boundary handling, but it only became visible with the refinement changes added in this PR

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to work with the extended grid then, to also keep boundary conditions fulfilled?

function get_extended_ticks( ax::DiscreteAxis{T, :reflecting, :reflecting} )::Vector{T} where {T}
ticks_ext::Vector{T} = Array{T}(undef, length(ax.ticks) + 2)
ticks_ext[2:end-1] = ax.ticks
set_periodic_boundary_ticks!(ticks_ext, ax.interval)
return ticks_ext
end
function get_extended_ticks( ax::DiscreteAxis{T, :fixed, :reflecting} )::Vector{T} where {T}
ticks_ext::Vector{T} = Array{T}(undef, length(ax.ticks) + 2)
ticks_ext[2:end-1] = ax.ticks
set_periodic_boundary_ticks!(ticks_ext, ax.interval)
return ticks_ext
end
function get_extended_ticks( ax::DiscreteAxis{T, :reflecting, :fixed} )::Vector{T} where {T}
ticks_ext::Vector{T} = Array{T}(undef, length(ax.ticks) + 2)
ticks_ext[2:end-1] = ax.ticks
set_periodic_boundary_ticks!(ticks_ext, ax.interval)
return ticks_ext
end
function get_extended_ticks( ax::DiscreteAxis{T, :periodic, :periodic} )::Vector{T} where {T}
ticks_ext::Vector{T} = Array{T}(undef, length(ax.ticks) + 2)
ticks_ext[2:end-1] = ax.ticks
set_periodic_boundary_ticks!(ticks_ext, ax.interval)
return ticks_ext
end
function get_extended_ticks( ax::DiscreteAxis{T, :infinite, :infinite} )::Vector{T} where {T}
ticks_ext::Vector{T} = Array{T}(undef, length(ax.ticks) + 2)
ticks_ext[2:end-1] = ax.ticks
Δ::T = 1 * (ticks_ext[end-1] - ticks_ext[2])
ticks_ext[1] = ticks_ext[2] - Δ
ticks_ext[end] = ticks_ext[end - 1] + Δ
return ticks_ext
end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This looks like a better approach. I’ll check whether we can use the extended grid here instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

get_extended_ticks is already used in ϵ_r and all the approaches I tried to fix this issue didn’t work but clamp only affects cases where the index would go out of bounds, so for example the very refined grid of 1e-5m, for all usual cases that previously ran without errors, it changes nothing, so I think it’s a safe fix for very fine grids or edge conditions

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just by looking at the code, I don't fully understand this?
Can you clarify in the most simple terms why we need this clamping? When refining surface intervals close to the boundaries of the world? If so, why don't we care about the boundary conditions applied to the boundaries of the world?

end
end

@inline function get_ϵ_of_oktant(
ϵ_r::AbstractArray{T, 3}, ::Type{Cartesian},
i1, in1, i2, in2, i3, in3
) where {T}
# Cartesian: inner loop over x (1st dimension)
i1_safe = clamp(i1, 1, size(ϵ_r, 1))
in1_safe = clamp(in1, 1, size(ϵ_r, 1))
i2_safe = clamp(i2, 1, size(ϵ_r, 2))
in2_safe = clamp(in2, 1, size(ϵ_r, 2))
i3_safe = clamp(i3, 1, size(ϵ_r, 3))
in3_safe = clamp(in3, 1, size(ϵ_r, 3))
# The inner loop (over i1) is along the x-Dimension (Cartesian Case),
# which is the 1rd dimension for Cartesian coordinates: (x, y, z)
return @inbounds begin
ϵ_r[ in1, in2, in3 ],
ϵ_r[ in1, in2, i3 ],
ϵ_r[ in1, i2, in3 ],
ϵ_r[ in1, i2, i3 ],
ϵ_r[ i1, in2, in3 ],
ϵ_r[ i1, in2, i3 ],
ϵ_r[ i1, i2, in3 ],
ϵ_r[ i1, i2, i3 ]
ϵ_r[in1_safe, in2_safe, in3_safe],
ϵ_r[in1_safe, in2_safe, i3_safe],
ϵ_r[in1_safe, i2_safe, in3_safe],
ϵ_r[in1_safe, i2_safe, i3_safe],
ϵ_r[ i1_safe, in2_safe, in3_safe],
ϵ_r[ i1_safe, in2_safe, i3_safe],
ϵ_r[ i1_safe, i2_safe, in3_safe],
ϵ_r[ i1_safe, i2_safe, i3_safe]
Comment on lines +196 to +213
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I still need to implement this

end
end

Expand Down Expand Up @@ -242,4 +255,4 @@ end
np::NTuple{6, T}, ::Type{Cartesian}, i::Int
) where {T}
return np
end
end
Loading
Loading