add pt dependent SF and add legends on summary plots #41240

yuanchao · 2023-03-31T03:29:57Z

PR description:

As requested in AlCa meeting, adding support for JetResolutionObject SF records with Eta and Pt (2-bin). Corresponding histograms are added. Adding legends for PlotImage outputs.

PR validation:

Tested locally with getPayloadData.py script. Running scram code-checks and code-format.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

This is not a backport.

cmsbuild · 2023-03-31T03:36:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41240/34966

This PR adds an extra 12KB to repository

cmsbuild · 2023-03-31T03:36:48Z

A new Pull Request was created by @yuanchao (Yuan CHAO) for master.

It involves the following packages:

CondCore/JetMETPlugins (db)

@cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please review it and eventually sign? Thanks.
@mmusich this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2023-03-31T09:56:25Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41240/34975

This PR adds an extra 16KB to repository

cmsbuild · 2023-03-31T09:56:46Z

Pull request #41240 was updated. @cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

tvami · 2023-03-31T12:17:51Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc

+      //  leg->SetFillColor(0);
+      //  leg->SetLineColor(1);


please remove commented out code

tvami · 2023-03-31T12:18:04Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc

+      //  leg->SetFillColor(0);
+      //  leg->SetLineColor(1);


please remove commented out code

tvami · 2023-03-31T12:18:12Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc


+            //               payload->getDefinition().getBinName(1) == "JetPt" &&
+            //               !record.getBinsRange()[1].is_inside(500.) )
+            //              continue; // for 2-bin payload, take jetpt=500
+


please remove commented out code

tvami

The legend entry question is something I only pointed out once, but it's there several times

tvami · 2023-03-31T12:18:31Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc

-              if (record.getBinsRange()[0].is_inside(x_axis)) {
+              if (((is_2bin == false) || (is_2bin == true && record.getBinsRange()[1].is_inside(500.))) &&
+                  record.getBinsRange()[0].is_inside(x_axis)) {
+                //edm::LogWarning("JRObj_PI") << "JetEta " << x_axis << ", SF "


please remove commented out code

tvami · 2023-03-31T12:18:54Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc

+            for (size_t it = 0; it <= NBIN_PT; it++) {
+              double x_axis = (it + 0.5) * (MAX_PT - MIN_PT) / NBIN_PT + MIN_PT;
+              if (record.getBinsRange()[1].is_inside(x_axis)) {
+                //edm::LogWarning("JRObj_PI") << "JetPt " << x_axis << ", SF " << record.getParametersValues()[0];


please remove commented out code, here actually you could make it LogDebug right?

tvami · 2023-03-31T12:19:29Z

CondCore/JetMETPlugins/test/testJetResolutionPayloadInspector.cpp

@@ -52,7 +52,7 @@ int main(int argc, char** argv) {
  histo4.process(connectionString, PI::mk_input(tag_eta, start, end));
  std::cout << histo4.data() << std::endl;

-  std::cout << "## Jet SF Histograms" << std::endl;
+  std::cout << "## Jet SF vs. Eta Histograms" << std::endl;


Can we void using the couts?

This is simply a copy from the example unit test code provided. Surely I can replace them with LogINFO.

tvami · 2023-03-31T12:19:54Z

CondCore/JetMETPlugins/test/testJetResolutionPayloadInspector.cpp

+
+  JME::JetScaleFactorVsPtUP histo10;
+  histo10.process(connectionString, PI::mk_input(tag_sf, start, end));
+  std::cout << histo10.data() << std::endl;


Can we void using the couts? In all the other occurrences too

tvami · 2023-03-31T12:21:00Z

CondCore/JetMETPlugins/plugins/JetResolution_PayloadInspector.cc

+          sf_pt_norm->Draw("axis same");
+
+          leg_pt->AddEntry(sf_pt_norm, (tag_ver + '_' + tag_jet).c_str(), "l");
+          leg_pt->AddEntry((TObject*)nullptr, "JetEta=2.55", "");


why is this hardcoded to 2.55? Also isnt this less than 2.55 instead of the equal sign?

For the Jet resolution part, I follow the plots in the presentation of Andrea that rho and eta or pt are fixed. As to SF, for this two-bin case, I pick up the eta region that has most records. (here 2.5-2.6, 2.55 is the central value)

Hmm but then the PI has the option to pass arguments, wouldnt it be better to be able to specify the eta instead of having these hardcoded numbers?

Yes, the old JEC/JER web viewer shown in Andreas' slides uses fixed values and in the latest version users can choose from a drop-down menu, PI can only take float inputs. I need to check if some default values can be set, otherwise it's rather not-ideal. I'll deal with this after finish the JEC PI.

cmsbuild · 2023-04-06T09:21:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41240/35091

This PR adds an extra 16KB to repository

cmsbuild · 2023-04-06T09:21:36Z

Pull request #41240 was updated. @cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

add tests for SF vs. Pt removing commented code and use edm::MessageLogger

cmsbuild · 2023-04-06T14:57:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41240/35100

This PR adds an extra 16KB to repository

cmsbuild · 2023-04-06T14:58:16Z

Pull request #41240 was updated. @cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

tvami · 2023-04-06T19:11:18Z

+db

cmsbuild · 2023-04-06T19:11:45Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

cmsbuild · 2023-04-06T21:39:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b2a4c6/31878/summary.html
COMMIT: 2fc509a
CMSSW: CMSSW_13_1_X_2023-04-06-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41240/31878/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 3 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3459609
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3459584
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

perrotta · 2023-04-07T06:56:22Z

CondCore/JetMETPlugins/test/testJetResolutionPayloadInspector.cpp

@@ -32,39 +32,55 @@ int main(int argc, char** argv) {
  cond::Time_t start = static_cast<unsigned long long>(1);
  cond::Time_t end = static_cast<unsigned long long>(1);

-  std::cout << "## Jet Pt Resolution Histograms" << std::endl;
+  edm::LogWarning("JER_PI") << "## Jet Pt Resolution Histograms" << std::endl;


For test code, not used in productions, there is normally no strict requirement to move out of std::cout.
In any case, since you are migrating to LogWarning, please add the corresponding header include

#include "FWCore/MessageLogger/interface/MessageLogger.h"

which is now missing

perrotta · 2023-04-07T06:57:51Z

@tvami your comment https://github.com/cms-sw/cmssw/pull/41240/files#r1154408873 was not implemented yet. Could you please confirm that this is still fine with you?

tvami · 2023-04-07T11:56:51Z

Could you please confirm that this is still fine with you?

Yes, Yuan agreed to follow-up on that in a next PR. Thanks for double-checking!

rappoccio · 2023-04-07T15:15:46Z

+1

cmsbuild added this to the CMSSW_13_1_X milestone Mar 31, 2023

cmsbuild added db-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 31, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 31, 2023

yuanchao force-pushed the pi_jetres2 branch from bcdc946 to 40aabaf Compare March 31, 2023 09:50

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 31, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 31, 2023

tvami reviewed Mar 31, 2023

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 6, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 6, 2023

add pt dependent SF and add legends on summary plots

2fc509a

add tests for SF vs. Pt removing commented code and use edm::MessageLogger

yuanchao force-pushed the pi_jetres2 branch from 46e6284 to 2fc509a Compare April 6, 2023 14:49

cmsbuild removed the code-checks-approved label Apr 6, 2023

cmsbuild added the code-checks-pending label Apr 6, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 6, 2023

cmsbuild added fully-signed tests-started db-approved and removed db-pending pending-signatures tests-pending labels Apr 6, 2023

cmsbuild added tests-approved and removed tests-started labels Apr 6, 2023

perrotta reviewed Apr 7, 2023

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Apr 7, 2023

cmsbuild merged commit a07f243 into cms-sw:master Apr 7, 2023

This was referenced Apr 8, 2023

Update onnxruntime to 1.14.1 cms-sw/cmsdist#8397

Merged

Make thread safe statistics accounting in TStorageFactoryFile #41308

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add pt dependent SF and add legends on summary plots #41240

add pt dependent SF and add legends on summary plots #41240

yuanchao commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

tvami Mar 31, 2023

yuanchao Apr 2, 2023

tvami Mar 31, 2023

tvami Mar 31, 2023

tvami left a comment

tvami Mar 31, 2023

tvami Mar 31, 2023

yuanchao Apr 2, 2023

tvami Mar 31, 2023

yuanchao Apr 2, 2023

tvami Mar 31, 2023

tvami Mar 31, 2023

yuanchao Apr 2, 2023

tvami Apr 3, 2023

yuanchao Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

tvami commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

perrotta Apr 7, 2023

perrotta commented Apr 7, 2023

tvami commented Apr 7, 2023

rappoccio commented Apr 7, 2023

add pt dependent SF and add legends on summary plots #41240

add pt dependent SF and add legends on summary plots #41240

Conversation

yuanchao commented Mar 31, 2023

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

cmsbuild commented Mar 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvami left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

tvami commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

cmsbuild commented Apr 6, 2023

Comparison Summary

Choose a reason for hiding this comment

perrotta commented Apr 7, 2023

tvami commented Apr 7, 2023

rappoccio commented Apr 7, 2023