JuliaPhysics/SolidStateDetectors.jl

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:

  • Calculating the electric field before having calculated the electric potential
    missing_electric_field

  • Plotting potentials that were not yet calculated
    missing_plot

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.

lmh91 commented

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!.