Skip to content
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

Enable new pixel cluster position features #5384

Merged
merged 1 commit into from Oct 21, 2014

Conversation

cmsbuild
Copy link
Contributor

Three new features for pixel cluster position estimation (CPE) were included in 7_1_0, but not yet activated since they rely on global tag information, available since MC_72_V2 and GR_R_72_V3.
Here these features are activated.

  1. Enable GenErrors database object for generic CPE, which are faster than the full template database objects previously used.
  2. Enable to take the lorentz angle width from database (instead of configuration)
  3. Enable lorentz angle estimate from alignment from database. Currently database entries are 0 such that this has no effect until alignment input.
    It was checked that none of these features changes pixel cluster position resolution within 1% of the resolution.
    Automatically ported from CMSSW_7_2_X Enable new pixel cluster position features #5238

@cmsbuild
Copy link
Contributor Author

A new Pull Request was created by @cmsbuild for CMSSW_7_3_X.

Enable new pixel cluster position features

It involves the following packages:

RecoLocalTracker/SiPixelRecHits

@cmsbuild, @nclopezo, @StoyanStoynev, @slava77 can you please review it and eventually sign? Thanks.
@GiacomoSguazzoni, @rovere, @VinInn, @dkotlins, @gpetruc, @cerati, @threus, @venturia this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor Author

-1
Tested at: baeaa02
When I ran the RelVals I found an error in the following worklfows:
4.53 step2

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT.log
----- Begin Fatal Exception 18-Sep-2014 19:06:08 CEST-----------------------
An exception of category 'NoRecordFromDependentRecord' occurred while
   [0] Processing run: 194533
   [1] Calling beginRun for unscheduled module EventSetupRecordDataGetter/'hltGetConditions'
   [2] Using EventSetup component PixelCPEGenericESProducer/'hltESPPixelCPEGeneric' to make data PixelClusterParameterEstimator/'hltESPPixelCPEGeneric' in record TkPixelCPERecord
Exception Message:
No "SiPixelGenErrorDBObjectRcd" record found in the dependent record "TkPixelCPERecord".
 Please add an ESSource or ESProducer that delivers the "SiPixelGenErrorDBObjectRcd" record.
----- End Fatal Exception -------------------------------------------------

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5384/7/summary.html

@slava77
Copy link
Contributor

slava77 commented Sep 19, 2014

@ahinzmann
Hi Andreas,
how is the progress with getting the SiPixelGenErrorDBObjectRcd in all needed GTs?

@ahinzmann
Copy link
Contributor

Here is the status from Pal from last week:

I have updated the pixel GT summary page, which has not been used since 53X:
https://twiki.cern.ch/twiki/bin/viewauth/CMS/PixelDBTags
Clicking on the 72X GTs I see the following:

  1. the run1 and run2 pp mc and data reprocessing GTs are OK, they have GenError and labelled LA
  2. HLT and express don't
  3. phase1 and phase2 MC don't
  4. HI MC don't

@hidaspal
Could you keep this thread updated about the latest status? Have the missing tags been requested?

@slava77
Copy link
Contributor

slava77 commented Sep 23, 2014

So, items 2, 3, 4 still need to appear in DB, right?
Is somebody working/pushing it?

@ahinzmann
Copy link
Contributor

As far as I understand from Pal this will happen after #5386 is included in the first 73X pre-release, but @hidaspal should confirm, as I missed the discussion at the pixel reco meeting.

@dkotlins
Copy link
Contributor

Slava,
Sorry, I do not understand. What do you mean by items 2,3,4?
Are you refering to Pal’s presentation?
Danek


Danek Bohdan Kotlinski
danek.kotlinski@psi.ch
Phone: +41 563104030 (PSI), 0764875909 or 165909 (CERN)
Paul Scherrer Institut OFLC/004

On 23 Sep 2014, at 16:30, Slava Krutelyov notifications@github.com wrote:

So, items 2, 3, 4 still need to appear in DB, right?
Is somebody working/pushing it?


Reply to this email directly or view it on GitHub.

@davidlange6
Copy link
Contributor

Hi Danek

I believe the 1-4 comes from the discussion of the thread - Its this list of GTs:

Clicking on the 72X GTs I see the following:

  1. the run1 and run2 pp mc and data reprocessing GTs are OK, they have GenError and labelled LA
  2. HLT and express don't
  3. phase1 and phase2 MC don't
  4. HI MC don't

On Sep 24, 2014, at 9:47 AM, dkotlins notifications@github.com
wrote:

Slava,
Sorry, I do not understand. What do you mean by items 2,3,4?
Are you refering to Pal’s presentation?
Danek


Danek Bohdan Kotlinski
danek.kotlinski@psi.ch
Phone: +41 563104030 (PSI), 0764875909 or 165909 (CERN)
Paul Scherrer Institut OFLC/004

On 23 Sep 2014, at 16:30, Slava Krutelyov notifications@github.com wrote:

So, items 2, 3, 4 still need to appear in DB, right?
Is somebody working/pushing it?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@dkotlins
Copy link
Contributor

David,
Thanks, now it is clear.

I think/hope that Pal will push 2 & 4. I will check with him.
There is not reason for a delay, these are the same objects as in (1).
(3) is special, we do not have any real DB objects for phase1/phase2.
For phase we hope to have them at the end of 2014. For phase2, I think the
detector has to be designed first, anyway this is for later.

Danek


Danek Bohdan Kotlinski
danek.kotlinski@psi.ch
Phone: +41 563104030 (PSI), 0764875909 or 165909 (CERN)
Paul Scherrer Institut OFLC/004

On 24 Sep 2014, at 10:03, David Lange notifications@github.com wrote:

Hi Danek

I believe the 1-4 comes from the discussion of the thread - Its this list of GTs:

Clicking on the 72X GTs I see the following:

  1. the run1 and run2 pp mc and data reprocessing GTs are OK, they have GenError and labelled LA
  2. HLT and express don't
  3. phase1 and phase2 MC don't
  4. HI MC don't

On Sep 24, 2014, at 9:47 AM, dkotlins notifications@github.com
wrote:

Slava,
Sorry, I do not understand. What do you mean by items 2,3,4?
Are you refering to Pal’s presentation?
Danek


Danek Bohdan Kotlinski
danek.kotlinski@psi.ch
Phone: +41 563104030 (PSI), 0764875909 or 165909 (CERN)
Paul Scherrer Institut OFLC/004

On 23 Sep 2014, at 16:30, Slava Krutelyov notifications@github.com wrote:

So, items 2, 3, 4 still need to appear in DB, right?
Is somebody working/pushing it?


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub.

@hidaspal
Copy link
Contributor

Sorry, this thread remained hidden in my git folder. I shall try to get information about the tag inclusion in HLT GTs. It is not automatic now.
a) The old GT Collector (https://cms-conddb.cern.ch/gtc/) does not know SiPixelGenErrorDBObjectRcd
b) The new GT manager is not operational yet, does not know SiPixelGenErrorDBObject
I shall ask alcadb to include SiPixelGenErrorDBObject_38T_v2_hlt in the next Run II HLT global tag and we shall see it.

@hidaspal
Copy link
Contributor

I have sent this message for SiPixelGenError the HLT GT:
https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/1560.html

@slava77
Copy link
Contributor

slava77 commented Sep 30, 2014

Hi.
I just wanted to check (or/and also for the record in this PR thread): are the GTs available already?

@dkotlins
Copy link
Contributor

Pal,
Have you received any news concerning the HLT GTs?
Best regards,
Danek

-----Original Message-----
From: Pal Hidas [mailto:notifications@github.com]
Sent: Wed 9/24/2014 3:06 PM
To: cms-sw/cmssw
Cc: Kotlinski Bohdan
Subject: Re: [cmssw] Enable new pixel cluster position features (#5384)

I have sent this message for SiPixelGenError the HLT GT:
https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/1560.html


Reply to this email directly or view it on GitHub:
#5384 (comment)

@hidaspal
Copy link
Contributor

hidaspal commented Oct 1, 2014

Hi Danek,

The last info was, that we have to wait for the 73X Gts.
They will be created after CMSSW_7_2_0_pre7 would be issued.
Then they will open the 73X Gt queues.

Cheers,
Pal

On Tue, 30 Sep 2014, dkotlins wrote:

Pal,
Have you received any news concerning the HLT GTs?
Best regards,
Danek

-----Original Message-----
From: Pal Hidas [mailto:notifications@github.com]
Sent: Wed 9/24/2014 3:06 PM
To: cms-sw/cmssw
Cc: Kotlinski Bohdan
Subject: Re: [cmssw] Enable new pixel cluster position features (#5384)

I have sent this message for SiPixelGenError the HLT GT:
https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/1560.html


Reply to this email directly or view it on GitHub:
#5384 (comment)

?
Reply to this email directly or view it onGitHub.[4940954__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcyNzcwOTE5NSwiZGF0YSI6eyJpZCI6NDI3ODgzNzl9fQ==--94dc43abaf369
301333e22e2132c7fea9a08dee4.gif]

@dkotlins
Copy link
Contributor

dkotlins commented Oct 1, 2014

Slava,
The short answer is NO.
I am not sure what to do, maybe you have a way to speed it up.

The DB object have not been accepted by AlcaDB for a long time because
the software was not in git (if I understand it right?).
Now we cannot make modification to the software because the DB objects
are not there.
I hear now that the DB objects will only be present in 73 GTs.
Which I guess means that we will not be able to finalize the software
in git until 74?
The whole thing has now dragged for 1/2 year.
Best regards,
Danek

-----Original Message-----
From: Slava Krutelyov [mailto:notifications@github.com]
Sent: Tue 9/30/2014 3:12 PM
To: cms-sw/cmssw
Cc: Kotlinski Bohdan
Subject: Re: [cmssw] Enable new pixel cluster position features (#5384)

Hi.
I just wanted to check (or/and also for the record in this PR thread): are the GTs available already?


Reply to this email directly or view it on GitHub:
#5384 (comment)

@slava77
Copy link
Contributor

slava77 commented Oct 1, 2014

Uhm, I hope the picture is not that grim.
I'd naively expect the payloads/GTs to show up after pre7 is built
or in 730pre1 (should be right after pre7)

@hidaspal
Copy link
Contributor

@slava77
All the 73X GTs have the latest SiPixel tags. Please, see
https://indico.cern.ch/event/337180/session/4/contribution/10/material/slides/0.pdf
for details. As I see, comparison and tests are OK, so only resonstruction signature is pending.

@slava77
Copy link
Contributor

slava77 commented Oct 20, 2014

ok, I will check this later today

@slava77
Copy link
Contributor

slava77 commented Oct 20, 2014

Just to double-check: what was the first IB with the full set of GTs included?

@diguida
Copy link
Contributor

diguida commented Oct 20, 2014

@slava77
CMSSW_7_3_X_2014-10-14-0200

@slava77
Copy link
Contributor

slava77 commented Oct 21, 2014

Here are a few more plots (not noticed in the summary from #5238.)

  • most tested workflows show pretty small featureless variations in distributions. The gsf electron tracks and conversion step tracks vary the most (typical for small regressions changes in tracking).
  • some of the most visible ones show up in 2012D data workflow 4.77 ZEE skim (Photon2012B 4.53 wasn't run with enough stats or didn't have enough diffs). Since the time of testing Enable new pixel cluster position features #5238 we removed pixelTracks from the RECO outputs, so, one of the most discrepant plots (showing up in pixelTracks) is no more.

In 4.77:

  • softPFElectron sip2d is wider
    all_sign426vsorig_zelskim2012dwf4p77c_recosoftleptontaginfos_softpfelectronstaginfos__rereco_obj_m_leptons_second_sip2d
  • smaller btag efficiency for jets (ok, I suppose, since this is light-flavor sample; if the MVAs were trained on MC, should be OK)
    wf4p77_csv_0-1-eff
  • about 0.1% drop in the number of tracks, mainly in the chi2 tail (0 chi2 prob): after pt cut of 1 GeV, almost the same number of tracks come out (-1 / 65K, could be a coincidence of migrations though), but there is a noticeable increase in chi20 and the number of layers or hits per track is lower
    wf4p77_hits_pt1
  • dedx has some lower number of MIP tracks
    wf4p77_hits_dedxmip
  • visibly fewer iter0 seeds (-0.7%), this makes sense given smaller hit uncertainties
    wf4p77_iter0_seeds

All these differences are consistent with the low-level change in the hit uncertainties.
And these, in turn are expected to change as discussed in https://hypernews.cern.ch/HyperNews/CMS/get/pixelOfflineSW/1071.html?inline=-1 (pointed by Andreas).

@slava77
Copy link
Contributor

slava77 commented Oct 21, 2014

+1

for baeaa02

tested in CMSSW_7_3_X_2014-10-14-1400 (test area sign426).

all workflows that failed before (as of early Sept in #5238 testing) now pass (since GTs are now in)

@cmsbuild
Copy link
Contributor Author

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_3_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@dkotlins
Copy link
Contributor

Slava,
Indeed, I think we expect some changes.
The hit errors are somewhat different, and there is a small change in some
Lorentz Angle parameters.
Best regards,
Danek

-----Original Message-----
From: Slava Krutelyov [mailto:notifications@github.com]
Sent: Tue 10/21/2014 2:33 AM
To: cms-sw/cmssw
Cc: Kotlinski Bohdan
Subject: Re: [cmssw] Enable new pixel cluster position features (#5384)

Here are a few more plots (not noticed in the summary from #5238.)

  • most tested workflows show pretty small featureless variations in distributions. The gsf electron tracks and conversion step tracks vary the most (typical for small regressions changes in tracking).
  • some of the most visible ones show up in 2012D data workflow 4.77 ZEE skim (Photon2012B 4.53 wasn't run with enough stats or didn't have enough diffs). Since the time of testing Enable new pixel cluster position features #5238 we removed pixelTracks from the RECO outputs, so, one of the most discrepant plots (showing up in pixelTracks) is no more.

In 4.77:

  • softPFElectron sip2d is wider
    all_sign426vsorig_zelskim2012dwf4p77c_recosoftleptontaginfos_softpfelectronstaginfos__rereco_obj_m_leptons_second_sip2d
  • smaller btag efficiency for jets (ok, I suppose, since this is light-flavor sample; if the MVAs were trained on MC, should be OK)
    wf4p77_csv_0-1-eff
  • about 0.1% drop in the number of tracks, mainly in the chi2 tail (0 chi2 prob): after pt cut of 1 GeV, almost the same number of tracks come out (-1 / 65K, could be a coincidence of migrations though), but there is a noticeable increase in chi20 and the number of layers or hits per track is lower
    wf4p77_hits_pt1
  • dedx has some lower number of MIP tracks
    wf4p77_hits_dedxmip
  • visibly fewer iter0 seeds (-0.7%), this makes sense given smaller hit uncertainties
    wf4p77_iter0_seeds

All these differences are consistent with the low-level change in the hit uncertainties.
And these, in turn are expected to change as discussed in https://hypernews.cern.ch/HyperNews/CMS/get/pixelOfflineSW/1071.html?inline=-1 (pointed by Andreas).


Reply to this email directly or view it on GitHub:
#5384 (comment)

nclopezo added a commit that referenced this pull request Oct 21, 2014
RecoLocalTracker/SiPixelRecHits -- Enable new pixel cluster position features
@nclopezo nclopezo merged commit de3fbe6 into cms-sw:CMSSW_7_3_X Oct 21, 2014
@venturia
Copy link
Contributor

Inform the alignment group about this (small) change: again it is something which has to be reabsorbed in the next APE determination.

                                  Andrea

On Oct 21, 2014, at 9:15 , David Mendez <notifications@github.commailto:notifications@github.com> wrote:

Merged #5384#5384.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5384#event-181348861.

@diguida
Copy link
Contributor

diguida commented Oct 21, 2014

@venturia
Doing this on hn. For the moment, let me ping the conveners: @mmusich @frmeier

@mmusich
Copy link
Contributor

mmusich commented Oct 22, 2014

@diguida @venturia, acknowledged. We wil first check for the impact on alignment observables.

@diguida
Copy link
Contributor

diguida commented Oct 22, 2014

@mmusich
Yesterday I forgot to send the hn message. Do you want me to do this now?

@mmusich
Copy link
Contributor

mmusich commented Oct 22, 2014

@diguida yes please, for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants