-
Notifications
You must be signed in to change notification settings - Fork 1
FLArE + common output format overhaul #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…into output_flare
|
Thanks @mvicenzi and @koolz2151 for this overhaul!! I'll review this as soon as I can! Just to give you a heads up, I'm a bit busy these next couple of weeks due prepping for the FASER collaboration meeting, so I might need a bit longer than usual to review this |
benw22022
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @mvicenzi and @koolz2151 for this really nice PR! I have a couple of small suggested changes which I think would be nice to add before merging
In AnalysisManager.cc:
- Could we make its so that the
HDF5is only written out if FLArE is enabled?
In AnalysisManagerMessenger.h:
- Should probably refactor class member variables to use a consistent naming pattern i.e.
outDir$\rightarrow$ fOutDir
|
Thanks @benw22022 -- I added your suggestions! |
WenjieWu-Sci
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tons of work, thank you. I just have two comments, maybe for future PRs.
- The 0.5 mm step limit was originally set trying to look for the kink when tau decays in the detector. I tried with lower step limit, but would crash because of taking up too much memory. I think in general cases, we probably don't need this fine granularity. But it would be good to have it as least as an option.
- I agree it's a good idea to move the pseudo-reco part into another package.
@WenjieWu-Sci That's a good point. I will add it in the to-dos of #19 . |
This PR implements the vision originally laid out from the discussion in #2 . It provides most of the infrastructural changes, although one key item in particular is missing and will need another PR. It is quite a fat one, so please take your time to review it in full. Many thanks to @koolz2151 for her help in getting this started.
List of changes
event(currently only filling theeventID-- see below),primaries(storing primary particles for each vertex in the event) andtrajectories(optional, stores tracks for all particles in the even)..macparameters are update to be in the\out\directory for general settings and\out\<detector>\for specific settings that impact the output of a single detector (eg.,\out\flare\pseudoReco).\out\faser\saveActs falsedisables the output of FASER2 regardless whether it was included or not in the geometry).flare_hitstree for liquid argon hits,hcal_hitstree for the BabyMIND/HCAL hits. If thesave2DEvdoption is enabled, 2D histograms are saved in sub-directories of theflareoutput directory. IfpseudoRecois true, a pseud-reco tree is also written to theflaredirectory.0.5 mmin LAr has been removed in order to keep the output file size reasonable. Need to discuss more on a full strategy.AnalysisManager, which is updated inStackingActionright as a track is registered. The "custom" definition of primary particle (like direct decays of primary lepton being treated as primaries) is no longer being used, including theTrackInformationfilled inTrackingAction-- to be discussed in items 2 and 3 below.AnalysisManager::EndOfEventhas been extensively cleaned-up. Trees are booked and filled in separate functions. Generally, it's much easier to navigate the file.TPCLayerLogicalhas been removed in favor of direct placement of the 21 TPC logical modules.reco/CircleFithas been removed.What still remains open:
These are items that still need to be completed, but I believe it's best to push this PR through before proceeding.
The first one is key, while the other three are less urgent. I will open sub-issues to track them properly.
eventTTree with generator-level information. This requires introducing a general way to store generator metadata that can work across the different generators we support. More in Generator-level info ineventoutput tree #17 .primariesandtrajectoriesinto a single "particles" tree which will contain the entire particle hierarchy. Add a flag to enable saving the full trajectories. It would serve as the ACTS particle tree as well. More in Mergeprimariesandtrajectoriesinto a single "particles" tree #18 .Other small tweaks to the output formats might be needed, but I think these are the biggest open things. Out of which getting an usable
eventtree is the highest priority.