HBHE: add M2-chi2 into the HBHErecHit data formats #15916

mariadalfonso · 2016-09-20T07:59:45Z

Added the Method 2-chisquare into the HBHERecHit data formats.
@igv4321

cmsbuild · 2016-09-20T08:00:24Z

A new Pull Request was created by @mariadalfonso for CMSSW_8_1_X.

It involves the following packages:

DataFormats/HcalRecHit
RecoLocalCalo/HcalRecAlgos

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

cms-bot commands are list here #13028

slava77 · 2016-09-20T12:07:40Z

@mariadalfonso
is it really just for M2?
The name is generic enough to support other use cases.
Is there an equivalent of chi2 in M3?

slava77 · 2016-09-20T12:09:20Z

@cmsbuild please test

cmsbuild · 2016-09-20T12:09:40Z

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

cmsbuild · 2016-09-20T14:04:50Z

+1
Tested at: a2c6139
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15916/15283/summary.html

mariadalfonso · 2016-09-20T14:10:45Z

@slava77
M3 doens't have a meaningful value @jaylawhorn

cmsbuild · 2016-09-20T15:09:18Z

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

slava77 · 2016-09-22T18:05:12Z

@mariadalfonso @igv4321
is chi2 enough for your needs?
Should ndf be added as well? (we have one and 3-pulse fit versions active now; unclear if 2-pulse can show up in the future or some shape parameter be added. So, e.g. chi2~10 may be on a high side for 1-pulse, but kind of OK for 3-pulse).

Also, as more values are added, to the event content, the cost on disk increases, even if it is with a shorter life time.
Consider packing this as some e.g int/255, which should be well compressable, compared to typically worse than 2 compression factor for floats.

slava77 · 2016-09-23T19:04:37Z

DataFormats/HcalRecHit/interface/HBHERecHit.h

@@ -22,6 +22,9 @@ class HBHERecHit : public CaloRecHit {
  /// get the id
  inline HcalDetId id() const { return HcalDetId(detid()); }

+  inline void setChiSquared(const float chi2) {chiSquared_ = chi2;}
+  inline float getChiSquared() const {return chiSquared_;}


please change to chiSquared to preserve the style with all other access methods, which do not have a "get" prefix.

... better yet, use chi2 in the method names to match what we have in track and ecal hit classes.

@slava77 renamed getChiSquared into chi2

cmsbuild · 2016-09-24T16:23:19Z

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

mariadalfonso · 2016-09-24T16:46:15Z

@slava77 The ndof is not needed at the moment.
There is a flag in the rechit to distinguish 1 and 3-pulse fits, and in the future we can store the reduced chi2.

slava77 · 2016-09-24T17:07:55Z

@cmsbuild please test

cmsbuild · 2016-09-24T17:08:13Z

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

cmsbuild · 2016-09-24T18:47:19Z

+1
Tested at: be23969
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15916/15370/summary.html

cmsbuild · 2016-09-24T19:52:40Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

10424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017NewFPix_GenSimFull+DigiFull_2017NewFPix+RecoFull_2017NewFPix+HARVESTFull_2017NewFPix
10624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017HCALdev_GenSimFull+DigiFull_2017HCALdev+RecoFull_2017HCALdev+HARVESTFull_2017HCALdev
10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017AllNew_GenSimFull+DigiFull_2017AllNew+RecoFull_2017AllNew+HARVESTFull_2017AllNew

slava77 · 2016-09-26T22:44:42Z

chi2 values are now filled
(color code is inverted wrt the legend: red is baseline and black is with this PR)

Looking at data
2016 single photon wf 136.731

about 15% have a default value (999).
MC with pileup has a similar distribution.

2017 HCALdev wf has a larger fraction at default value

slava77 · 2016-09-26T22:45:57Z

+1

for #15916 be23969

code change is in line with the PR description and the follow up review
jenkins tests pass and comparisons show no differences (the new variable is not monitored)
local tests with added monitoring of chi2 shows that the variable is now filled

cmsbuild · 2016-09-26T22:46:11Z

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

davidlange6 · 2016-09-27T07:45:00Z

+1

cmsbuild added this to the Next CMSSW_8_1_X milestone Sep 20, 2016

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Sep 20, 2016

cmsbuild added tests-started and removed tests-pending labels Sep 20, 2016

cmsbuild added tests-approved and removed tests-started labels Sep 20, 2016

cmsbuild added comparison-available and removed comparison-pending labels Sep 20, 2016

slava77 reviewed Sep 23, 2016

View reviewed changes

mariadalfonso added 4 commits September 24, 2016 15:50

add the chi2 into the HBHEDataFormats

0199fc7

add the chi2 in the run2 and phase1 M2

2b65f67

store computed chi2 value for run2

e41c803

renamed getChiSquared into chi2

be23969

mariadalfonso force-pushed the HBHEDataFormatChi2 branch from a2c6139 to be23969 Compare September 24, 2016 16:23

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Sep 24, 2016

cmsbuild added tests-started and removed tests-pending labels Sep 24, 2016

cmsbuild added tests-approved and removed tests-started labels Sep 24, 2016

cmsbuild added comparison-available and removed comparison-pending labels Sep 24, 2016

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Sep 26, 2016

cmsbuild added orp-approved and removed orp-pending labels Sep 27, 2016

cmsbuild merged commit f948787 into cms-sw:CMSSW_8_1_X Sep 27, 2016

mariadalfonso mentioned this pull request Jan 12, 2017

HBHE M2: noise definition cleanup #17129

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HBHE: add M2-chi2 into the HBHErecHit data formats #15916

HBHE: add M2-chi2 into the HBHErecHit data formats #15916

mariadalfonso commented Sep 20, 2016

cmsbuild commented Sep 20, 2016

slava77 commented Sep 20, 2016

slava77 commented Sep 20, 2016

cmsbuild commented Sep 20, 2016 •

edited

cmsbuild commented Sep 20, 2016

mariadalfonso commented Sep 20, 2016

cmsbuild commented Sep 20, 2016

slava77 commented Sep 22, 2016

slava77 Sep 23, 2016

slava77 Sep 24, 2016

mariadalfonso Sep 24, 2016

cmsbuild commented Sep 24, 2016

mariadalfonso commented Sep 24, 2016

slava77 commented Sep 24, 2016

cmsbuild commented Sep 24, 2016 •

edited

cmsbuild commented Sep 24, 2016

cmsbuild commented Sep 24, 2016

slava77 commented Sep 26, 2016

slava77 commented Sep 26, 2016

cmsbuild commented Sep 26, 2016

davidlange6 commented Sep 27, 2016

HBHE: add M2-chi2 into the HBHErecHit data formats #15916

HBHE: add M2-chi2 into the HBHErecHit data formats #15916

Conversation

mariadalfonso commented Sep 20, 2016

cmsbuild commented Sep 20, 2016

slava77 commented Sep 20, 2016

slava77 commented Sep 20, 2016

cmsbuild commented Sep 20, 2016 • edited

cmsbuild commented Sep 20, 2016

mariadalfonso commented Sep 20, 2016

cmsbuild commented Sep 20, 2016

slava77 commented Sep 22, 2016

slava77 Sep 23, 2016

Choose a reason for hiding this comment

slava77 Sep 24, 2016

Choose a reason for hiding this comment

mariadalfonso Sep 24, 2016

Choose a reason for hiding this comment

cmsbuild commented Sep 24, 2016

mariadalfonso commented Sep 24, 2016

slava77 commented Sep 24, 2016

cmsbuild commented Sep 24, 2016 • edited

cmsbuild commented Sep 24, 2016

cmsbuild commented Sep 24, 2016

slava77 commented Sep 26, 2016

slava77 commented Sep 26, 2016

cmsbuild commented Sep 26, 2016

davidlange6 commented Sep 27, 2016

cmsbuild commented Sep 20, 2016 •

edited

cmsbuild commented Sep 24, 2016 •

edited