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

Mustache SuperCluster Regression for 800 #13000

Merged
merged 7 commits into from Feb 3, 2016

Conversation

matteosan1
Copy link
Contributor

The PR updates the online and offline regression for mustache SC.
At the moment the corrections are off by default due to the useRegression flag set to false.
Once activated:

  • Offline regression impact only pixel-matching since it is the only place where the energy is taken directly from the mustache supercluster.
  • Online regression represent the "highest" level energy corrections used at HLT for electrons and photons.
    We plan to switch the corrections on once the payloads will be integrated into the DB.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @matteosan1 (Matteo Sani) for CMSSW_8_0_X.

It involves the following packages:

RecoEcal/EgammaClusterAlgos
RecoEcal/EgammaClusterProducers
RecoEgamma/EgammaTools

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @argiro, @lgray this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@@ -0,0 +1,316 @@
//#include <TFile.h>
#include "../interface/SCEnergyCorrectorSemiParm.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relative paths are not allowed

@slava77
Copy link
Contributor

slava77 commented Jan 19, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10595/console

@cmsbuild
Copy link
Contributor

-1
Tested at: 73373dc
When I ran the RelVals I found an error in the following worklfows:
134.911 step2

runTheMatrix-results/134.911_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT/step2_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT.log
----- Begin Fatal Exception 19-Jan-2016 23:54:31 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
----- Begin Fatal Exception 19-Jan-2016 23:59:08 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

1306.0 step2

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step2_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 20-Jan-2016 00:00:53 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

1330.0 step2

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step2_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 20-Jan-2016 00:06:04 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

25202.0 step2

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step2_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
----- Begin Fatal Exception 20-Jan-2016 00:13:02 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

50202.0 step2

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step2_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log
----- Begin Fatal Exception 20-Jan-2016 00:14:50 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFECALSuperClusterProducer label='hltParticleFlowSuperClusterECALL1Seeded'
Exception Message:
Illegal parameters found in configuration.  The parameters are named:
 'regressionKeyEB'
 'regressionKeyEE'
You could be trying to use parameter names that are not
allowed for this plugin or they could be misspelled.
----- End Fatal Exception -------------------------------------------------

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

@cmsbuild
Copy link
Contributor

Pull request #13000 was updated. @cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Jan 20, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10598/console

#include "RecoEcal/EgammaCoreTools/interface/EcalClusterTools.h"
#include "DataFormats/EcalDetId/interface/EcalSubdetector.h"
#include "DataFormats/VertexReco/interface/Vertex.h"
#include "TStreamerInfo.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TStreamerInfo looks unnecessary as well

@cmsbuild
Copy link
Contributor

-1
Tested at: 306067a
When I ran the RelVals I found an error in the following worklfows:
5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log

4.22 step2

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log

8.0 step1

runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step1_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log

9.0 step1

runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step1_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log

25.0 step1

runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step1_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log

134.911 step2

runTheMatrix-results/134.911_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT/step2_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT.log

140.53 step2

runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log

1306.0 step1

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

1330.0 step1

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step1_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log

101.0 step1

runTheMatrix-results/101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log

25202.0 step1

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step1_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log

50202.0 step1

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step1_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log

1000.0 step2

runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log

1001.0 step2

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4.log

1003.0 step2

runTheMatrix-results/1003.0_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM/step2_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM.log

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 1, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 2, 2016

@mmusich
Copy link
Contributor

mmusich commented Feb 2, 2016

+1

@slava77
Copy link
Contributor

slava77 commented Feb 2, 2016

@mmusich I'm hoping that there is no more update in the autoCond.py planned imminently.
If that changes, this PR will hopefully become unmergeable, but can instead override changes.
So, @matteosan1, it may be better to remove the commit changing autoCond

@mmusich
Copy link
Contributor

mmusich commented Feb 2, 2016

@slava77 not imminent, but soon we will have to add few ingredients for the final definition 80X asymptotic scenarios, i.e. Ecal noise, Ecal Selective readout, and a bunch of other items, so yes it would be definitely better to remove the commit

@slava77
Copy link
Contributor

slava77 commented Feb 2, 2016

@matteosan1 which energies did you plot in #13000 (comment) ?
Are these cluster level or the final candidate (e.g. gedGsfElectron) pt or energy()?

Most of the significant changes that I observe at the high level object level (gedGsfElectron, gedPhoton, and particleFlow candidates) are all localized to pt~10 GeV which seems to be just a cluster threshold effects:
e.g. gamma pt=10 GeV gun events:
all_sign665vsorig_singlegamma13pt10wf1318p0c_recophotons_gedphotons__reco_obj_et

vs gamma 35 GeV sample
all_sign665vsorig_singlegamma13pt35wf1319p0c_recophotons_gedphotons__reco_obj_et

Similar story for gsfElectrons and PFCandidates (particleFlow) with electron or photon ID.

DQM doesn't seem to have much to offer in terms of ECAL SC reco/true energy plots.
This one in electrons (in ele 35 GeV gun sample) appears to show an improvement of a similar kind posted by Matteo
wf1317_elescovertrue_pf_vs_egamma

At the lower level, the supercluster energies are changing noticeably: plots from H->gammagamma (workflow 1332, no pileup)
wf1332_ebsc_e
wf1332_eesc_e

There is a rather significant change in R9 plots (but I suspect these are plots of an incorrect variable; if the actual "good" R9 is changing: it looks like more clusters are pushed to lower R9 values, which is not so good)
here is a plot in H->gammagamma
wf1332_ebsc_r9
wf1332_eesc_r9

Based on the PR description, the effects appear to be somewhat expected, but some comments are needed to confirm the behavior is expected before this can be signed off.

@matteosan1
Copy link
Contributor Author

Hi Slava,
SC energy plots look consistent to mine, I am a bit surprised of R9 and I am running a simple test to cross-check it.

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2016

On 2/3/16 5:02 AM, Matteo Sani wrote:

Hi Slava,
SC energy plots look consistent to mine, I am a bit surprised of R9 and
I am running a simple test to cross-check it.

I recall that in the past we had R9 plot in DQM which was not computed
correctly (using inconsistent numerator and denominator).
It may be worth to check the code that makes the plot (it should be easy
given the path and the histogram name).
Someone in ECAL/EGM should fix it, if it is the case here.


Reply to this email directly or view it on GitHub
#13000 (comment).

@matteosan1
Copy link
Contributor Author

Hi Slava,
I don't see big issues with R9 in my plots (attached, blue is new, red is old). I'll follow up on the DQM issue with ECAL guys.
ele_r9_eb
ele_r9_ee

@slava77
Copy link
Contributor

slava77 commented Feb 3, 2016

+1

for #13000 09e078a

@davidlange6
while it's still coherent, please merge this at your earliest.
If there are some issues and this can't be merged imminently, this PR will have to be updated to remove changes in autoCond.py file #13000 (comment)

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 3, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Feb 3, 2016
Mustache SuperCluster Regression for 800
@cmsbuild cmsbuild merged commit bcb909d into cms-sw:CMSSW_8_0_X Feb 3, 2016
@matteosan1 matteosan1 deleted the mustacheRegression_800 branch April 3, 2016 13:49
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

6 participants