EBSaver for EBV2 #337
EBSaver for EBV2 #337
Conversation
|
Looks good! I don't see any other major problems beyond those raised by Marcus in PR #307. Ideally we would drop the .txt files outputted by this tool (addressed by marcus in his final comment), or find a better way to store that information in some database. There are also quite a few cout remaining, although you have added conditions on the verbosity for most of them. Given the EventBuilder already compiles pretty slowly we should add a condition (or make them Log) for all final remaining cout. Aside from these, it's ready to merge and is currently in use and I have not identified any problematic aspects of the tool when testing. |
jminock
added
Ready for Level 0 Review and merge
and removed
Ready for Software meeting discussion
labels
| return true; | ||
| } | ||
|
|
||
| bool EBSaver::SaveOrphanPMT(int runCode) |
There was a problem hiding this comment.
these methods dont do anything?? is this intended
|
One comment I have is that the "ANNIEEvent" has a well-defined format. Does the new event builder change that format? There seem to be variables being populated that I couldn't spot in the Google doc. |
| { | ||
| Log("EBSaver: Saving PMT data with PMTTime " + std::to_string(PMTTime), v_debug, verbosityEBSaver); | ||
| // find PMTTime in InProgressHits, if not found, return false | ||
| if (InProgressHits->find(PMTTime) == InProgressHits->end()) |
There was a problem hiding this comment.
use count rather than find
|
|
||
| void EBSaver::LoadBeamInfo() | ||
| { | ||
| TFile *file = new TFile(beamInfoFileName.c_str(), "READ"); |
There was a problem hiding this comment.
check file->IsZombie()
add return type to this method so it can propagate errors such as this one and check on invocations
| if (!tree) | ||
| { | ||
| cout << "EBSaver: Failed to load beam info from file with name: " << beamInfoFileName << endl; | ||
| return; |
|
|
||
| Log("EBSaver: Finished loading beam info from " + beamInfoFileName, v_message, verbosityEBSaver); | ||
|
|
||
| return; |
There was a problem hiding this comment.
close and delete file, return true
| m_data->CStore.Get("AmBeTriggerMain", AmBeTriggerMain); | ||
| m_data->CStore.Get("PPSMain", PPSMain); | ||
|
|
||
| InProgressHits = new std::map<uint64_t, std::map<unsigned long, std::vector<Hit>> *>; |
There was a problem hiding this comment.
These are populated via Get from CStore so no need to declare new - they'll leak
| BuildEmptyMRDData(); | ||
| if (triggerTrack == 14) | ||
| { | ||
| // find the time of trigger 8 in current group |
| } | ||
| else if (triggerTrack == 36) | ||
| { | ||
| // find the time of trigger 8 in current group |
| Log("EBSaver: PMTHits new time " + std::to_string(time.first) + " size " + std::to_string(time.second.size()), v_debug, verbosityEBSaver); | ||
| for (auto const &time : *OldPMTHits) | ||
| { | ||
| if (PMTHits->count(time.first) == 0) |
There was a problem hiding this comment.
this would be a valid use of operator[]:
PMTHits[time.first].push_back(hit);
avoids the if/else branch
| if (saveRawRWMWaveform) | ||
| { | ||
| // find PMTTime as key in RWMRawWaveforms, if found, save it, if not, save an empty vector | ||
| if (RWMRawWaveforms->find(PMTTime) != RWMRawWaveforms->end() && RWMRawWaveforms->at(PMTTime).size() > 0) |
| bool EBSaver::GotAllDataFromOriginalBuffer() | ||
| { | ||
| // got PMT data | ||
| bool gotPMTHits = m_data->CStore.Get("InProgressHits", InProgressHits); |
There was a problem hiding this comment.
as far as I can tell, if these Get calls fail, the previous values of InProgressHits etc get propagated.
There appears to be no clear or resetting of these maps, and a failed Get call simply prints a message (not even an error!) and that's all....?
5 participants
EBSaver Tool in EventBuildingV2 tool set.
This is splitted from PR #307
Changes based on your comments:
· EBSaver has a lot of cout usage. Can we switch these for Log calls?
Most of them are for printing debug timestamps, I just add verbosity requirements for them. There are some couts left like line 337 just for step checks.
· The block of lines 163-214 seems to be printing out all removed events, is this needed? Maybe we can put this all in a debug verbosity check.
Added! Maybe not in a elegent way.
· line 215,242,267, - english please.
Translated, I will blame myself and chatGPT. (Learn some Chinese will be benifit tho. Just a joke)
· EBSaver finalise - seems like this is printing out some information about unbuilt event information, with MRD information specifically written to file. It seems like this could be useful information to track over time. Perhaps all that information (maybe also with the summary information on what was built too) could be put into a text summary for that event-builder output, and that could be stored as meta-information with the output file. It seems like this would be a nice way to track information about file contents and runs. We could read the contents of those meta-info files to track those metrics over time, or possibly later put them into SAM dataset meta info.
I will add this after merging current EBV2 tools to main repo. Maybe there can be better way to track rather than just txt print out. For now since we have all analysis variables saved to boost store and root file (built events), we can manually check them like in Steven's data quality report, but I agree save how many things are built and not built is useful.