renew matched lepton ids for every jet #41325

menglu21 · 2023-04-12T14:05:02Z

PR description:

< initially the matched lepton ids are declared only once https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/plugins/LeptonInJetProducer.cc#L79-L80, in the case of that when there is no matched lepton, the previous matched lepton id will be used for the next jet, i.e., two fat jets can match to one lepton which is not correct. >

PR validation:

<before the modification:

Events->Scan("FatJet_electronIdx3SJ:FatJet_pt")
***********************************************
*    Row   * Instance * FatJet_el * FatJet_pt *
***********************************************
*        0 *        0 *         0 *       751 *
*        0 *        1 *         0 *       709 *
*        1 *        0 *        -1 *       958 *
*        1 *        1 *         0 *     598.5 *
*        2 *        0 *        -1 *     849.5 *
*        2 *        1 *        -1 *       835 *
*        2 *        2 *         0 *     240.5 *
*        3 *        0 *         0 *       995 *
*        3 *        1 *        -1 *     932.5 *
*        3 *        2 *        -1 *       335 *
*        4 *        0 *         0 *       751 *
*        4 *        1 *        -1 *     603.5 *
*        4 *        2 *        -1 *       365 *
*        5 *        0 *        -1 *     624.5 *
*        5 *        1 *         0 *       616 *
*        5 *        2 *         0 *   184.125 *
......

after the modification:

Events->Scan("FatJet_electronIdx3SJ:FatJet_pt")
***********************************************
*    Row   * Instance * FatJet_el * FatJet_pt *
***********************************************
*        0 *        0 *         0 *       751 *
*        0 *        1 *        -1 *       709 *
*        1 *        0 *        -1 *       958 *
*        1 *        1 *         0 *     598.5 *
*        2 *        0 *        -1 *     849.5 *
*        2 *        1 *        -1 *       835 *
*        2 *        2 *         0 *     240.5 *
*        3 *        0 *         0 *       995 *
*        3 *        1 *        -1 *     932.5 *
*        3 *        2 *        -1 *       335 *
*        4 *        0 *         0 *       751 *
*        4 *        1 *        -1 *     603.5 *
*        4 *        2 *        -1 *       365 *
*        5 *        0 *        -1 *     624.5 *
*        5 *        1 *         0 *       616 *
*        5 *        2 *        -1 *   184.125 *

you see that before modification, for the first event, the first and second fatjet match to the first lepton. similar case in 6th event. (the pt of jet is to confirm other info is not changed )

cmsbuild · 2023-04-12T14:13:13Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41325/35148

This PR adds an extra 12KB to repository

cmsbuild · 2023-04-12T14:13:35Z

A new Pull Request was created by @menglu21 (Meng_Lu) for master.

It involves the following packages:

PhysicsTools/NanoAOD (xpog)

@cmsbuild, @simonepigazzini, @swertz, @vlimant can you please review it and eventually sign? Thanks.
@gpetruc, @AnnikaStein 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

simonepigazzini · 2023-04-12T14:26:19Z

PhysicsTools/NanoAOD/plugins/LeptonInJetProducer.cc

@@ -91,6 +91,8 @@ void LeptonInJetProducer<T>::produce(edm::StreamID streamID, edm::Event &iEvent,
      fastjet::PseudoJet p(d->px(), d->py(), d->pz(), d->energy());
      lClusterParticles.emplace_back(p);
    }
+    ele_pfmatch_index = -1;


the two variables can be created here, removing the definitions lines 79-80

I prefer the current one since define the variables every time in the loop may use more memory, though the difference could be small since they are "int".

I think optimization this is not an issue, and actually it ensures that the variables goes out of scope once the loop is compelted. Anyway I'll let the release manages commenting on this.

I think optimization this is not an issue, and actually it ensures that the variables goes out of scope once the loop is compelted. Anyway I'll let the release manages commenting on this.

Since you are asking, I think that having a variable defined in a as small as possible scope is normally an advantage. In any case, it is a minor issue here, and I'd let you decide what you prefere for it.

update to what @simonepigazzini suggested

simonepigazzini · 2023-04-12T14:27:32Z

This indeed looks like a bug. Still could you please provide more details concerning the validation (sample used, expected number of jets/ele, etc).

Thank you

menglu21 · 2023-04-12T14:43:02Z

This indeed looks like a bug. Still could you please provide more details concerning the validation (sample used, expected number of jets/ele, etc).

Thank you

The sample I used for check is /store/mc/RunIISummer20UL17MiniAODv2/NMSSM_XToYHTo2Z2BTo2L2J2B_MX-2000_MY-600_TuneCP5_13TeV-madgraph-pythia8/MINIAODSIM/106X_mc2017_realistic_v9-v2/50000/0C182B07-16FE-004A-8CAE-BB94CD140219.root, sorry but I don't understand "expected number of jet/ele", the number should not change, only the information of jet-ele matching is affected

simonepigazzini · 2023-04-12T14:48:18Z

I mean if you expected or not to have an ele/jet matching at all, and if so how many are expected.

menglu21 · 2023-04-12T15:07:03Z

I mean if you expected or not to have an ele/jet matching at all, and if so how many are expected.

the information should be useful anyway, I'm not sure how many match should be stored, currently only the closest ele is stored for every fatjet, it could be more. If all the matched information are stored then that's great, but it need some design on the output tree

cmsbuild · 2023-04-12T15:25:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41325/35153

This PR adds an extra 16KB to repository

cmsbuild · 2023-04-12T15:25:30Z

Pull request #41325 was updated. @cmsbuild, @simonepigazzini, @swertz, @vlimant can you please check and sign again.

swertz · 2023-04-17T07:51:20Z

I was going through the logic here again: https://github.com/cms-sw/cmssw/pull/41325/files#diff-667e23cfcc737cc2ceb2f59eddcdc74a19d2c21bd11fea5e02c09d3944626eb6R96

If a matched muon is found, the electron index is reset to -1 -> it looks like it's because the idea is to store only the lepton that is used in the LSF computation (leading and closest lepton), but that's not what the docstring for the electron/muon indices states (simply index of electron/muon matched to jet). I would suggest to improve the docstring to make this clear.

The matching logic is anyway a bit strange, e.g. an electron with dR(j,e)=0.3 and pT=10 GeV is given priority over a muon with dR(j,mu)=0.31 and pT=100 GeV...

simonepigazzini · 2023-04-24T12:22:04Z

enable nano

simonepigazzini · 2023-04-24T12:22:10Z

please test

cmsbuild · 2023-04-24T14:57:32Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-50a5c7/32102/summary.html
COMMIT: 2d03924
CMSSW: CMSSW_13_1_X_2023-04-24-1100/el8_amd64_gcc11
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41325/32102/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 3 lines to the logs
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3460685
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3460657
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

NANO Comparison Summary

Summary:

You potentially added 9 lines to the logs
Reco comparison results: 22 differences found in the comparisons
DQMHistoTests: Total files compared: 12
DQMHistoTests: Total histograms compared: 12072
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 12072
DQMHistoTests: Total skipped: 0
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 11 files compared)
Checked 25 log files, 11 edm output root files, 12 DQM output files

Nano size comparison Summary:

Sample	kb/ev	ref kb/ev	diff kb/ev	ev/s/thd	ref ev/s/thd	diff rate	mem/thd	ref mem/thd
2500.31	2.343	2.343	-0.000 ( -0.0% )	5.50	5.57	-1.2%	1.405	1.469
2500.311	2.440	2.440	-0.000 ( -0.0% )	5.02	5.01	+0.2%	1.786	1.773
2500.312	2.391	2.391	-0.000 ( -0.0% )	5.15	5.21	-1.2%	1.781	1.768
2500.33	1.172	1.172	-0.000 ( -0.0% )	10.55	10.58	-0.3%	1.492	1.491
2500.331	1.508	1.508	-0.000 ( -0.0% )	5.70	5.71	-0.2%	1.661	1.656
2500.332	1.409	1.409	-0.000 ( -0.0% )	8.21	8.14	+0.9%	1.603	1.577
2500.401	2.137	2.137	-0.000 ( -0.0% )	10.36	10.36	+0.0%	1.062	1.149
2500.501	1.710	1.710	-0.000 ( -0.0% )	16.65	16.79	-0.9%	1.019	1.091
2500.511	1.122	1.122	-0.000 ( -0.0% )	31.18	31.62	-1.4%	1.255	1.326
2500.5111	1.477	1.477	-0.000 ( -0.0% )	20.03	20.06	-0.2%	1.303	1.370
2500.601	2.038	2.038	-0.000 ( -0.0% )	12.60	12.62	-0.2%	1.032	1.078

simonepigazzini · 2023-04-25T08:04:20Z

+1

simonepigazzini · 2023-04-25T08:04:45Z

@menglu21 can you please open a backport for 13_0_X

cmsbuild · 2023-04-25T08:04:47Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). 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)

rappoccio · 2023-04-25T14:32:02Z

+1

menglu21 · 2023-04-25T14:50:53Z

@menglu21 can you please open a backport for 13_0_X

sure, will do it soon

renew matched lepton ids for every jet

4fa3e95

cmsbuild added this to the CMSSW_13_1_X milestone Apr 12, 2023

cmsbuild added pending-signatures tests-pending orp-pending code-checks-pending xpog-pending labels Apr 12, 2023

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

simonepigazzini reviewed Apr 12, 2023

View reviewed changes

update definition place

2d03924

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

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

cmsbuild added tests-started and removed tests-pending labels Apr 24, 2023

cmsbuild removed the tests-started label Apr 24, 2023

cmsbuild added the tests-approved label Apr 24, 2023

cmsbuild added fully-signed xpog-approved and removed pending-signatures xpog-pending labels Apr 25, 2023

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

cmsbuild merged commit b265809 into cms-sw:master Apr 25, 2023

This was referenced Apr 25, 2023

cleanup spurious change introduced in PAT photon/muon producer auto fwd-port in 2015 #41403

Merged

Drop mxnet (cms-sw/cmssw#30432) cms-sw/cmsdist#8451

Merged

Add unit test for TriggerResults format #41395

Merged

menglu21 mentioned this pull request Apr 26, 2023

renew matched lepton ids for every jet #41407

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

renew matched lepton ids for every jet #41325

renew matched lepton ids for every jet #41325

menglu21 commented Apr 12, 2023 •

edited

Loading

cmsbuild commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

simonepigazzini Apr 12, 2023

menglu21 Apr 12, 2023

simonepigazzini Apr 12, 2023

perrotta Apr 12, 2023 •

edited

Loading

menglu21 Apr 12, 2023

simonepigazzini commented Apr 12, 2023

menglu21 commented Apr 12, 2023

simonepigazzini commented Apr 12, 2023

menglu21 commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

swertz commented Apr 17, 2023 •

edited

Loading

simonepigazzini commented Apr 24, 2023

simonepigazzini commented Apr 24, 2023

cmsbuild commented Apr 24, 2023

simonepigazzini commented Apr 25, 2023

simonepigazzini commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

rappoccio commented Apr 25, 2023

menglu21 commented Apr 25, 2023

renew matched lepton ids for every jet #41325

renew matched lepton ids for every jet #41325

Conversation

menglu21 commented Apr 12, 2023 • edited Loading

PR description:

PR validation:

cmsbuild commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

simonepigazzini Apr 12, 2023

Choose a reason for hiding this comment

menglu21 Apr 12, 2023

Choose a reason for hiding this comment

simonepigazzini Apr 12, 2023

Choose a reason for hiding this comment

perrotta Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

menglu21 Apr 12, 2023

Choose a reason for hiding this comment

simonepigazzini commented Apr 12, 2023

menglu21 commented Apr 12, 2023

simonepigazzini commented Apr 12, 2023

menglu21 commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

cmsbuild commented Apr 12, 2023

swertz commented Apr 17, 2023 • edited Loading

simonepigazzini commented Apr 24, 2023

simonepigazzini commented Apr 24, 2023

cmsbuild commented Apr 24, 2023

Comparison Summary

NANO Comparison Summary

simonepigazzini commented Apr 25, 2023

simonepigazzini commented Apr 25, 2023

cmsbuild commented Apr 25, 2023

rappoccio commented Apr 25, 2023

menglu21 commented Apr 25, 2023

menglu21 commented Apr 12, 2023 •

edited

Loading

perrotta Apr 12, 2023 •

edited

Loading

swertz commented Apr 17, 2023 •

edited

Loading