-
Notifications
You must be signed in to change notification settings - Fork 4
Description
Summary
Address issues and inconsistencies related to the is_closed_toroidal parameter in the vacuum code, as discussed in PR #126 ("Cleaning up kernel function").
Background
The recent refactoring of the kernel! function in the vacuum code (PR #126) highlighted questions and potential problems with the use and propagation of the is_closed_toroidal parameter. Ensuring this parameter is handled properly is essential for correct vacuum region modeling, especially as the code evolves.
Key Issues from PR #126 Discussion
-
Hard-coded value in
kernel!function: Currentlyis_closed_toroidal = trueis hard-coded inVacuumInternals. jl: 246, preventing the use of open toroidal walls even though the parameter exists in theWallGeometrystruct. -
Parameter not passed to
kernel!: TheWallGeometrystruct has anis_closed_toroidalfield (defaulttrue), but this value is never passed to thekernel!function where it's needed for residue calculations. -
Residue calculation depends on wall type: The residue calculation uses different formulas for closed vs. open toroidal walls (Chance 1997 eq. 89 vs. eq. 90), but currently only the closed case is implemented.
-
Unclear support status: The discussion in PR Cleaning up kernel function #126 revealed uncertainty about whether open walls should be supported, with @logan-nc noting that "we only support closed toroidal walls" but the flag was left in to indicate where changes would be needed.
Relevant Code Locations
1. Hard-coded value in kernel! (VacuumInternals. jl: 244-257)
# Set residue based on logic similar to Table I of Chance 1997 + existing δⱼᵢ in eq. 69
# Would need to pass in wall geometry to generalize this to open walls
is_closed_toroidal = true2. Residue calculation using the parameter (VacuumInternals.jl:258-270)
if is_closed_toroidal
residue = (j1 == 2.0) ? 0.0 : (j2 == 1 ? 2.0 : -2.0) # Chance eq. 89
else
# TODO: this line can be gotten rid of if we are never doing open walls
residue = (j1 == j2) ? 2.0 : 0.0 # Chance eq. 90
end3. WallGeometry struct definition (VacuumStructs. jl:76-96)
@kwdef struct WallGeometry
nowall::Bool = true
is_closed_toroidal::Bool = true
x::Vector{Float64} = Float64[]
z::Vector{Float64} = Float64[]
dx_dtheta::Vector{Float64} = Float64[]
dz_dtheta::Vector{Float64} = Float64[]
end4. Usage in Vacuum.jl (line 267)
(abs(n) <= 1e-5 && !wall.nowall && wall.is_closed_toroidal) && begin
@warn "Adding $cn0 to diagonal of grdgre to regularize n=0 mode; this may affect accuracy of results."
...
end5. Equal arc length check (VacuumStructs. jl:336-353)
if wall_settings.equal_arc_wall && (wall_settings.shape != "nowall")
@info "Re-distributing wall points to equal arc length spacing"
if ! is_closed_toroidal
@error "Wall is not closed toroidally; equal arc length distribution assumes periodicity and cannot be safely used."
end
...
endProposed Solutions
Based on the PR #126 discussion, choose one of:
Option A: Remove support for open walls entirely
- Remove
is_closed_toroidalfield fromWallGeometrystruct - Remove the
elsebranch in the residue calculation - Add clear documentation stating only closed toroidal walls are supported
- Update docstrings for
kernel!and wall initialization functions
Option B: Properly implement open wall support
- Pass
wall. is_closed_toroidaltokernel!function - Remove hard-coded
is_closed_toroidal = trueline - Verify residue calculations for open walls (Chance 1997 eq. 90)
- Add tests for open wall cases
- Handle zero-crossing detection properly for open walls
Tasks
- Decide on Option A or B based on current and future use cases
- If Option A: Remove
is_closed_toroidalparameter and simplify code - If Option B: Pass parameter correctly and verify all calculations
- Update documentation (docstrings and web docs) to clearly state wall support
- Add or update tests covering the chosen approach
- Remove TODO comment once resolved
References
- PR Cleaning up kernel function #126 discussion: Cleaning up kernel function #126
- Relevant comments: #126 (comment), #126 (comment)
- Chance 1997 equations: eq. 89 (closed walls), eq. 90 (open walls)
- More search results for
is_closed_toroidal