Closed fhagemann closed 1 year ago
Don't merge until LegendTestData.jl is updated.
@fhagemann I updated this json to be of the correct format at some point when it stabilized, I think before the collab meeting
https://github.com/legend-exp/legend-testdata/blob/main/data/ldsim/invcoax-metadata.json
Yes, but the package LegendTestData.jl is not yet linking to the commit where this is updated. So, when using LegendTestData.jl, it's still pulling a config file in the old format from before the collaboration meeting.
You can see the commit hash it's linking to the commit 7df474059cb6cf1d7c7ff836ddfb7e8d3373abb0 (see line 6 in Artifacts.toml)
which is this one (still in the old format): https://github.com/legend-exp/legend-testdata/blob/7df474059cb6cf1d7c7ff836ddfb7e8d3373abb0/data/ldsim/invcoax-metadata.json
With the new releases of LegendTestData.jl and RadiationDetectorSignals.jl, the current tests should pass. I also added tests for waveforms with noise applied to them.
Patch coverage: 100.00%
and project coverage change: +36.24%
:tada:
Comparison is base (
eaae6ee
) 13.60% compared to head (bd00eca
) 49.84%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I started adding tests after @fhagemann told me "add tests" :D But he also added tests and submitted the PR before me.
I think I guess we just work with this PR and I scratch what I added. Although, I would recommend to not use json files in the tests but explicitly written out dictionaries so that it's clear what's happening there.
For example, this is what I ended up having:
@testset "Package LegendGeSim" begin
environment_settings = Dict(
"crystal_temperature_in_K" => 77,
"medium" => "vacuum"
)
simulation_settings = Dict(
"method" => "ssd"
)
for (filename, capacitance) in test_dict
detector_metadata_filename = joinpath(testdata_path, filename)
@testset "Field Simulation of $filename (SSD)" begin
sim = LegendGeSim.simulate_fields(detector_metadata_filename, environment_settings, simulation_settings)
C = LegendGeSim.capacitance_matrix(sim)
@testset "Capacitances" begin
@test isapprox(C[1,2], capacitance, atol = 0.2u"pF")
end
end
@testset "Pulse Simulation" begin
path_to_pet_file = joinpath(LegendTestData.legend_test_data_path(), "data", "ldsim", "single-invcoax-th228-geant4.csv")
# --- ideal
# pet -> pss
pss_table, pss_truth = LegendGeSim.simulate_pulses(det_metadata, path_to_pet_file, environment_settings, simulation_settings)
# --- realistic
setup_settings = Dict(
"preamp" => Dict(
"type" => "generic",
"t_decay" => 50,
"t_rise" => 15,
"max_e" => 10000,
"offset" => 2000
),
"fadc" => Dict(
"type" => "generic",
"sampling_interval" => 16
),
"trigger" => Dict(
"type" => "trapezoidal",
"window_lengths" => [250,250,250],
"threshold" => 9
),
"daq" => Dict(
"type" => "generic",
"nsamples" => 3750,
"baseline_length" => 1875
)
)
# pss -> raw
raw_table = LegendGeSim.pss_to_raw(pss_table, pss_truth, setup_settings)
# pet -> raw
raw_table1 = LegendGeSim.simulate_waveforms(det_metadata, path_to_pet_file, environment_settings, simulation_settings, setup_settings)
end
# ToDo
# - noise model: FromSim & FromData
end
end
Note: to make this work I had to add a function that can do pss_to_raw(pss_table, ...)
(I only added a wrapper that takes pss_name
and reads table, doesn't take table itself, the mid step was missing)
@sagitta42
With your example test file, I get the error that det_metadata
is undefined.
So you want det_metadata
to be defined in the tests as dictionary instead of being read in from LegendTestData.jl?
And we also cannot use the CSV file with hits for an ICPC detector for a random BEGe detector if we use your loop that iterates over all detector geometries that we want to test (?)
@fhagemann
With your example test file, I get the error that det_metadata is undefined.
So you want det_metadata to be defined in the tests as dictionary instead of being read in from LegendTestData.jl?
Sorry that was just copy-paste typo. In my snippet, it should be detector_metadata_filename
.
No, metadata is indeed easier to read from LegendTestData, but settings are better defined as dictionaries rather than json. This way it's more clear what's being tested, and also can loop e.g. for method in ["SSD", "siggen"] ...
And we also cannot use the CSV file with hits for an ICPC detector for a random BEGe detector if we use your loop that iterates over all detector geometries that we want to test (?)
Well, inside pet->stp there's the part that removes events outside of the detector :D So I thought this might work anyway, just not perfectly tailored to each detector, but some events would eventually be in the bulk x)
What I have in plan now is to have a function in LegendGeSim which can toy-monte-carlo a pet table for given detector with given SSE/MSE fraction. This is useful way beyond testing for many a study for PSS. I'm planning to make a call about it. Should be nice in Julia because I could basically generate positions in a cylinder and then use that same feature to remove the ones outside of the detector. And, it belongs in LegendGeSim! Generate your PET sample to test PSD or do a study of noise impact at lower energies or whatnot, then process right there with LegendGeSim. Sorry I got carried away
The tests now pass with the newest version of LegendHDF5IO.jl
@sagitta42 merge?
Two possibilities:
@sagitta42 what's your preference?
@fhagemann @oschulz Let's merge now, then deal with other PRs, then add tests and other potential mods.
I slightly modified the
pet -> raw
simulation chain I was given by @sagitta42 to act as a test script.Disclaimer: the tests will fail now because LegendTestData.jl still links to an old commit of legend-test-data where
invcoax-metadata.json
is still in the old metadata format. Once LegendTestData.jl is updated, these tests should pass. I would ask @oschulz to please take care of this.Running the tests, the function
propdict
threw a deprecation warning. I updated it to use the PropDictsreadprops
function. We could think about getting rid ofpropdict
in LegendGeSim.jl and replace allpropdict
byreadprops
. Any thoughts about this?