Product tofv1#50
Conversation
paullglebrun
commented
Oct 2, 2022
paullglebrun
commented
- Convert the TOF/T0toRPC (Time of flight, T0 counter to RPC) module from being an analyzer to a producer. It produce a TOFHit, a signle data struct containina very much abbreviated data from the Trigger Counter, and vectors of T0Hit ans RPCHits.
- Create new corresponding data structures in RecoBase
- Fix the RPC strip numerology bug in T0/TotoRPC_module, discussed at the Sept 29 Software meeting.
- Wrote an example on how to include this information, as well the ARICHHits into a new DataQuality Analyzer module.
- Fix the hit Multi-Anode TRB3 hit multiplicty in the ARICHReco module.
…ter, T0SegmentHits and, hopefully RPCStripHits )
…nstead of the sum of these hits over a run
…the Trigger counter, the T0 segment hits, the RPC hits and the ARICH info
lackey32
left a comment
lackey32
left a comment
There was a problem hiding this comment.
I've left specific comments on some of the files - some just need their descriptions updated, others are a bit more involved.
| @@ -0,0 +1,60 @@ | |||
|
|
|||
There was a problem hiding this comment.
Since this is a fcl that actually runs the job, coding conventions dictate that its name should be all lowercase. On emphatic we have also generally been adding _job at the end of the name. Any defaults that are generally set for the modules you've written would sit in their own (capitalized) fcl specific to that module. Those fcls then get included at the top of this job fcl.
There was a problem hiding this comment.
O.K. got it... I don't like this convention very much, it forces me to edit twice the number of files. And litter the directories with twice the number of files.. In a production environment, this makes sense, but not in an environment where this particular fcl file will be outdated in days..
| @@ -0,0 +1,169 @@ | |||
| //////////////////////////////////////////////////////////////////////// | |||
| /// \brief Analyzer module to create online monitoring plots | |||
There was a problem hiding this comment.
Fix description to say what this module does.
I think this module belongs somewhere other than the DataQuality package, unless the intent is to produce items that we are going to cut on downstream based on the quality of events. I'm not sure the right name for a new package, but maybe something as generic as 'Reconstruction' or 'Analysis' where modules like this can live until we decide if they belong in some more specialized location.
There was a problem hiding this comment.
Sorry about not updating the comment.. My mistake..
And yes, this should belong some where else ! My suggestion is to create the "StoneSoup" directory, where we can stash preliminay cross-detector modules, as prorotypes of real analysis & cross-detector reconstruction.
| fRunHistory = new runhist::RunHistory(run.run()); | ||
| fChannelMap->LoadMap(fRunHistory->ChanFile()); | ||
| } | ||
| void AnalyzeTOFARICH::beginJob() |
There was a problem hiding this comment.
I think you can remove this function altogether unless you plan on adding something to it later.
There was a problem hiding this comment.
Yes, I am hoping, soon, to peak at raw SSD iformation for instance. So, I would leave it there..
| fRun = evt.run(); | ||
| if (!fFilesAreOpen) this->openOutputCsvFiles(); | ||
| fSubRun = evt.subRun(); | ||
| fEvtNum = evt.id().event(); |
There was a problem hiding this comment.
Is this any different from evt.event()?
| fFilesAreOpen = false; | ||
| } | ||
|
|
||
| void AnalyzeTOFARICH::reconfigure(const fhicl::ParameterSet& pset) |
There was a problem hiding this comment.
Perhaps I'm misremembering, but I thought reconfigure() was somewhat deprecated a few years ago? You can do all of the pset.get<> lines above in the constructor eg.
AnalyzeTOFARICH::AnalyzeTOFARICH(fhicl::ParameterSet const& pset) : EDAnalyzer(pset),
fTokenJob (pset.get<std::string>("tokenJob", "UnDef")),
fTOFInfoLabel (pset.get<std::string>("TOFInfoLabel")),
fARingInfoLabel (pset.get<std::string>("ARingInfoLabel"))
{
| emph::cmap::DChannel dchan = fChannelMap->DetChan(echan); | ||
| int detchan = dchan.Channel(); | ||
| if (detchan == 500) fNT0NumCase500++; | ||
| long double time_T0 = trb3.GetEpochCounter()*10240026.0 + trb3.GetCoarseTime() * |
There was a problem hiding this comment.
Not an issue for this commit specifically, but I worry about people copying and pasting this time conversion everywhere. If it's the same for each TRB3 digit, we should have this in some central location (perhaps even in the raw digit?).
There was a problem hiding this comment.
Good point, I have the same worry, the TRB3 data is not yet corrected for slew, and, indeed, perhaps different FPGA code, leading to different bit assigment into the TRB3 measurement words..
…stency with latest geometry changes
2 participants
