Replace "Missing" as default value in Simulation
fhagemann opened this issue · 7 comments
The current implementation of Simulation{T}
looks like this:
mutable struct Simulation{T <: SSDFloat} <: AbstractSimulation{T}
detector::Union{SolidStateDetector{T}, Missing}
q_eff_imp::Union{EffectiveChargeDensity{T}, Missing} # Effective charge coming from the impurites of the semiconductors
q_eff_fix::Union{EffectiveChargeDensity{T}, Missing} # Fixed charge coming from fixed space charges, e.g. charged up surface layers
ϵ_r::Union{DielectricDistribution{T}, Missing}
point_types::Union{PointTypes{T}, Missing}
electric_potential::Union{ElectricPotential{T}, Missing}
weighting_potentials::Vector{Any}
electric_field::Union{ElectricField{T}, Missing}
charge_drift_model::Union{<:AbstractChargeDriftModel{T}, Missing}
electron_drift_field::Union{ElectricField{T}, Missing}
hole_drift_field::Union{ElectricField{T}, Missing}
end
with the default constructor being
function Simulation{T}() where {T <: SSDFloat}
Simulation{T}(
SolidStateDetector{T}(),
missing,
missing,
missing,
missing,
missing,
[missing],
missing,
ElectricFieldChargeDriftModel{T}(),
missing,
missing
)
end
I would be in favor of replacing missing
with something more type-stable. Why?
If simulation.electric_potential = missing
, then calling plot(simulation.electric_potential)
results in an error that missing
cannot be converted to Float64
to be plotted rather than an error message that simulation.electric_potential
is not yet calculated.
One way of doing this could be introducing new types, e.g. MissingPointTypes{T} <: PointTypes{T}
and changing to point_types::PointTypes{T}
. What would be your comments/thoughts on this?
This is essentially a defaulting mechanism, though, right? If the user specifies missing
, we replace it with a default anyway along the way, correct?
I'm not quite sure I understand what you mean.
When the simulation is defined using a config file, the fields of the Simulation
struct will be missing
until they are calculated.
For example, sim.electric_potential
will stay missing
until it is explicitly calculated,
e.g. through calculate_electric_potential!(sim)
or simulate!(sim)
.
Sometimes, having missing
as default value will lead to rather unhelpful error messages:
Oh, yes, silly me.
I would be in favor of replacing missing with something more type-stable
If we can predict the exact type that will result from field-calc, etc., we could will it with "empty" fields, etc. That would be nice for type stability, but can we determine all of the types when Simulation
is instantiated?
I think, except for the ChargeDriftModel
, yes.
Most of the types depend on the precision type and the coordinate system and will be defined when calling Simulation
and parsing the config file.
For the ChargeDriftModel
, we can implement a default.
Simulation
is a mutable struct and was never intended to be type stable as it is only a collecting struct.
Internal functions which need to be fast should only get fields of the simulation and not the simulation itself.
We should fix these parts if that is the case somewhere.
I would close this.
Even if this is not type stable: should we introduce our own missing
or undefined
such that we can throw more helpful warnings when something undefined is plotted?
plot(sim.electric_potential) # missing
now gives TypeError: non-boolean (Missing) used in boolean context
...
It would be more helpful to get something likePotential not defined. Please run calculate_electric_potential!
.