Hcal simple reconstructor with method3 #11769

igv4321 · 2015-10-13T16:44:55Z

Including "method 3" initialization in the HcalSimpleReconstructor module.

Making a separate python cff for method 3 configuration and importing this configuration into HcalHitReconstructor and HcalSimpleReconstructor configs.

cmsbuild · 2015-10-13T16:46:11Z

A new Pull Request was created by @igv4321 (Igor Volobouev) for CMSSW_7_6_X.

Hcal simple reconstructor with method3

It involves the following packages:

RecoLocalCalo/HcalRecProducers

@cmsbuild, @cvuosalo, @slava77 can you please review it and eventually sign? Thanks.
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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

slava77 · 2015-10-13T19:17:24Z

RecoLocalCalo/HcalRecProducers/python/HcalHitReconstructor_hbhe_cfi.py

-    )
-
-
+    pedestalSubtractionType = cms.int32(method3.pedestalSubtractionType),


I think you can do plain assignment if you define the types in HBHEMethod3Parameters, without this "cast".

igv4321 · 2015-10-13T20:24:46Z

Yes, indeed. This is also what I tried earlier, but it did not work for me initially. Probably had an extra comma somewhere of something like that.

cmsbuild · 2015-10-13T20:26:44Z

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

slava77 · 2015-10-13T22:31:07Z

RecoLocalCalo/HcalRecProducers/python/HBHEMethod3Parameters_cfi.py

+import FWCore.ParameterSet.Config as cms
+
+# Configuration parameters for Method 3
+pedestalSubtractionType = cms.int32(1)


you can wrap this in a PSet and then include it in the module (has to be included before any named parameters of the module
https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideAboutPythonConfigFile#Parameter_Set_PSet_Objects
)
cms-sw:CMSSW_7_6_X...slava77:CMSSW_7_6_0_pre7/sign613/alt-11769

igv4321 · 2015-10-13T23:55:16Z

OK, fillDescriptions added

cmsbuild · 2015-10-13T23:56:17Z

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

slava77 · 2015-10-14T07:36:03Z

@cmsbuild please test
[unsure yet if the IB is alive]

cmsbuild · 2015-10-14T07:36:18Z

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

cmsbuild · 2015-10-14T08:11:41Z

-1
Tested at: 37620ae
you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11769/8803/summary.html

slava77 · 2015-10-14T10:04:08Z

As discussed in #11735
more strict M3 initialization strategy is desired (currently, construction and initialization are done in separate methods)
One compile time choice is proposed here
slava77@62449ed

Another alternative, less desired IMO, is to do a run-time check

smuzaffar · 2015-10-14T19:48:32Z

please test

cmsbuild · 2015-10-14T19:51:47Z

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

cmsbuild · 2015-10-14T21:21:58Z

+1
Tested at: 37620ae
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11769/8847/summary.html

cmsbuild · 2015-10-14T22:15:15Z

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

slava77 · 2015-10-19T22:12:37Z

+1

for #11769 37620ae

resolves uninitialized conditions/undefined behavior reported in [UBSAN] RecoLocalCalo/HcalRecAlgos/interface/HcalDeterministicFit.h #11735 . A more robust solution forcing initialization at construction (without delays to special init function) was discussed as something to be done reasonably soon.
jenkins tests pass and comparisons with the baseline show no difference

cmsbuild · 2015-10-19T22:16:29Z

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

davidlange6 · 2015-10-23T08:21:08Z

+1

…ethod3 Hcal simple reconstructor with method3

Igor added 3 commits October 13, 2015 17:18

initializing HBHE Method 3 from HcalSimpleReconstructor

92c39da

fixing config file to initialize Method 3

b6ff8f4

fixing config file to initialize Method 3

959ae69

cmsbuild added this to the Next CMSSW_7_6_X milestone Oct 13, 2015

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending comparison-pending labels Oct 13, 2015

slava77 reviewed Oct 13, 2015
View reviewed changes

using cms types inside HBHEMethod3Parameters_cfi

5fa3654

igv4321 mentioned this pull request Oct 13, 2015

[UBSAN] RecoLocalCalo/HcalRecAlgos/interface/HcalDeterministicFit.h #11735

Closed

slava77 reviewed Oct 13, 2015
View reviewed changes

Igor added 2 commits October 14, 2015 01:08

made a PSet for method 3 parameters

9d6e24c

added fillDescriptions function to HcalSimpleReconstructor

37620ae

cmsbuild added tests-started and removed tests-pending labels Oct 14, 2015

cmsbuild added tests-rejected and removed tests-started labels Oct 14, 2015

cmsbuild added tests-pending tests-started and removed tests-rejected tests-pending labels Oct 14, 2015

cmsbuild added tests-approved and removed tests-started labels Oct 14, 2015

cmsbuild added comparison-available and removed comparison-pending labels Oct 14, 2015

cmsbuild mentioned this pull request Oct 19, 2015

Hcal simple reconstructor with method3 #11918

Merged

cmsbuild added reconstruction-approved fully-signed and removed pending-signatures reconstruction-pending labels Oct 19, 2015

cmsbuild added orp-approved and removed orp-pending labels Oct 23, 2015

cmsbuild added a commit that referenced this pull request Oct 23, 2015

Merge pull request #11769 from igv4321/HcalSimpleReconstructor-with-m…

2dd6291

…ethod3 Hcal simple reconstructor with method3

cmsbuild merged commit 2dd6291 into cms-sw:CMSSW_7_6_X Oct 23, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hcal simple reconstructor with method3 #11769

Hcal simple reconstructor with method3 #11769

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 Oct 13, 2015

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 Oct 13, 2015

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

slava77 commented Oct 14, 2015

smuzaffar commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

slava77 commented Oct 19, 2015

cmsbuild commented Oct 19, 2015

davidlange6 commented Oct 23, 2015

		)


		pedestalSubtractionType = cms.int32(method3.pedestalSubtractionType),

Hcal simple reconstructor with method3 #11769

Hcal simple reconstructor with method3 #11769

Conversation

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 Oct 13, 2015

Choose a reason for hiding this comment

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 Oct 13, 2015

Choose a reason for hiding this comment

igv4321 commented Oct 13, 2015

cmsbuild commented Oct 13, 2015

slava77 commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

slava77 commented Oct 14, 2015

smuzaffar commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

cmsbuild commented Oct 14, 2015

slava77 commented Oct 19, 2015

cmsbuild commented Oct 19, 2015

davidlange6 commented Oct 23, 2015