Refactoring & minor modifications of TriggerActivityMakerTPC module#44
Refactoring & minor modifications of TriggerActivityMakerTPC module#44
Conversation
|
Hi Klaudia, thank you so much for preparing this! I have a couple general comments on the list of changes you have kindly provided, before digging into the code itself:
The original implementation (by Wes) was actually changed from ROPID to the current TPCSetID + View tuple indexing because we never decided if the two collection planes should be treated as the same "plane" or not in trigger algorithms. I believe the DAQ currently considers the two collection plane as one plane, but it is not obvious if it is better to splilt it in half. Therefore, the option was left open for either.
This is all fine, but I do not believe that the default value logic for
I'm a bit worried about this. All pre-existing TAMakers (ADCSW, etc) does not have flush implemented, as far as I can tell. Perhaps a compromise is to have the default
This seems unnecessary to simplify -- if this field continues to exist in the TA struct, we should check for it for equality.
Like I said, I have not looked at the code very closely to make any comments on the actual implementation but hopefully these are useful things to think about:) JS |
This PR refactors TriggerActivityMakerTPC to fix a logic issue with readout plane handling, improve modularity, and make configuration more flexible.
(TPCsetID, geo::View_t)scope type. This is fine for ProtoDUNE, but as the FD will contain a mix of single and double-sided APAs, grouping by readout_plane_id ensures that each collection TAMaker sees the same TPC drift volume.readout::ROPID, one TAMaker per physical readout plane, avoiding merges across APA sides. This is also extracted directly from the geometry service, without introducing unnecessary abstractions.algconfig_plane0–3) was keyed onView_t, plane3 was effectively unused (as views run from 0-2)algconfig_plane{0–3}replaced with fcl-configurable:active_rops: list of ROPs to process (if empty will process all that are present)algconfig_ropX: config per ROPstd::map<unsigned int, nlohmann::json> algconfigs_and built once in the constructor.3). Refactored and clarified module structure:
produce()handles pipeline orchestration only.buildAlgConfigs()sets up algorithm configuration per readout plane.toJson()converts FHiCL to JSON-like config.buildAssociations()handles TP–TA associations.alg->flush()directly with a max timestamp instead of feeding a dummy TP (which is problematic for algorithms which expect time-ordered input).std::binary_search(but this expects sorted input!)