Use of Max-Sample algorithm for gain-switch cases in the barrel #17259

emanueledimarco · 2017-01-24T12:15:04Z

This is a fix for the ECAL local reconstruction that, for EB only, where the problem is visible,
changes the algorithm from multifit to the simple max-sample.
This solution for EB grants the consistency of the (legacy) re-reco data with the Summer16 MC samples already existing [1]. For EE it would introduce a small extra smearing in the data/MC, so it is switched off for the re-reco (since the problem affects only EB).
This should then be customised to be used for 2015-2016 data only.

Depending to what studies for 90X with improved multifit show, related to PR #17205, we could either use them, if positive, or fall back on this max-sample solution, for both EB and EE.
Should this be implemented for 90X as well on top of PR #17205 ?

@bendavid @amassiro @slava77

[1] https://indico.cern.ch/event/590956/contributions/2391934/attachments/1384258/2105710/multifit-gainswitch.pdf

…ples

…pact on data/MC consistency

…es only for EB, to be used for 80X 2016 legacy re-reco

cmsbuild · 2017-01-24T12:15:23Z

A new Pull Request was created by @emanueledimarco (Emanuele Di Marco) for CMSSW_8_0_X.

It involves the following packages:

RecoLocalCalo/EcalRecAlgos
RecoLocalCalo/EcalRecProducers

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

cms-bot commands are listed here #13028

slava77 · 2017-01-24T14:14:27Z

On 1/24/17 4:15 AM, Emanuele Di Marco wrote: Should this be implemented for 90X as well on top of PR #17205 <#17205> ?

the nominal policy is to have the features in 90X first

slava77 · 2017-01-24T14:16:05Z

RecoLocalCalo/EcalRecProducers/plugins/EcalUncalibRecHitWorkerMultiFit.cc

@@ -573,6 +575,7 @@ EcalUncalibRecHitWorkerMultiFit::getAlgoDescription() {
 	      edm::ParameterDescription<bool>("ampErrorCalculation", true, true) and
 	      edm::ParameterDescription<bool>("useLumiInfoRunHeader", true, true) and
 	      edm::ParameterDescription<int>("bunchSpacing", 0, true) and
+              edm::ParameterDescription<bool>("gainSwitchEBMaxSample", true, true) and 


I think that the default here should be "false"

OK, will do

emanueledimarco · 2017-01-24T14:19:05Z

For 90X, I will add it as configurable on top of PR #17205, since it is already merged

slava77 · 2017-01-24T14:19:26Z

@cmsbuild please test

@emanueledimarco
what is needed to enable the feature?
Is it just process.ecalMultiFitUncalibRecHit.gainSwitchEBMaxSample = False ?

cmsbuild · 2017-01-24T14:19:40Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17404/console Started: 2017/01/24 15:21

emanueledimarco · 2017-01-24T14:42:09Z

@slava77
what is needed to enable the feature?
Is it just process.ecalMultiFitUncalibRecHit.gainSwitchEBMaxSample = False ?

The opposite: process.ecalMultiFitUncalibRecHit.algoPSet.gainSwitchEBMaxSample = cms.bool(True)

only for 2016 data for the time being

cmsbuild · 2017-01-24T14:52:02Z

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

slava77 · 2017-01-24T14:52:13Z

On 1/24/17 6:42 AM, Emanuele Di Marco wrote: @slava77 <https://github.com/slava77> what is needed to enable the feature? Is it just process.ecalMultiFitUncalibRecHit.gainSwitchEBMaxSample = False ? The opposite: process.ecalMultiFitUncalibRecHit.gainSwitchEBMaxSample = True only for 2016 data for the time being

Thanks. It is "True" indeed, I copy-pasted too much without thinking.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbnpH2hI672vq1lUKFjy21Q_Xjq8sks5rVg3BgaJpZM4LsMP->.

slava77 · 2017-01-24T14:54:53Z

@cmsbuild please test

cmsbuild · 2017-01-24T14:55:06Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17405/console Started: 2017/01/24 15:56

emanueledimarco · 2017-01-24T14:58:17Z

Forgot to add the validation with 2016 data (from Shervin Nourbakhsh and Giuseppe Fasanella)
@shervin86

slewRate_fix.pdf

slava77 · 2017-03-10T16:39:26Z

@cmsbuild please test

cmsbuild · 2017-03-10T16:39:45Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18345/console Started: 2017/03/10 22:08

cmsbuild · 2017-03-10T22:18:39Z

+1
Tested at: c8b69f0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17259/18345/summary.html

cmsbuild · 2017-03-10T22:18:42Z

Comparison job queued.

cmsbuild · 2017-03-10T22:52:55Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17259/18345/summary.html

slava77 · 2017-03-17T18:11:46Z

+1

for #17259 c8b69f0

updated "max-sample" choice for the gain-switched hit reco; the default behavior is still unchanged. Changes are in sync with Minor improvements to Multifit handling of max sample fallback (91x) #17877 9fbc630
jenkins tests pass
sign off relies on checks done in Minor improvements to Multifit handling of max sample fallback (91x) #17877 9fbc630

cmsbuild · 2017-03-17T18:12:03Z

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. @davidlange6, @smuzaffar

emanueledimarco · 2017-04-03T08:51:51Z

This is still pending and should be:

included in 8.0.X
turned ON for the 2016 legacy re-reco only for the ECAL barrel, i.e.: process.ecalMultiFitUncalibRecHit.algoPSet.gainSwitchEBMaxSample = cms.bool(True)

is the legacy re-reco release built?
@paramatti @fcouderc @shervin86 also follow this.

amassiro · 2017-04-05T14:37:31Z

As discussed today at the AlCa meeting (https://indico.cern.ch/event/626460/), can this be switched on for the legacy re-reco cmssw release?
Cheers, ECAL

slava77 · 2017-04-05T14:42:42Z

On 4/5/17 4:37 PM, Andrea Massironi wrote: As discussed today at the AlCa meeting (https://indico.cern.ch/event/626460/), can this be switched on for the legacy re-reco cmssw release? Cheers, ECAL

we can still preserve data-MC consistency by keeping the default off and will enable this feature in the workflow configuration using

…

--customise_command "process.ecalMultiFitUncalibRecHit.algoPSet.gainSwitchEBMaxSample = cms.bool(True)\n"

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbvHCL3V_E-a17VAMPdTO1R_bCsxgks5rs6csgaJpZM4LsMP->.

amassiro · 2017-04-05T14:45:32Z

Perfect. Thanks a lot for fast reply.

emanueledimarco · 2017-04-05T14:57:09Z

@slava77 ok. But this should be merged in 8.0.X still, in order to be turned on by configuration, right?

slava77 · 2017-04-05T15:05:12Z

On 4/5/17 4:57 PM, Emanuele Di Marco wrote: @slava77 <https://github.com/slava77> ok. But this should be merged in 8.0.X still, right?

yes

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17259 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbkPHcqFj7z7cNF4pFlx5G1mI5LJoks5rs6vFgaJpZM4LsMP->.

davidlange6 · 2017-04-05T20:21:15Z

hi @emanueledimarco - please submit a PR turning this feature on by default. Thanks

emanueledimarco · 2017-04-06T09:11:41Z

@davidlange6 I have understood from @slava77 in #17259 (comment) that we want to leave the default off and turn on this feature by a customise command on the legacy re-reco in 8.0.X.
Is this correct?

davidlange6 · 2017-04-06T09:20:02Z

would prefer to make the default correspond to the "legacy" (or at least the first legacy)

…

On Apr 6, 2017, at 11:11 AM, Emanuele Di Marco ***@***.***> wrote: @davidlange6 I have understood from @slava77 in #17259 (comment) that we want to leave the default off and turn on this feature by a customise command on the legacy re-reco in 8.0.X. Is this correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

emanueledimarco · 2017-04-06T09:24:52Z

fair. Doing it now.

emanueledimarco added 5 commits December 2, 2016 16:46

use max sample in case of any hit with gain switch in the allowed sam…

37f4529

…ples

changes to run many MFs in one job

6e88fd5

for legacy re-reco of 2016 data, max-sample can be used for EB w/o im…

196d29b

…pact on data/MC consistency

add a python switch to turn on the max-sample algo in gain switch cas…

ed57a8e

…es only for EB, to be used for 80X 2016 legacy re-reco

add a python switch to turn on the max-sample algo in gain switch cas…

725190e

…es only for EB, to be used for 80X 2016 legacy re-reco

cmsbuild added this to the Next CMSSW_8_0_X milestone Jan 24, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jan 24, 2017

slava77 reviewed Jan 24, 2017

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Jan 24, 2017

change the default to false

e0d9b1d

cmsbuild added tests-pending and removed tests-started labels Jan 24, 2017

cmsbuild added tests-started and removed tests-pending labels Jan 24, 2017

cmsbuild added tests-started and removed tests-pending labels Mar 10, 2017

cmsbuild added tests-approved and removed tests-started labels Mar 10, 2017

cmsbuild added comparison-available and removed comparison-pending labels Mar 10, 2017

cmsbuild removed pending-signatures reconstruction-pending labels Mar 17, 2017

cmsbuild added fully-signed reconstruction-approved labels Mar 17, 2017

davidlange6 merged commit b869ac7 into cms-sw:CMSSW_8_0_X Apr 5, 2017

This was referenced Apr 6, 2017

Activate Max-Sample algorithm in EB by default for 80X legacy re-reco #18239

Closed

Activate Max-Sample algorithm in EB by default for 80X legacy re-reco #18240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use of Max-Sample algorithm for gain-switch cases in the barrel #17259

Use of Max-Sample algorithm for gain-switch cases in the barrel #17259

emanueledimarco commented Jan 24, 2017

cmsbuild commented Jan 24, 2017

slava77 commented Jan 24, 2017 via email

slava77 Jan 24, 2017

emanueledimarco Jan 24, 2017

emanueledimarco commented Jan 24, 2017

slava77 commented Jan 24, 2017

cmsbuild commented Jan 24, 2017 •

edited

emanueledimarco commented Jan 24, 2017 •

edited

cmsbuild commented Jan 24, 2017

slava77 commented Jan 24, 2017 via email

slava77 commented Jan 24, 2017

cmsbuild commented Jan 24, 2017 •

edited

emanueledimarco commented Jan 24, 2017 •

edited

slava77 commented Mar 10, 2017

cmsbuild commented Mar 10, 2017 •

edited

cmsbuild commented Mar 10, 2017

cmsbuild commented Mar 10, 2017

cmsbuild commented Mar 10, 2017

slava77 commented Mar 17, 2017

cmsbuild commented Mar 17, 2017

emanueledimarco commented Apr 3, 2017 •

edited

amassiro commented Apr 5, 2017

slava77 commented Apr 5, 2017 via email

amassiro commented Apr 5, 2017

emanueledimarco commented Apr 5, 2017 •

edited

slava77 commented Apr 5, 2017 via email

davidlange6 commented Apr 5, 2017

emanueledimarco commented Apr 6, 2017

davidlange6 commented Apr 6, 2017 via email

emanueledimarco commented Apr 6, 2017

Use of Max-Sample algorithm for gain-switch cases in the barrel #17259

Use of Max-Sample algorithm for gain-switch cases in the barrel #17259

Conversation

emanueledimarco commented Jan 24, 2017

cmsbuild commented Jan 24, 2017

slava77 commented Jan 24, 2017 via email

slava77 Jan 24, 2017

Choose a reason for hiding this comment

emanueledimarco Jan 24, 2017

Choose a reason for hiding this comment

emanueledimarco commented Jan 24, 2017

slava77 commented Jan 24, 2017

cmsbuild commented Jan 24, 2017 • edited

emanueledimarco commented Jan 24, 2017 • edited

cmsbuild commented Jan 24, 2017

slava77 commented Jan 24, 2017 via email

slava77 commented Jan 24, 2017

cmsbuild commented Jan 24, 2017 • edited

emanueledimarco commented Jan 24, 2017 • edited

slava77 commented Mar 10, 2017

cmsbuild commented Mar 10, 2017 • edited

cmsbuild commented Mar 10, 2017

cmsbuild commented Mar 10, 2017

cmsbuild commented Mar 10, 2017

slava77 commented Mar 17, 2017

cmsbuild commented Mar 17, 2017

emanueledimarco commented Apr 3, 2017 • edited

amassiro commented Apr 5, 2017

slava77 commented Apr 5, 2017 via email

amassiro commented Apr 5, 2017

emanueledimarco commented Apr 5, 2017 • edited

slava77 commented Apr 5, 2017 via email

davidlange6 commented Apr 5, 2017

emanueledimarco commented Apr 6, 2017

davidlange6 commented Apr 6, 2017 via email

emanueledimarco commented Apr 6, 2017

cmsbuild commented Jan 24, 2017 •

edited

emanueledimarco commented Jan 24, 2017 •

edited

cmsbuild commented Jan 24, 2017 •

edited

emanueledimarco commented Jan 24, 2017 •

edited

cmsbuild commented Mar 10, 2017 •

edited

emanueledimarco commented Apr 3, 2017 •

edited

emanueledimarco commented Apr 5, 2017 •

edited