Add EBLoadRaw for EBV2 tool chain#338
Add EBLoadRaw for EBV2 tool chain#338fengyvoid wants to merge 1 commit intoANNIEsoft:Applicationfrom
Conversation
|
For the force matching, we have tested them a while ago, and current processed data on pnfs are all already with that update. |
|
Looks good. |
jminock
added
Ready for Level 0 Review and merge
and removed
Ready for Software meeting discussion
labels
| CData = new std::vector<CardData>; | ||
| TData = new TriggerData; | ||
| MData = new MRDOut; | ||
| LData = new PsecData; |
There was a problem hiding this comment.
CData TData MData and LData appear to be leaking
There was a problem hiding this comment.
I'm not so sure....
on line 597 CData is populated with PMTData->Get("CardData", *CData); (note the de-reference means it should set the value of CData, not overwrite the pointer), and subsequently on line 599 the CData pointer is passed to the CardData BoostStore, which will take ownership and handle deletion.
That does make me somewhat nervous that the CardData BoostStore may delete CData, resulting in the de-reference on line 597 segfaulting.... 🤔 Presumably if this doesn't happen CData CardData doesn't get deleted, or at least by the time it does, line 597 is never encountered again....
There was a problem hiding this comment.
your correct i rescind my objection
There was a problem hiding this comment.
but agree with your comment about tis reuse...
one can set the pointer and ask the boost store not to manage the data which is a fix for that
| // First, parse the lines and get all files. | ||
| std::string line; | ||
| ifstream myfile(FileList.c_str()); | ||
| if (myfile.is_open()) |
There was a problem hiding this comment.
perhaps an else { std::cerr<<... would be sensible
| m_data->CStore.Set("subrunNumber", -9999); | ||
| } | ||
|
|
||
| if (std::regex_search(fileName, match, rawFileNumber_regex) && match.size() > 1) |
There was a problem hiding this comment.
you can do a single regex match that captures multiple matches at once....
sscanf may be a simpler alternative for something this simple.
| ss_trigoverlap << "TrigOverlap_R" << RunNumber << "S" << SubRunNumber << "p" << PartFileNumber; | ||
| std::cout << "EBLoadRaw: Loading Trigger Overlap data: " << ss_trigoverlap.str() << std::endl; | ||
| BoostStore TrigOverlapStore; | ||
| bool store_exist = TrigOverlapStore.Initialise(ss_trigoverlap.str().c_str()); |
There was a problem hiding this comment.
should this have a configurable path prefix?
| } | ||
| LoadedLAPPDTotalEntries += LAPPDTotalEntries; | ||
| } | ||
| catch (std::exception &e) |
There was a problem hiding this comment.
does anything in this try block actually throw?
| { | ||
| BoostStore TrigOverlapStore; | ||
| std::stringstream ss_trigoverlap; | ||
| ss_trigoverlap << "TrigOverlap_R" << RunNumber << "S" << SubRunNumber << "p" << PartFileNumber; |
5 participants
EBLoadRaw Tool in EventBuildingV2 tool set.
This is splitted from PR #307
Changes based on your comments:
· EBLoadRaw line 295 - maybe relax this check to if line[0]=='#' otherwise trailing comments would result in the file being omitted.
Changed, now it's at line 314. I leave the previous commented line there just in case.
New changes:
I add some variables like 'PMTMatchingForced' to force a matching at the end of each partfile. Without it there maybe ~1 to 5% of events unpaired because of the asynchronizely loading and pairing of PMT and trigger.