Conversation
app/tool/algorithm/get_calibs.cxx
Outdated
| daq_run(tgt, "CHARGE", buffer, 1, 100); | ||
| // daq_run(tgt, "CHARGE", buffer, n_events, 100) |
There was a problem hiding this comment.
I think sharing the n_events setting down to daq_run through get_calibs is a good idea. I don't like hardcoding it to 1 here.
If you want to have it be 1 in some scan, then you can provide that within pftool. I'd even be okay with the default within pftool being 1.
app/tool/algorithm/tot_vref_scan.cxx
Outdated
| calibs = | ||
| get_calibs<pflib::packing::SingleROCEventPacket>(tgt, roc, target_adc); | ||
| calibs = get_calibs<pflib::packing::SingleROCEventPacket>( | ||
| tgt, roc, n_events, target_adc); |
There was a problem hiding this comment.
This is what I was talking about earlier. Why is n_events = 100 here? It is very confusing for a person running the code to see n_events = 100 but then only 1 event is run later.
I understand wanting the default to only be 1 event, but then you should pass in 1 event into the functions that call get_calibs so that others reading the code can understand how many events are being used.
If one algorithm needs multiple n_event arguments for different parts of the algorithm, that also makes sense to me.
There was a problem hiding this comment.
I changed so that you input the n_events for that function. Probably should have done that from the beginning. We don't need to change the n_events for any other function than get_calibs (unless we would like to increase statistics in the future).
2 participants
Changes to
get_calibsare for future calibrations when we will want to minimize uncertainties, by setting n_events to some larger number than 1. For now, runningdaq_runinget_calibswith 1 event is fast and good enough for analysis.