LISTSERV mailing list manager LISTSERV 16.5

Help for HPS-SOFTWARE Archives


HPS-SOFTWARE Archives

HPS-SOFTWARE Archives


HPS-SOFTWARE@LISTSERV.SLAC.STANFORD.EDU


View:

Message:

[

First

|

Previous

|

Next

|

Last

]

By Topic:

[

First

|

Previous

|

Next

|

Last

]

By Author:

[

First

|

Previous

|

Next

|

Last

]

Font:

Proportional Font

LISTSERV Archives

LISTSERV Archives

HPS-SOFTWARE Home

HPS-SOFTWARE Home

HPS-SOFTWARE  September 2016

HPS-SOFTWARE September 2016

Subject:

Re: changes to DST maker?

From:

Nathan Baltzell <[log in to unmask]>

Reply-To:

Software for the Heavy Photon Search Experiment <[log in to unmask]>

Date:

Wed, 21 Sep 2016 21:31:29 -0400

Content-Type:

text/plain

Parts/Attachments:

Parts/Attachments

text/plain (265 lines)

I expect we'll definitely be reverting A to keep collection name unchanged.
But still a useful test of B.

-Nathan


On Sep 21, 2016, at 9:14 PM, Maurik Holtrop <[log in to unmask]> wrote:

> Hello Sebouh, all,
> 
> Could you please try the updated version of hps-dst on the hps-dst_Issue44 branch on GitHub?
> 
> (i.e. in your source directory: git pull; git checkout hps-dst_Issue44; then rebuild the dst-maker )
> 
> That version should: 
> 
> A) Test to see of the TimeCorrEcalHits exist in the lcio file. If it does, use it, if it does not, fall back to EcalCalHits.
> B) Use an std::pair<int,int> for the map key, with the first int the id0 of the hit and the second int 10*getTime(). This should uniquely identify any hit. 
> 
> Please let me know if you still see any issue with that version. 
> 
> Tomorrow in the software meeting we will discuss if we are going to keep the “TimeCorrEcalHits” collection, but either way, I think the version I just put up should work.
> 
> Best,
> 	Maurik
> 
> 
>> On Sep 21, 2016, at 3:26 PM, Maurik Holtrop <[log in to unmask]> wrote:
>> 
>> Yes. 1) They “should” be there. 2) If you want to check anything regarding ECAL you need all the hits, 3) The previous scheme would write shared hits twice.
>> I am the one who needed this, as I was checking the clustering, looking for photons.
>> 
>> Furthermore, the 2016 recon was changed. The new recon changed the name of the corrected Ecal hits to “TimeCorrEcalHits", so if you now want to analyze that data, whether with the DST or directly, you need to use this new name. I don’t think that is a good idea, I think it is confusing. I agree with Nathan that we should revert to the old name, “EcalCalHits”.
>> 
>> Best,
>> 	Maurik
>> 
>> 
>> 
>>> On Sep 21, 2016, at 3:08 PM, Omar Moreno <[log in to unmask]> wrote:
>>> 
>>> Maurik, was there a reason that you decided to add all Ecal hits in an event to the DST?  Did someone request it or did you just feel like they needed to be in there?  The reason I ask is because if everyone is OK with just having hits associated with clusters, then we can just revert the code to what I originally had and just apply a fix for the repeated hits.  This can be easily done using a single map, without the need of using Cell ID's.  The original version of the code worked with both the 2015 and 2016 recon so there will be no need to make changes to the LCIO recon.
>>> 
>>> On Wed, Sep 21, 2016 at 11:43 AM, Maurik Holtrop <[log in to unmask]> wrote:
>>> Hello Sebouh,
>>> 
>>> Thank you for looking further into this. It is very useful debugging too.
>>> 
>>> Unfortunately, I don’t think it is quite so simple as using the other collection. The Engrun2015 data does not have the TimeCorrEcalHits collection at all, it only has EcalCalHits. So if you switch the collection that is being used to TimeCorrEcalHits, you now break the older passes and all the 2015 data.
>>> 
>>> Perhaps it is better for us to fix the Java code so that it does not introduce yet another collection of Ecal hits? 
>>> ​ ​ 
>>> 
>>> Also, I thought that the CellID0 identified a hit uniquely, but as you point out, that was a misunderstanding. The SVT code uses a similar scheme to build the map to the hits.  I should look into that part of the code as well. Instead of using two maps, with a second “overflow” map, it would be better design to use a single map where each hit has a unique identification. I can look into what would work best. Perhaps some combination of the id0 and the time.
>>> 
>>> ​When I wrote the SVT code, I made distributions to make sure things worked as expected.  ​ ​Please don't touch that code ... ​
>>> 
>>> So please wait with a commit until we can figure out what is the best path forward.
>>> 
>>> Best,
>>> 	Maurik
>>> 
>>>> On Sep 21, 2016, at 2:16 PM, 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
>>>> 
>>>> 
>>>> <EcalDataWriter.cxx>
>>> 
>>> 
>>> 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

Top of Message | Previous Page | Permalink

Advanced Options


Options

Log In

Log In

Get Password

Get Password


Search Archives

Search Archives


Subscribe or Unsubscribe

Subscribe or Unsubscribe


Archives

February 2024
January 2024
December 2023
November 2023
October 2023
September 2023
August 2023
July 2023
June 2023
May 2023
April 2023
March 2023
February 2023
January 2023
December 2022
November 2022
October 2022
September 2022
August 2022
June 2022
April 2022
March 2022
February 2022
January 2022
December 2021
November 2021
October 2021
September 2021
August 2021
July 2021
June 2021
May 2021
April 2021
March 2021
February 2021
January 2021
December 2020
November 2020
October 2020
September 2020
August 2020
July 2020
June 2020
May 2020
April 2020
March 2020
February 2020
January 2020
December 2019
November 2019
October 2019
September 2019
August 2019
July 2019
June 2019
May 2019
April 2019
March 2019
February 2019
January 2019
December 2018
November 2018
October 2018
September 2018
August 2018
July 2018
June 2018
May 2018
April 2018
March 2018
February 2018
January 2018
December 2017
November 2017
October 2017
September 2017
August 2017
July 2017
June 2017
May 2017
April 2017
March 2017
February 2017
January 2017
December 2016
November 2016
October 2016
September 2016
August 2016
July 2016
June 2016
May 2016
April 2016
March 2016
February 2016
January 2016
December 2015
November 2015
October 2015
September 2015
August 2015
July 2015
June 2015
May 2015
April 2015
March 2015
February 2015
January 2015
December 2014
November 2014
October 2014
September 2014
August 2014
July 2014
June 2014
May 2014
April 2014
March 2014
February 2014
January 2014
December 2013
November 2013
October 2013
September 2013
August 2013
July 2013
June 2013
May 2013
April 2013
March 2013
February 2013
January 2013
December 2012
November 2012
October 2012
September 2012
August 2012
July 2012
June 2012
May 2012
April 2012
March 2012
February 2012
January 2012
December 2011
November 2011
October 2011
September 2011
August 2011
July 2011
June 2011
May 2011
April 2011
March 2011

ATOM RSS1 RSS2



LISTSERV.SLAC.STANFORD.EDU

Secured by F-Secure Anti-Virus CataList Email List Search Powered by the LISTSERV Email List Manager

Privacy Notice, Security Notice and Terms of Use