legend-exp / LegendGeSim.jl

High-level Julia package for LEGEND HPGe detector simulation
Other
2 stars 8 forks source link

ToDo | General #37

Open sagitta42 opened 1 year ago

sagitta42 commented 1 year ago

ToDo items that span several stages from detector to elec response simulation

sagitta42 commented 11 months ago

@oschulz @fhagemann a small question: advice on how to name an optional bool variable in simulate_pulses() and simulate_waveforms() that corresponds to overwrite in simulate_fields() i.e. determines whether to read cached simulation if present with given cached name or overwrite it. I'm thinking simply overwrite could be ambiguous when talking about waveform simulation (even though the only thing written is always only detector simulation). So maybe something like overwrite_fields?

Then it would look something like this:

pss_table, pss_truth = LegendGeSim.simulate_pulses(detector_metadata_filename, pet_input_file,
    environment_settings, simulation_settings, noise_model; n_waveforms=100, overwrite_fields=true)
oschulz commented 11 months ago

Which of the arguments of simulate_pulses contains data that would be overwritten?

sagitta42 commented 11 months ago

Nothing "contains" data that should be overwritten. The argument overwrite should be passed to stp_to_pss() in simulate_pulses() where it would then be passed to simulate_detector(). Currently in stp_to_pss() the function simulate_detector() is called with a hard-coded overwrite = false.

https://github.com/legend-exp/LegendGeSim.jl/blob/249a22280d728ce059c32e1e679dcbf70b806330/src/stp_to_pss.jl#L19

fhagemann commented 11 months ago

How about, in general, using recalculate (or recalculate_fields) rather than overwrite (or overwrite_fields)?

sagitta42 commented 11 months ago

I like it! Maybe some how include the word "cached" in the variable, e.g. overwrite_cached to refer to the "cached name" in the simulation settings. That could be intuitive. The most descriptive and accurate name would be recalculate_cached_fields, but do we want an optional variable name be that long...

oschulz commented 11 months ago

Makes sense.

fhagemann commented 11 months ago

Question is: what exactly does the keyword do?

I feel like there might be three options which might be wanted by the user:

We might need two keywords for this, something like save_fields and load_cached_fields? Or is any of the three scenarios never the case? Then, we might be able to work with one keyword argument and can be fixed according to what we actually select.

sagitta42 commented 11 months ago

This keyword controls whether to read cached simulation from file based on given cached name or overwrite it.

Option 1 is achieved by not providing a cached name in the simulation, then it doesn't matter what overwrite is.

Option 2 calculate the fields and cache them to a file - currently done if file does not exist yet; if it exists, the fields will be read unless overwrite is true (= option 3)