Skip to content

Temperature handling for charge drift and trapping models#611

Open
claudiaalvgar wants to merge 3 commits into
JuliaPhysics:mainfrom
claudiaalvgar:Temperature_handling
Open

Temperature handling for charge drift and trapping models#611
claudiaalvgar wants to merge 3 commits into
JuliaPhysics:mainfrom
claudiaalvgar:Temperature_handling

Conversation

@claudiaalvgar

Copy link
Copy Markdown
Collaborator

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.

Comment on lines +66 to +73
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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks like a breaking change..
Any way that we can keep config_dict as the first argument?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
BoggsChargeTrappingModel{T}(temperature, config_dict)
BoggsChargeTrappingModel{T}(config_dict, temperature)

?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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 (?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants