Print

Print


Before checking anything in, can you make some comparison plots between
pass0 and what you see now in order to check that they are consistent?

On Sep 21, 2016 11:16 AM, "Sebouh Paul" <[log in to unmask]> wrote:

> 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