Two things I've found 1) The collection name of the Ecal hits with corrections on the hit times is "TimeCorrEcalHits". "EcalCalHits" does not have these corrections (see EcalTimeCorrectionDriver.java , which is the driver that makes these corrections) 2) 8.7% of the hits are duplicates (second hits) on the same crystal, so sometimes using the map<int, EcalHit*> will give us the wrong hit. The attached file contains corrections for both of those two problems. Maurik and Omar, do I have your permission to check these changes into the git repository? On Wed, Sep 21, 2016 at 10:07 AM, Nathan Baltzell <[log in to unmask]> wrote: > Hi Maurik, > > Well, I think we probably should instead change recon (probably only > steering files > are necessary) so the corrected hit collection name is back to what it > used to be, > instead of changing the dst-maker. People (and the dst-maker previously) > usually > get to the hits via the clusters, so no one noticed a problem, but > changing a > collection's name is asking for trouble ... > > -Nathan > > > On Sep 21, 2016, at 10:01, Maurik Holtrop <[log in to unmask]> wrote: > > > Hello Nathan, Sebouh, DST users, > > > > My bad. I apologize for incorrectly linking the ECAL hits from the DST > to the EcalCalHits collection, which contrary to my understanding, seems to > not contain the corrected hits. I will fix this problem as soon as I have > figured out where the corrected hits are stored. > > > > I do think that the DST needs to contain the full collection of hits and > not only the hits used by clusters, and certainly it should not store the > same hit twice. Some some change in the DST is needed from the early > behavior. > > > > I would appreciate it if someone could please point me to the > documentation or the code that does the Ecal hit corrections, so I can > figure out what it does and where it stores the output. See my email from > yesterday about “CopyClusterCollectionDriver”. > > > > Thanks, > > Maurik > > > > > >> On Sep 21, 2016, at 6:28 AM, Nathan Baltzell <[log in to unmask]> wrote: > >> > >> Hi All, > >> > >> It looks like at least 2 things happened that are contributing to what > >> Sebouh sees: > >> > >> 1. Prior to the release 3-9 used for 2016 pass0, ecal recon was modified > >> to have a separate driver do hit time corrections (a first step in the > >> suggested code cleanup from a while a ago). It makes a hit collection > >> with a *new* name used by clustering. > >> > >> 2. Something was late changed in how dst-maker reads ecal hits. > >> > >> Somehow, after #1 but before #2 dsts were still ok. Seems #1, for > backward > >> compatibility, should be changed to output hit collection of same name > as > >> before, or modify the input hit collection in-place. > >> > >> -Nathan > >> > >> > >> > >> > >> On Sep 20, 2016, at 4:09 PM, Maurik Holtrop <[log in to unmask]> > wrote: > >> > >>> Hello ECAL folks, > >>> > >>> I am mightily confused about our Ecal collections in LCIO. Without > reading a ton of code, could someone please help me sort some things out > here? > >>> > >>> According to the Confluence page: > >>> > >>> EcalClustersCorr Cluster CopyClusterCollectionDriver > >>> reconstructed ECal clusters with corrected energies > >>> > >>> This is very confusing. The driver “CopyClusterCollectionDriver” is > supposed to just copy a collection, not also make corrections to the timing > and energy, and indeed, it doesn’t seem to, so then EcalClustersCorr and > EcalClusters are identical, or not? If they are the same, why? If not the > same, how? > >>> > >>> I just confirmed that the hits that written in the “EcalClustersCorr” > collection are NOT the same as those in EcalCalHits, when these hits have > the same ID. So that puzzles me. Where are those hits changed? What > collection are the corrected hits in? > >>> > >>> The “ReconClusterer” (nicely commented and quite readable code, thank > you) that made the EcalClusters collection uses the EcalCalHits and does > not appear to make any corrections to them. Am I wrong? > >>> > >>> Any help here would be appreciated. We can update the Confluence page > with the answers. > >>> > >>> Best, > >>> Maurik > >>> > >>> > >>>> On Sep 20, 2016, at 2:54 PM, Maurik Holtrop <[log in to unmask]> > wrote: > >>>> > >>>> Hello Omar and Sebouh, > >>>> > >>>> I will look into the issue of the hits timing, and directly compare > the various collections we have in LCIO. Give me a little time though. > >>>> > >>>> In the LCIO files, I can find the EcalReadoutHit (sounds like it is > “raw” hit from Ecal) and hte EcalCalHit. According to the driver > “EcalRawConverterDriver” the EcalCalHit is the output (and EcalReadoutHit > the input), so it seems to me that is the one to use. Also, on Confluence: > >>>> > >>>> EcalCalHits CalorimeterHit EcalRawConverterDriver calibrated > ECal Hits > >>>> > >>>> Documentation is pretty sparse though, so correct me if I am wrong > and need to use a different collection. I find the large number of > collection more than a little confusing at times. > >>>> > >>>> Best, > >>>> Maurik > >>>> > >>>> > >>>> > >>>>> On Sep 20, 2016, at 1:51 PM, Omar Moreno <[log in to unmask]> > wrote: > >>>>> > >>>>> > >>>>> Originally, as Maurik said, the DST maker only wrote Ecal hits > associated with a corrected Ecal cluster (i.e. > cluster->getCalorimeterHits();) With Maurik's changes, all Ecal hits > written to the DST now come from the collection "EcalCalHits". The cluster > time comes from the seed hit of the cluster so if there was a correction > applied to the seed that wasn't propagated to the original EcalCalHits > collection, then this might be the issue. This might be the reason why I > didn't write all of the Ecal hits to the DST maker to begin with but I > don't remember. > >>>>> > >>>>> Maurik, did you look at distributions of Ecal clusters variables > before and after your changes in order to make sure that there wasn't any > changes? If not, then we should revert all of your changes and do these > checks. > >>>>> > >>>>> > >>>>> On Sep 20, 2016 9:59 AM, "Sebouh Paul" <[log in to unmask]> > wrote: > >>>>> To get technical, I did not look at the recon LCIO files directly, > but rather generated DQM from them. And the DQM did not look different > between the two files. Now what Nathan had suggested was that possibly the > DST maker is reading the hits from a different collection than it was > reading from before, for instance uncallibrated hits instead of callibrated > hits? > >>>>> > >>>>> On Tue, Sep 20, 2016 at 11:21 AM, Omar Moreno <[log in to unmask]> > wrote: > >>>>> Just to make sure, this behavior isn't observed looking at the recon > LCIO files correct? > >>>>> > >>>>> > >>>>> On Sep 20, 2016 7:59 AM, "Sebouh Paul" <[log in to unmask]> > wrote: > >>>>> Without pulse fitting, or the mode7 emulation, all of the hit times > would be at times that are multiples of 4 ns, corresponding to the time of > the first sample crossing the threshold. However, we use a pulse fitting > algorithm, which improves our resolution on timing for the hit, as well as > time-walk corrections. If a fit fails the pulse fitting and the mode-7 > emulation, then we use the time of the first sample that crosses the > threshold. In the "bad files" we see a lot of low energy hits with > multiples of 4ns times, which probably failed both these two methods of > time reconstruction. > >>>>> > >>>>> There is also another thing in here I forgot to mention: there is > also a shift in the timing window of when multiple hits are, between the > "bad" files and the "good" files. What i mean is, you'll see a peak in hit > time, corresponding to the trigger window. This peak is shifted between > files generated using the old version of the DST and the new version. > >>>>> > >>>>> On Tue, Sep 20, 2016 at 10:47 AM, Maurik Holtrop < > [log in to unmask]> wrote: > >>>>> Hello Sebouh, > >>>>> > >>>>> There was a bug in the DST maker where it would only write ECAL hits > that were part of a cluster, and if a hit appeared in two clusters, it > would write that hit twice. This is not the correct behavior. I fixed this > in Issue44 (https://github.com/JeffersonLab/hps-dst/issues/44) and then > merged to master. > >>>>> > >>>>> So, #1, is because now the unrelated and out of time hits are also > in the DST. > >>>>> I don’t think this explains #2. > >>>>> #3 Since the FADC samples at 250Mhz, I would expect all ECAL hit > times to be at multiples of 4ns, but perhaps I am missing something? > >>>>> > >>>>> Best, > >>>>> Maurik > >>>>> > >>>>>> On Sep 20, 2016, at 9:37 AM, Sebouh Paul <[log in to unmask]> > wrote: > >>>>>> > >>>>>> Has anyone made modifications to the DST maker since pass0 began in > June 6? Nathan ran recon and DST maker on some files at the beginning of > August, and there are some problems with the DST files produced: > >>>>>> > >>>>>> 1) 1.5x more ecal hits per event than in DST files generated in > pass0. > >>>>>> 2) two cluster time difference has a much larger sigma than usual. > >>>>>> 3) many ecal hits have hit times that are exact multiples of 4 ns. > >>>>>> > >>>>>> I ran DQM on the corresponding LCIO files, and saw none of these > effects, so the problem must be in the DST maker. > >>>>>> > >>>>>> thanks. > >>>>>> > >>>>>> Use REPLY-ALL to reply to list > >>>>>> > >>>>>> To unsubscribe from the HPS-SOFTWARE list, click the following link: > >>>>>> https://listserv.slac.stanford.edu/cgi-bin/wa? > SUBED1=HPS-SOFTWARE&A=1 > >>>>>> > >>>>> > >>>>> > >>>>> Use REPLY-ALL to reply to list > >>>>> > >>>>> To unsubscribe from the HPS-SOFTWARE list, click the following link: > >>>>> https://listserv.slac.stanford.edu/cgi-bin/wa? > SUBED1=HPS-SOFTWARE&A=1 > >>>>> > >>>>> > >>>>> > >>>>> Use REPLY-ALL to reply to list > >>>>> > >>>>> To unsubscribe from the HPS-SOFTWARE list, click the following link: > >>>>> https://listserv.slac.stanford.edu/cgi-bin/wa? > SUBED1=HPS-SOFTWARE&A=1 > >>>>> > >>>>> > >>>> > >>> > >>> > >>> Use REPLY-ALL to reply to list > >>> > >>> To unsubscribe from the HPS-SOFTWARE list, click the following link: > >>> https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=HPS-SOFTWARE&A=1 > > ######################################################################## Use REPLY-ALL to reply to list To unsubscribe from the HPS-SOFTWARE list, click the following link: https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=HPS-SOFTWARE&A=1