JER integration into conditions database #10448

blinkseb · 2015-07-29T09:39:36Z

(This a a resubmission of #10350 targeting CMSSW 7.6.x and with corrections)

This PR adds the necessary machinery to store Jet energy Resolutions and associated Scale Factors to the conditions database.

Values are parsed from text files using the same format as Jet Energy Corrections text files. Two new records are added, JetResolutionRcd and JetResolutionScaleFactorRcd, storing respectively resolutions and scale factors. Both resolutions and scale factors are converted to a common object, JetResolutionObject, which is serialized into the corresponding records.

Facility are provided to create a database from text files [1] and to create text files from a database or a GT [2].

Users can access values using the classes JetResolution and JetResolutionScaleFactors. A static function is provided to hide the boilerplate to retrieve the object from the condition database. A demo analyzer is included as an how-to tutorial [3] and is ran as an unit-test for the package.

This depends also on two data files used for unit-testing. ~~Corresponding PR is pending for inclusion on the cms-data org [4]~~. PR is merged.

This interface has been presented and validated at the JERC meeting on 07/23.

This will need to be backported into CMSSW 7.5.x and probably also 7.4.x

[1] https://github.com/blinkseb/cmssw/blob/665bd5d702f5ed976a898b7f8c426a5955eaf103/JetMETCorrections/Modules/test/JetResolutionDBWriter_cfg.py
[2] https://github.com/blinkseb/cmssw/blob/665bd5d702f5ed976a898b7f8c426a5955eaf103/JetMETCorrections/Modules/test/JetResolutionLocalDBReader_cfg.py
[3] https://github.com/blinkseb/cmssw/blob/665bd5d702f5ed976a898b7f8c426a5955eaf103/JetMETCorrections/Modules/plugins/JetResolutionDemo.cc
[4] #10350

cmsbuild · 2015-07-29T09:41:22Z

A new Pull Request was created by @blinkseb (Sébastien Brochet) for CMSSW_7_6_X.

JER integration into conditions database

It involves the following packages:

CondCore/JetMETPlugins
CondCore/Utilities
CondFormats/DataRecord
CondFormats/JetMETObjects
JetMETCorrections/Modules

@diguida, @cerminar, @monttj, @cmsbuild, @ggovi, @vadler, @mmusich can you please review it and eventually sign? Thanks.
@ghellwig, @apfeiffer1, @ahinzmann, @rappoccio, @mmarionncern, @TaiSakuma, @jdolen, @nhanvtran, @schoef, @mariadalfonso 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.
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.

blinkseb · 2015-08-03T11:26:48Z

Is there any news on this? This is quite high priority for JME. Thanks!

The PR for data has been merged on cms-data so everything is available in latest IB.

davidlange6 · 2015-08-07T11:09:24Z

@cmsbuild, please test

diguida · 2015-08-07T11:10:29Z

@blinkseb sorry for late reply, looking into it.

diguida · 2015-08-07T11:16:23Z

CondFormats/JetMETObjects/interface/JetResolutionObject.h

+            std::shared_ptr<JetResolutionObject> m_object;
+    };
+
+    class JetResolutionScaleFactor {


@blinkseb I am a bit confused by this class. It is basically an observer for JetResolutionObject.
I would definitively not put it here, where you declare entity classes (observers fall in the control class domain).
See more comments on the implementation on each method.

Sorry for the delay, I was in holidays.

Do you want me to move these classes into another files?

blinkseb · 2015-09-08T17:21:51Z

As suggested by @diguida, I moved the classes JetResolution and JetResolutionScaleFactors into JetMETCorrections/Modules.

schoef · 2015-09-14T16:12:09Z

@diguida Could we please proceed with this? Anything wlese required from our side?

cmsbuild · 2015-09-14T16:44:35Z

Pull request #10448 was updated. @diguida, @cerminar, @monttj, @cmsbuild, @ggovi, @vadler, @mmusich can you please check and sign again.

mmusich · 2015-09-14T16:54:15Z

please test

cmsbuild · 2015-09-14T16:56:54Z

The tests are being triggered in jenkins.

cmsbuild · 2015-09-14T18:07:23Z

-1
Tested at: facd68e
I found an error when building:

>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetCorrectorProducers.cc 
>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetResolutionDBReader.cc 
>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetResolutionDBWriter.cc 
>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetResolutionDemo.cc 
>> Compiling edm plugin /tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/METCorrectorDBWriter.cc 
/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetResolutionDemo.cc:12:63: fatal error: JetMETCorrections/Modules/interface/JetResolution.h: No such file or directory
 #include "JetMETCorrections/Modules/interface/JetResolution.h"
                                                               ^
compilation terminated.
/tmp/cmsbuild/workspace/ib-any-integration/CMSSW_7_6_X_2015-09-13-1100/src/JetMETCorrections/Modules/plugins/JetResolutionDemo.cc:12:63: fatal error: JetMETCorrections/Modules/interface/JetResolution.h: No such file or directory
 #include "JetMETCorrections/Modules/interface/JetResolution.h"

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

blinkseb · 2015-09-14T18:40:00Z

I've updated the PR to fix build issues.

cmsbuild · 2015-09-15T10:56:53Z

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

blinkseb · 2015-09-20T16:00:01Z

Is there something else I need to do for this PR to be merged? Just let me know.

mmusich · 2015-09-21T07:45:06Z

@blinkseb we are reviewing it. Thanks

diguida · 2015-09-25T15:30:58Z

+1

New condition object JetResolutionObject
Import and fetch macros able to manage the object
Two records associated to it
Two control classes defined for later use in client code.

Things to be fixed or possibly reviewed in later PR:

remove spurious include in https://github.com/cms-sw/cmssw/pull/10448/files#diff-69cc9f239fd56430a5acddec1b1fa1f3R5 as the control class JetResolutionObject which is handling the record via the EventSetup is now in a different package;
check how you dispatch the JetResolutionObject payloads as in https://github.com/cms-sw/cmssw/pull/10448/files#diff-4069f7250ad296887d167d001ac989f9R23 and https://github.com/cms-sw/cmssw/pull/10448/files#diff-4069f7250ad296887d167d001ac989f9R48 (where you return by value the payload from the EventSetup delivered via Records).

blinkseb · 2015-09-26T09:47:37Z

Thanks for the review @diguida! I'll update the PR with the changes requested. Just one question about your latest point (dispatching of JetResolutionObject).

I tried to return a const ref but it's obviously not possible since the Handle is local to the function. What would you advise to return? Directly the Handle maybe?

diguida · 2015-09-29T12:19:42Z

@ggovi ping :-)

diguida · 2015-09-29T12:24:53Z

@blinkseb do not update this PR now, otherwise it has to be re-tested!
Make a new PR once this is included.
Concerning your question, the issue is how to manage objects delivered via EventSetup. The quick reply is: do not use them outside the context where the EventSetup makes them available to the clients.
I discussed this with @ggovi , and we agreed on this "quick summary".
I hope @Dr15Jones can confirm this is correct.

ggovi · 2015-09-29T15:07:22Z

+1

Dr15Jones · 2015-09-29T16:03:00Z

Seems reasonable

diguida · 2015-09-29T16:44:12Z

@monttj @vadler ping :-)

diguida · 2015-09-29T16:44:41Z

@Dr15Jones thanks for looking into that.

monttj · 2015-09-30T07:09:31Z

+1

cmsbuild · 2015-09-30T07:11:23Z

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-09-30T11:59:11Z

+1

JER integration into conditions database

cmsbuild added this to the Next CMSSW_7_6_X milestone Jul 29, 2015

cmsbuild added alca-pending analysis-pending comparison-pending db-pending orp-pending pending-signatures tests-pending labels Jul 29, 2015

diguida reviewed Aug 7, 2015
View reviewed changes

blinkseb force-pushed the jer_gt_rebased_76x branch 2 times, most recently from 3d793af to facd68e Compare September 8, 2015 17:21

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

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

Sébastien Brochet added 2 commits September 14, 2015 20:35

Interface to add JER and JER SF to conditions database

a4c8a25

Add demo code to show usage of new JER code

e5409fd

blinkseb force-pushed the jer_gt_rebased_76x branch from facd68e to e5409fd Compare September 14, 2015 18:41

cmsbuild removed the tests-rejected label Sep 14, 2015

cmsbuild added comparison-available and removed comparison-pending labels Sep 15, 2015

cmsbuild added alca-approved and removed alca-pending labels Sep 25, 2015

cmsbuild added db-approved and removed db-pending labels Sep 29, 2015

cmsbuild added analysis-approved fully-signed and removed analysis-pending pending-signatures labels Sep 30, 2015

cmsbuild added orp-approved and removed orp-pending labels Sep 30, 2015

cmsbuild added a commit that referenced this pull request Sep 30, 2015

Merge pull request #10448 from blinkseb/jer_gt_rebased_76x

b8ac33e

JER integration into conditions database

cmsbuild merged commit b8ac33e into cms-sw:CMSSW_7_6_X Sep 30, 2015

This was referenced Oct 7, 2015

[74X] JER integration into conditions database #11664

Closed

[75X] JER integration into conditions database #11665

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JER integration into conditions database #10448

JER integration into conditions database #10448

blinkseb commented Jul 29, 2015

cmsbuild commented Jul 29, 2015

blinkseb commented Aug 3, 2015

davidlange6 commented Aug 7, 2015

diguida commented Aug 7, 2015

diguida Aug 7, 2015

blinkseb Aug 31, 2015

blinkseb commented Sep 8, 2015

schoef commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

mmusich commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

blinkseb commented Sep 14, 2015

cmsbuild commented Sep 15, 2015

blinkseb commented Sep 20, 2015

mmusich commented Sep 21, 2015

diguida commented Sep 25, 2015

blinkseb commented Sep 26, 2015

diguida commented Sep 29, 2015

diguida commented Sep 29, 2015

ggovi commented Sep 29, 2015

Dr15Jones commented Sep 29, 2015

diguida commented Sep 29, 2015

diguida commented Sep 29, 2015

monttj commented Sep 30, 2015

cmsbuild commented Sep 30, 2015

davidlange6 commented Sep 30, 2015

JER integration into conditions database #10448

JER integration into conditions database #10448

Conversation

blinkseb commented Jul 29, 2015

cmsbuild commented Jul 29, 2015

blinkseb commented Aug 3, 2015

davidlange6 commented Aug 7, 2015

diguida commented Aug 7, 2015

diguida Aug 7, 2015

Choose a reason for hiding this comment

blinkseb Aug 31, 2015

Choose a reason for hiding this comment

blinkseb commented Sep 8, 2015

schoef commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

mmusich commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

cmsbuild commented Sep 14, 2015

blinkseb commented Sep 14, 2015

cmsbuild commented Sep 15, 2015

blinkseb commented Sep 20, 2015

mmusich commented Sep 21, 2015

diguida commented Sep 25, 2015

blinkseb commented Sep 26, 2015

diguida commented Sep 29, 2015

diguida commented Sep 29, 2015

ggovi commented Sep 29, 2015

Dr15Jones commented Sep 29, 2015

diguida commented Sep 29, 2015

diguida commented Sep 29, 2015

monttj commented Sep 30, 2015

cmsbuild commented Sep 30, 2015

davidlange6 commented Sep 30, 2015