-
Notifications
You must be signed in to change notification settings - Fork 37
Add support for simulation for dead layer-Step3: BulkSurfaceConstantLifetimeChargeTrappingModel #480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: transition
Are you sure you want to change the base?
Conversation
@fhagemann |
ff8fb10
to
b8bfdee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, the struct BulkSurfaceConstantLifeTimeChargeTrappingModel
is very long and I would avoid having a function if there if possible.
What about this:
- Just having a
ConstantLifeTimeChargeTrappingModel
with electron and hole lifetime. - Define a virtual volume that defines "the surface" where you just apply another
ConstantLifeTimeChargeTrappingModel
This way you don't need the function if_in_bulk
and can have a simpler struct that is easier to deal with.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
if haskey(parameters, "bel") bel = _parse_value(T, parameters["bel"], internal_time_unit) end | ||
if haskey(parameters, "shl") shl = _parse_value(T, parameters["shl"], internal_time_unit) end | ||
if haskey(parameters, "sel") sel = _parse_value(T, parameters["sel"], internal_time_unit) end | ||
if haskey(parameters, "if_in_bulk") if_in_bulk = parameters["if_in_bulk"] end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you envision defining this in a config file?
I expected parameters["if_in_bulk"]
to be a String
and to fail this line.
@@ -85,6 +85,11 @@ function Semiconductor{T}(dict::AbstractDict, input_units::NamedTuple, outer_tra | |||
haskey(dict["charge_trapping_model"], "model") && | |||
dict["charge_trapping_model"]["model"] == "Boggs" | |||
BoggsChargeTrappingModel{T}(dict["charge_trapping_model"], temperature = temperature) | |||
# TODO: implementation method of the BulkSurfaceConstantLifetimeChargeTrappingModel using the Configuration file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, this is still WIP. Do you plan to do this in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only problem is we need the the is_in_bulk
function, which is tricky to implement in the config. I am gonna try first, using the virtual volume.
test/test_charge_drift_models.jl
Outdated
@@ -80,6 +80,21 @@ end | |||
|
|||
end | |||
end | |||
@timed_testset "Charge Trapping: BulkSurfaceConstantLifetimeChargeTrappingModel" begin | |||
bhl = bel = shl = sel = 0.1 # 0.1 ms -> signalsum: ~1.9778; 0.01 -> ~1.8029; 1 -> ~1.9963 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you know what the values are, you could test for exactly this: test lifetimes getting shorter and shorter and testing that the signal sum decreases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will add a few more tests.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
bel::T | ||
shl::T | ||
sel::T | ||
if_in_bulk::Function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be is_in_bulk
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, is_in_bulk
is more in line with the other function names. I will change this.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
|
||
@inbounds for i in eachindex(tmp_signal) | ||
Δt::T = (i > 1) ? (pathtimestamps[i] - pathtimestamps[i-1]) : zero(T) | ||
Δq::T = ctm.if_in_bulk(path[i]) ? q * Δt/bl : q * Δt/sl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if Δt
is bigger than bl
and sl
? Then, you will flip the sign of the charge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is a potential breaking. But the bl
and sl
are much bigger than 100 ns in most of the cases, while we usually set the Δt
to 1 ns.
I think we should add a reminder that the Δt
should be much smaller than bl
and sl
.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
|
||
parameters = haskey(config_dict, "parameters") ? config_dict["parameters"] : config_dict | ||
|
||
allowed_keys = ("bhl", "bel", "shl", "sel", "if_in_bulk") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the keywords should be a bit more descriptive than this?
sel
to me sound rather like selection
rather than the abbreviation of surface electron lifetime
.
Maybe use τ
(τh
and τe
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, use the symbol is better.
Sounds better. I will look into this virtual volume way. |
…within cofig) 2) use symbol 3) more test
@fhagemann I didn't choose to use the |
docs/src/man/charge_drift.md
Outdated
# only model the sensitive volume | ||
τh = τe = 1e6 | ||
parameters = Dict("parameters" => Dict("τh" => τh*u"ns", "τe" => τe*u"ns")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# only model the sensitive volume | |
τh = τe = 1e6 | |
parameters = Dict("parameters" => Dict("τh" => τh*u"ns", "τe" => τe*u"ns")) | |
# only model the sensitive volume | |
τh = τe = 1e6*u"ns" | |
parameters = Dict("parameters" => Dict("τh" => τh, "τe" => τe)) |
Or maybe even do τh = τe = 1u"ms"
if that also works
docs/src/man/charge_drift.md
Outdated
τh = τe = 1e6 | ||
τh_inactive = τe_inactive = 80 | ||
parameters = Dict("parameters" => Dict("τh" => τh*u"ns", "τe" => τe*u"ns", "τh_inactive" => τh_inactive*u"ns", "τe_inactive" => τe_inactive*u"ns"), "inactive_layer_geometry" => sim.detector.semiconductor.charge_trapping_model.inactive_layer_geometry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply the units already to τh
etc.:
τh = τe = 1e6 | |
τh_inactive = τe_inactive = 80 | |
parameters = Dict("parameters" => Dict("τh" => τh*u"ns", "τe" => τe*u"ns", "τh_inactive" => τh_inactive*u"ns", "τe_inactive" => τe_inactive*u"ns"), "inactive_layer_geometry" => sim.detector.semiconductor.charge_trapping_model.inactive_layer_geometry) | |
τh = τe = 1e6u"ns" | |
τh_inactive = τe_inactive = 80u"ns" | |
parameters = Dict("parameters" => Dict("τh" => τh, "τe" => τe, "τh_inactive" => τh_inactive, "τe_inactive" => τe_inactive), "inactive_layer_geometry" => sim.detector.semiconductor.charge_trapping_model.inactive_layer_geometry) |
""" | ||
struct ConstantLifetimeChargeTrappingModel{T <: SSDFloat} <: AbstractChargeTrappingModel{T} | ||
This constant-lifetime-based charge trapping model is similar to the Boggs model, which is constant-mean-free-path based. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe define this without referencing Boggs
here, something like
This constant-lifetime-based charge trapping model is similar to the Boggs model, which is constant-mean-free-path based. | |
This constant-lifetime-based charge trapping model assumes electrons and holes to have a constant free lifetime throughout the crystal. |
src/ChargeTrapping/ChargeTrapping.jl
Outdated
This constant-lifetime-based charge trapping model is similar to the Boggs model, which is constant-mean-free-path based. | ||
The model implemented has two sets of parameters for the sensitive (bulk) and inactive (dead layer/surface layer) volume respectively. | ||
The inactive layer effect will only be considered if the corresponding `virtual_drift_volume` is defined in the configuration file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a virtual_drift_volume
? You call it inactive_layer_geometry
in the documentation.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
running_sum::T = zero(T) | ||
q::T = charge | ||
|
||
inactive_layer_exist = ctm.inactive_layer_geometry !== missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inactive_layer_exist = ctm.inactive_layer_geometry !== missing | |
inactive_layer_exist = !ismissing(ctm.inactive_layer_geometry) |
Again, consider using nothing
as default, you can then use !isnothing
here.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
|
||
@inbounds for i in eachindex(tmp_signal) | ||
Δt::T = (i > 1) ? (pathtimestamps[i] - pathtimestamps[i-1]) : zero(T) | ||
Δq::T = (inactive_layer_exist && in(path[i], ctm.inactive_layer_geometry)) ? q * Δt/τ_inactive : q * Δt/τ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split this into two lines so it's easier to see what is happening:
Δq::T = (inactive_layer_exist && in(path[i], ctm.inactive_layer_geometry)) ? q * Δt/τ_inactive : q * Δt/τ | |
τi::T = (inactive_layer_exist && in(path[i], ctm.inactive_layer_geometry)) ? τ_inactive : τ | |
Δq::T = q * Δt/τi |
src/ChargeTrapping/ChargeTrapping.jl
Outdated
end | ||
|
||
parameters = haskey(config_dict, "parameters") ? config_dict["parameters"] : config_dict | ||
inactive_layer_geometry = haskey(config_dict, "inactive_layer_geometry") ? config_dict["inactive_layer_geometry"] : missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using nothing
here as default.
src/ChargeTrapping/ChargeTrapping.jl
Outdated
end | ||
|
||
parameters = haskey(config_dict, "parameters") ? config_dict["parameters"] : config_dict | ||
inactive_layer_geometry = haskey(config_dict, "inactive_layer_geometry") ? config_dict["inactive_layer_geometry"] : missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using nothing
here as default.
inactive_layer_geometry = haskey(config_dict, "inactive_layer_geometry") ? config_dict["inactive_layer_geometry"] : missing | |
inactive_layer_geometry = haskey(config_dict, "inactive_layer_geometry") ? config_dict["inactive_layer_geometry"] : nothing |
Once this PR is finished, could you update the test code to work with the new implementation/naming? :) |
Sure! I will update all the naming and the test code tomorrow. |
Updated test code for this PR: using Revise
using Plots
using Pkg
Pkg.develop(path="./SolidStateDetectors.jl")
using SolidStateDetectors
using Unitful
T = Float32
sim = Simulation{T}(SSD_examples[:TrueCoaxial])
simulate!(sim)
plots = plot()
τe = 1u"ms"
τh_list = (10u"ns", 100u"ns", 1u"µs", 10u"µs", 100u"µs", 1u"ms")
for τh in τh_list
trapping_model = ConstantLifetimeChargeTrappingModel{T}(Dict("parameters" => Dict("τh" => τh, "τe" => τe)))
sim.detector = SolidStateDetector(sim.detector, trapping_model)
energy_depos = [sim.detector.semiconductor.material.E_ionisation]
starting_positions = [CylindricalPoint{T}(0.005, 0, 0.005)]
evt = Event(starting_positions, energy_depos)
simulate!(evt, sim, Δt=1u"ns", max_nsteps=100)
plot!(evt.waveforms[1], xlabel="Time", ylabel="Charge", label="Hole lifetime = $(τh)", xlim=[0, 100])
end
plots |
config_dict = SolidStateDetectors.parse_config_file(SSD_examples[:TrueCoaxial]) | ||
simA = @test_nowarn Simulation{T}(config_dict) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't seem to use config_dict
anywhere else, so you could just do
config_dict = SolidStateDetectors.parse_config_file(SSD_examples[:TrueCoaxial]) | |
simA = @test_nowarn Simulation{T}(config_dict) | |
simA = @test_nowarn Simulation{T}(SSD_examples[:TrueCoaxial]) |
@test simA.detector.semiconductor.charge_trapping_model.inactive_layer_geometry.hZ == T(0.005) | ||
end | ||
|
||
simA = Simulation{T}(SSD_examples[:TrueCoaxial]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then you can also skip this line and continue using simA
from line 86
simA_inactive_layer_geometry=deepcopy(simA.detector.semiconductor.charge_trapping_model.inactive_layer_geometry) | ||
|
||
# test2: only model the bulk volume, test the bulk signals while varying the lifetime: τ | ||
pos = CartesianPoint{T}(0.005,0,0.005); Edep = 1u"eV" | ||
signalsum_list = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, if you create an Array
to push!
to it afterwards, consider specifying a type
signalsum_list = [] | |
signalsum_list = T[] |
pos_bulk = CartesianPoint{T}(0.0085,0,0.005); Edep = 1u"eV" | ||
@test !in(pos_bulk, simA_inactive_layer_geometry) | ||
signalsum_list_bulk = [] | ||
τ_list = (10 .^ (-2:0.2:0))*u"ms" | ||
pos_inactive = CartesianPoint{T}(0.0095,0,0.005); Edep = 1u"eV" | ||
@test in(pos_inactive, simA_inactive_layer_geometry) | ||
signalsum_list_inactive = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here:
pos_bulk = CartesianPoint{T}(0.0085,0,0.005); Edep = 1u"eV" | |
@test !in(pos_bulk, simA_inactive_layer_geometry) | |
signalsum_list_bulk = [] | |
τ_list = (10 .^ (-2:0.2:0))*u"ms" | |
pos_inactive = CartesianPoint{T}(0.0095,0,0.005); Edep = 1u"eV" | |
@test in(pos_inactive, simA_inactive_layer_geometry) | |
signalsum_list_inactive = [] | |
pos_bulk = CartesianPoint{T}(0.0085,0,0.005); Edep = 1u"eV" | |
@test !in(pos_bulk, simA_inactive_layer_geometry) | |
signalsum_list_bulk = T[] | |
τ_list = (10 .^ (-2:0.2:0))*u"ms" | |
pos_inactive = CartesianPoint{T}(0.0095,0,0.005); Edep = 1u"eV" | |
@test in(pos_inactive, simA_inactive_layer_geometry) | |
signalsum_list_inactive = T[] |
@info signalsum_list_bulk | ||
@test all(signalsum_list_bulk .< T(2.0)) | ||
@test all(diff(signalsum_list_bulk) .> 0) | ||
@info signalsum_list_inactive | ||
@test all(signalsum_list_inactive .< T(2.0)) | ||
@test all(diff(signalsum_list_inactive) .> 0) | ||
@test all(signalsum_list_bulk .> signalsum_list_inactive) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the @info
statements for the tests
@info signalsum_list_bulk | |
@test all(signalsum_list_bulk .< T(2.0)) | |
@test all(diff(signalsum_list_bulk) .> 0) | |
@info signalsum_list_inactive | |
@test all(signalsum_list_inactive .< T(2.0)) | |
@test all(diff(signalsum_list_inactive) .> 0) | |
@test all(signalsum_list_bulk .> signalsum_list_inactive) | |
@test all(signalsum_list_bulk .< T(2.0)) | |
@test all(diff(signalsum_list_bulk) .> 0) | |
@test all(signalsum_list_inactive .< T(2.0)) | |
@test all(diff(signalsum_list_inactive) .> 0) | |
@test all(signalsum_list_bulk .> signalsum_list_inactive) |
pos_inactive = CartesianPoint{T}(0.01-depth,0,0.005); Edep = 1u"eV" | ||
@test in(pos_inactive, simA_inactive_layer_geometry) | ||
evt_inactive = Event(pos_inactive , Edep) | ||
timed_simulate!(evt_inactive, simA, diffusion=false, Δt=1u"ns") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to have explicit diffusion = false
, this is the default
timed_simulate!(evt_inactive, simA, diffusion=false, Δt=1u"ns") | |
timed_simulate!(evt_inactive, simA, Δt=1u"ns") |
"inactive_layer_geometry" => simA_inactive_layer_geometry) | ||
trapping_model=ConstantLifetimeChargeTrappingModel{T}(parameters) | ||
simA.detector = SolidStateDetector(simA.detector, trapping_model) | ||
signalsum_list_inactive = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signalsum_list_inactive = [] | |
signalsum_list_inactive = T[] |
z: 5.0 | ||
``` | ||
|
||
The inactive layer could only be modeled with the help of the configuration file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that still true? I saw in the tests that you can pass a geometry as inactive_layer
to create a new SolidStateDetector
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can pass the geometry without the config, but the geometry has to be defined in the config.
# model both the sensitive and inactive volumes based on the inactive volume geometry defined in the configuration file | ||
τh = τe = 1u"ms" | ||
τh_inactive = τe_inactive = 80u"ns" | ||
parameters = Dict("parameters" => Dict("τh" => τh, "τe" => τe, "τh_inactive" => τh_inactive, "τe_inactive" => τe_inactive), "inactive_layer_geometry" => sim.detector.semiconductor.charge_trapping_model.inactive_layer_geometry) | ||
sim.detector = SolidStateDetector(sim.detector, ConstantLifetimeChargeTrappingModel{T}(parameters)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here, it looks like you can pass a geometry, even without using a configuration file.
* `τh_inactive::T`: Lifetime for holes in surface layer (dead layer/inactive layer) (default: `τh_inactive = 1ms`). | ||
* `τe_inactive::T`: Lifetime for electrons in surface layer (default: `τe_inactive = 1ms`). | ||
* `inactive_layer_geometry`: The geometry of the inactive layer. When this is defined, the model will consider the inactive effect. | ||
> Note: All τ must be much bigger than `Δt`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you explicitly check for this in the drift code? If not, it might make sense to throw a warning or error if τ
is smaller than Δt
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it works like expected when τ
is bigger than 10 ns, and it doesn't work when τ
is 1 ns. I didn't test more values, but it already makes a lot of sense. When solving the difference equation, you should always make sure the step is small enough. But it is indeed better to throw a warning when τ
and Δt
are close.
Actually, the Boggs model also has this problem. The parameter (mean free path) should also be much bigger than the typical step length.
inactive_layer_exist = !isnothing(ctm.inactive_layer_geometry) | ||
τ::T = ifelse(charge > 0, ctm.τh, ctm.τe) | ||
τ_inactive::T = ifelse(charge > 0, ctm.τh_inactive, ctm.τe_inactive) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a check here to see that τ
is bigger than Δt
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
function ConstantLifetimeChargeTrappingModel{T}(config_dict::AbstractDict = Dict()) where {T <: SSDFloat} | ||
τh::T = ustrip(u"s", 1u"ms") | ||
τe::T = ustrip(u"s", 1u"ms") | ||
τh_inactive::T = ustrip(u"s", 80u"ns") | ||
τe_inactive::T = ustrip(u"s", 80u"ns") | ||
|
||
if haskey(config_dict, "model") && !haskey(config_dict, "parameters") | ||
throw(ConfigFileError("`ConstantLifetimeChargeTrappingModel` does not have `parameters`")) | ||
end | ||
|
||
parameters = haskey(config_dict, "parameters") ? config_dict["parameters"] : config_dict | ||
inactive_layer_geometry = haskey(config_dict, "inactive_layer_geometry") ? config_dict["inactive_layer_geometry"] : nothing | ||
|
||
allowed_keys = ("τh", "τe", "τh_inactive", "τe_inactive") | ||
k = filter(k -> !(k in allowed_keys), keys(parameters)) | ||
!isempty(k) && @warn "The following keys will be ignored: $(k).\nAllowed keys are: $(allowed_keys)" | ||
|
||
if haskey(parameters, "τh") τh = _parse_value(T, parameters["τh"], internal_time_unit) end | ||
if haskey(parameters, "τe") τe = _parse_value(T, parameters["τe"], internal_time_unit) end | ||
if haskey(parameters, "τh_inactive") τh_inactive = _parse_value(T, parameters["τh_inactive"], internal_time_unit) end | ||
if haskey(parameters, "τe_inactive") τe_inactive = _parse_value(T, parameters["τe_inactive"], internal_time_unit) end | ||
ConstantLifetimeChargeTrappingModel{T, typeof(inactive_layer_geometry)}(τh, τe, τh_inactive, τe_inactive, inactive_layer_geometry) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it already possible to define the model something like this?
charge_trapping_model = Dict(
"model" => Dict(
"τh" => 1u"ms",
"τe" => 1u"ms",
"τh_inactive" => 80u"ns",
"τe_inactive" => 80u"ns"
),
"inactive_layer_geometry" => Dict(
"tube" => Dict(
"r" => Dict("from" => 9.0u"mm", "to" => 10.0u"mm"),
"h" => 10.0u"mm",
"origin" => Dict("z" => 5.0u"mm")
)
)
)
or can inactive_layer_geometry
only be a Geometry
object (or nothing
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes! This seems to be a way to define the full trapping model without the config. I will try to build the geometry with a dict in the trapping model struct.
if haskey(constant_lifetime_ctm_dict, "inactive_layer_geometry") | ||
constant_lifetime_ctm_dict["inactive_layer_geometry"] = Geometry(T, dict["charge_trapping_model"]["inactive_layer_geometry"], input_units, transformations) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this could be added to the constructor of ConstantLifetimeChargeTrappingModel
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly!
Changes
Add a new charge trapping model:
Test
The pulses while the hole lifetimes are 10 ns, 100 ns,,,1 ms. (the electron lifetime = 1ms)

Code for the test