Temperature handling for charge drift and trapping models#611
Temperature handling for charge drift and trapping models#611claudiaalvgar wants to merge 5 commits into
Conversation
| imp_model::AbstractImpurityDensity, input_units::NamedTuple, temperature::RealQuantity | ||
| ) where {T <: SSDFloat} | ||
|
|
||
| temperature = _parse_value(T, get(config, "temperature", 90u"K"), input_units.temperature) | ||
| temperature::T = _parse_value(T, temperature, input_units.temperature) | ||
|
|
||
| if temperature < 50 || temperature > 150 | ||
| @warn "Temperature = $(temperature) K is outside the typical validated range (50–150 K)." | ||
| end |
There was a problem hiding this comment.
Looking at maximum compatibility: how about still being able to define the temperature in the charge drift model, especially for the case where no temperature was explicitly defined for the semiconductor (and maybe throwing an error here in case two non-matching temperatures are defined in the semiconductor and the charge drift model)?
|
|
||
| BoggsChargeTrappingModel(args...; T::Type{<:SSDFloat}, kwargs...) = BoggsChargeTrappingModel{T}(args...; kwargs...) | ||
| function BoggsChargeTrappingModel{T}(config_dict::AbstractDict = Dict(); temperature::RealQuantity = T(78)) where {T <: SSDFloat} | ||
| function BoggsChargeTrappingModel{T}(temperature::RealQuantity, config_dict::AbstractDict = Dict()) where {T <: SSDFloat} |
There was a problem hiding this comment.
This looks like a breaking change..
Any way that we can keep config_dict as the first argument?
There was a problem hiding this comment.
Maybe give temperature a default value missing, and only read it from the config file if it is missing (?)
| throw(ConfigFileError("`model` is defined but empty in config. Remove it or provide a valid name.")) | ||
| elseif model == "Boggs" | ||
| BoggsChargeTrappingModel{T}(config_dict; temperature) | ||
| BoggsChargeTrappingModel{T}(temperature, config_dict) |
There was a problem hiding this comment.
| BoggsChargeTrappingModel{T}(temperature, config_dict) | |
| BoggsChargeTrappingModel{T}(config_dict, temperature) |
?
There was a problem hiding this comment.
Some of the changes you are proposing would be breaking changes, requiring a new minor version 0.12..
Is there any way this can be avoided? Is it because the temperature needs a default value?
Maybe set it to nothing and deal with it in the functions (if it is nothing, use the one from the config. If it has a value, it was passed from the semiconductor or by the user -- or something like that)?
| value: -1e10cm^-3 | ||
| charge_drift_model: | ||
| model: InactiveLayerChargeDriftModel | ||
| temperature: 90K |
There was a problem hiding this comment.
Maybe still allow for this syntax (to make this non-breaking), but check that it matches the temperature of the semiconductor, if any was given (?)
|
Just a follow up on the last commits: In the Charge Drift Model (CDM) the temperature is always taken from the semiconductor since For the Charge Trapping Model the temperature can come from either the argument or from temperature inside the parameters block of the config. If both are present they must agree; if only one is provided that value is used; if neither is provided an error is thrown. This is safe because no parallel code path reads |
| throw(ConfigFileError( | ||
| "Temperature mismatch: InactiveLayerChargeDriftModel defines temperature = $(cdm_temperature) K, " * | ||
| "but the semiconductor temperature is $(temperature) K. " * | ||
| "Remove `temperature` from the charge drift model and define it only in the semiconductor." |
There was a problem hiding this comment.
I mean: people could still define them both in the semiconductor and in the charge drift model, as long as they match. Maybe reword this accordingly?
| "Remove `temperature` from the charge drift model and define it only in the semiconductor." | |
| "Define `temperature` in the semiconductor and either remove it from the charge drift model or make sure that the temperatures match." |
or something like this?
| parameters = haskey(config_dict, "parameters") ? config_dict["parameters"] : config_dict | ||
|
|
||
|
|
||
| temp::Union{T, Missing} = temperature === missing ? missing : _parse_value(T, temperature, internal_temperature_unit) |
There was a problem hiding this comment.
looking at this, maybe a better default value than missing would be NaN, because NaN is of type RealQuantity and T already and would need these Unions then.
| temp::Union{T, Missing} = temperature === missing ? missing : _parse_value(T, temperature, internal_temperature_unit) | |
| temp::T = isnan(temperature) ? T(NaN) : _parse_value(T, temperature, internal_temperature_unit) |
Even though, missing might be semantically the right choice here..
2 participants
This PR fixes the temperature handling across charge drift and charge trapping models to ensure that model temperatures are inherited consistently from the semiconductor configuration rather than being independently specified
Updated semiconductor model construction to propagate the detector temperature to:
* InactiveLayerChargeDriftModel
* BoggsChargeTrappingModel
* CombinedChargeTrappingModel
Updated charge trapping tests to construct models using the detector semiconductor temperature.
Removed tests that relied on model-specific temperature values in configuration files.
This change eliminates duplicated temperature definitions across detector models and prevents inconsistencies between semiconductor and trapping/drift temperatures.