Use GEM Rechits (not segments) for GEMMuon

[watson-ij]

PR description:

Changes definition of GEMMuon to match to GEM RecHits, rather than GEM Segments. This change will enable us to use GEMMuon for GEM efficiency measurements with the common muon POG tag and probe code in the DQM. We also require that a GEMMuon be a good TrackerMuon, as there is no use case for a separate GEMMuon which is not also a TrackerMuon, and we want to keep the fake rate low. The code to change to rechits was copied from the RPC code and adjusted to GEM.

These changes were originally proposed to the muon POG in April [1], and it was agreed the changes would be okay, assuming the fake rate wouldn't increase. We found that segments to rechits would increase the fake rate [2], so we proposed by mail to require that GEMMuon be a subset of TrackerMuon, which still allows our TnP measurement, which is the use case of GEMMuon from the GEM side.

@jshlee

[1] https://indico.cern.ch/event/1153168/ [2] https://indico.cern.ch/event/1171113/contributions/4918837/attachments/2462011/4221295/Fake-rate-study_counting.pdf

PR validation:

Ran a ttbar workflow with the changes. Checked that we keep all GEMMuon that are also TrackerMuon. Checked efficiency measurements look okay.

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:

Before submitting your pull requests, make sure you followed this checklist:

• verify that the PR is really intended for the chosen branch
• verify that changes follow CMS Naming, Coding, And Style Rules
• verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31448

• This PR adds an extra 48KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

• code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31448/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31448/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31449

• This PR adds an extra 48KB to repository

[cmsbuild]

A new Pull Request was created by @watson-ij (Ian J. Watson) for master.

It involves the following packages:

• DataFormats/MuonReco (reconstruction)
• RecoMuon/MuonIdentification (reconstruction)
• TrackingTools/TrackAssociator (reconstruction)

@jpata, @cmsbuild, @clacaputo can you please review it and eventually sign? Thanks. @VourMa, @felicepantaleo, @abbiendi, @CeliaFernandez, @mmusich, @cericeci, @battibass, @JanFSchulte, @jhgoh, @dgulhan, @sscruz, @trocino, @rociovilar, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @ebrondol, @mtosi, @amagitte, @HuguesBrun, @Fedespring, @lecriste, @gpetruc this is something you requested to watch as well. @perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[watson-ij]

please test

[clacaputo]

Hi @watson-ij , do you plan to prepare a backport of this for 12_4_X? Also, did you know the impact on the event size?

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-938bfc/26693/summary.html COMMIT: 1a17b01f4c358a93f72868ceaf3ca1bf497bac03 CMSSW: CMSSW_12_5_X_2022-08-07-2300/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38990/26693/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 3366 differences found in the comparisons
• DQMHistoTests: Total files compared: 51
• DQMHistoTests: Total histograms compared: 3691612
• DQMHistoTests: Total failures: 4982
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3686608
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
• Checked 212 log files, 49 edm output root files, 51 DQM output files
• TriggerResults: found differences in 1 / 50 workflows

[watson-ij]

Hi @clacaputo. I believe that we are okay for it to just be in 12_5. Given that it will change the muon object my assumption was that we wouldn't be able to backport, but I will check with GEM DPG tonight if we want to push for a backport.

For the event size, it should reduce the number of GEMMuon, since we now require that GEMMuons also pass TrackerMuon criteria (though we are adding in a new vector of GEM matches to the muon object).

[watson-ij]

I had a look through the changes, and can see the expected drop in no. of reco muons from dropping GEMMuons that aren't TrackerMuons. However, there are also changes like in the no. of ecal hits [1], which I don't think this PR could affect, but maybe I'm missing some interaction? It seems to be causing a lot of knock on effects particularly in 11834.0. Do I need to be looking into these changes also?

[1] https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_5_X_2022-08-07-2300+938bfc/52021/validateJR/all_OldVSNew_TTbar14TeVPU2021wf11834p0/all_OldVSNew_TTbar14TeVPU2021wf11834p0c_EcalRecHitsSorted_reducedEcalRecHitsEB__RECO_obj_objAT_size.png

[clacaputo]

I had a look through the changes, and can see the expected drop in no. of reco muons from dropping GEMMuons that aren't TrackerMuons. However, there are also changes like in the no. of ecal hits [1], which I don't think this PR could affect, but maybe I'm missing some interaction? It seems to be causing a lot of knock on effects particularly in 11834.0. Do I need to be looking into these changes also?

[1] https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_5_X_2022-08-07-2300+938bfc/52021/validateJR/all_OldVSNew_TTbar14TeVPU2021wf11834p0/all_OldVSNew_TTbar14TeVPU2021wf11834p0c_EcalRecHitsSorted_reducedEcalRecHitsEB__RECO_obj_objAT_size.png

Digging a bit in the production flow of reducedEcalRecHitsEB, there is a dependence on muons:

• here you can find the configuration for the producer https://github.com/cms-sw/cmssw/blob/master/RecoEcal/EgammaClusterProducers/python/reducedRecHitsSequence_cff.py#L84-L108 , where one of the input is :

# muons
cms.InputTag("muonEcalDetIds"),

• This muonEcalDetIds collection is produced here https://github.com/cms-sw/cmssw/blob/master/RecoMuon/MuonIdentification/python/muons1stStep_cfi.py#L100-L102 , using this producer https://github.com/cms-sw/cmssw/blob/master/RecoMuon/MuonIdentification/plugins/InterestingEcalDetIdProducer.cc#L34-L54

So, I suspect that a change in the muons1stStep can affect https://github.com/cms-sw/cmssw/blob/master/RecoEcal/EgammaClusterProducers/src/ReducedRecHitCollectionProducer.cc

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31528

• This PR adds an extra 48KB to repository

[cmsbuild]

Pull request #38990 was updated. @jpata, @cmsbuild, @clacaputo can you please check and sign again.

[watson-ij]

I cleaned up the commented out code. I also checked through the muon1stStep code, and as @clacaputo suggested, the GEMMuon can affect that particular reducedEcalRecHit collection, as fillEnergy is on [1] so any type of muon store calo hits used in the producer @clacaputo mentioned [3], which can affect the output if we remove GEMMuon that don't pass TrackerMuon. I'm not sure if we should anything about this?

[1] https://github.com/cms-sw/cmssw/blob/1341cc6e60df6710d820eedad5d07268ac452498/RecoMuon/MuonIdentification/python/muons1stStep_cfi.py#L23 [2] https://github.com/cms-sw/cmssw/blob/1341cc6e60df6710d820eedad5d07268ac452498/RecoMuon/MuonIdentification/plugins/MuonIdProducer.cc#L860 [3] https://github.com/cms-sw/cmssw/blob/1341cc6e60df6710d820eedad5d07268ac452498/RecoMuon/MuonIdentification/plugins/InterestingEcalDetIdProducer.cc#L47

[jpata]

the GEMMuon can affect that particular reducedEcalRecHit collection ... I'm not sure if we should anything about this?

as far as I understand, if this is physically meaningful and not a bug, then I don't see a problem for 12_5.

[jpata]

@cmsbuild please test

[cmsbuild]

-1

Failed Tests: UnitTests Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-938bfc/26784/summary.html COMMIT: b4bec60695984deefc8e01465639e22722bd3509 CMSSW: CMSSW_12_5_X_2022-08-12-1100/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38990/26784/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test testEoP had ERRORS

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 3369 differences found in the comparisons
• DQMHistoTests: Total files compared: 51
• DQMHistoTests: Total histograms compared: 3692476
• DQMHistoTests: Total failures: 4965
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 3687488
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.004 KiB( 50 files compared)
• DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
• Checked 212 log files, 49 edm output root files, 51 DQM output files
• TriggerResults: found differences in 1 / 50 workflows

[jpata]

@cmsbuild please test

the failure looks transient and unrelated

[cmsbuild]

-1

Failed Tests: UnitTests Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-938bfc/26813/summary.html COMMIT: b4bec60695984deefc8e01465639e22722bd3509 CMSSW: CMSSW_12_5_X_2022-08-14-2300/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38990/26813/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test ExpressionEvaluatorUnitTest had ERRORS

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 3367 differences found in the comparisons
• DQMHistoTests: Total files compared: 51
• DQMHistoTests: Total histograms compared: 3692476
• DQMHistoTests: Total failures: 4963
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3687491
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
• Checked 212 log files, 49 edm output root files, 51 DQM output files
• TriggerResults: found differences in 1 / 50 workflows

[jpata]

0,1,2,3,4,Inconsistency detected by ld.so: dl-open.c: 939: _dl_open: Assertion `_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT' failed!
terminate called after throwing an instance of 'cms::Exception'
  what():  An exception of category 'ExpressionEvaluator' occurred.
Exception Message:
compilation/linking failed
c++ -H -Wall -shared -Winvalid-pch -DGNU_GCC -D_GNU_SOURCE -DEIGEN_DONT_PARALLELIZE -DTBB_USE_GLIBCXX_VERSION=100300 -DTBB_SUPPRESS_DEPRECATED_MESSAGES -DTBB_PREVIEW_RESUMABLE_TASKS=1 -DTBB_PREVIEW_TASK_GROUP_EXTENSIONS=1 -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -DBOOST_MATH_DISABLE_STD_FPCLASSIFY -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX -DCMSSW_GIT_HASH='"CMSSW_12_5_X_2022-08-14-2300"' -DPROJECT_NAME='"CMSSW"' -DPROJECT_VERSION='"CMSSW_12_5_X_2022-08-14-2300"'     -isystem/cvmfs/cms-ib.cern.ch/nweek-02746/el8_amd64_gcc10/external/boost/1.78.0-12075919175e8d078539685f9234134a/include    -isystem/cvmfs/cms-ib.cern.ch/nweek-02746/el8_amd64_gcc10/lcg/root/6.24.07-a31cbfc28a0c92b3c007615905b5b9b2/include -isystem/cvmfs/cms-ib.cern.ch/nweek-02746/el8_amd64_gcc10/external/tbb/v2021.5.0-3cd580209e999b2fb4f8344347204353/include        -O2 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++1z -ftree-vectorize -Wstrict-overflow -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -msse3 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-deprecated-copy -Wno-unused-parameter -Wunused -Wparentheses -Wno-deprecated -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -Wno-error=unused-variable -DBOOST_DISABLE_ASSERTS -DUSE_CMS_DEPRECATED -fPIC -I/data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-14-2300/include/el8_amd64_gcc10/ -o /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-14-2300/tmp/VI_c6dcb08c932347dcbf3257129fc85552.so /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-14-2300/tmp/VI_c6dcb08c932347dcbf3257129fc85552.cc 2>&1
dlerror /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-14-2300/tmp/VI_c6dcb08c932347dcbf3257129fc85552.so: cannot open shared object file: No such file or directory 

/bin/sh: line 1: 12378 Aborted                 timeout 3600 ExpressionEvaluatorUnitTest

---> test ExpressionEvaluatorUnitTest had ERRORS

this does not look like the usual transient xrootd error.

[watson-ij]

@jpata Looking at the package, I don't think my code could cause errors in the ExpressionEvaluator test, it looks to only use code in CommonTools/Utils. Though, with a brief look I also couldn't see any other PRs failing with this issue. Should we just try again?

[perrotta]

this does not look like the usual transient xrootd error.

OTOH it did not appear in the previous tests Let re-test and see what happens

[perrotta]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-938bfc/26839/summary.html COMMIT: b4bec60695984deefc8e01465639e22722bd3509 CMSSW: CMSSW_12_5_X_2022-08-15-2300/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38990/26839/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 3367 differences found in the comparisons
• DQMHistoTests: Total files compared: 51
• DQMHistoTests: Total histograms compared: 3692476
• DQMHistoTests: Total failures: 4961
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 3687492
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: -0.004 KiB( 50 files compared)
• DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
• Checked 212 log files, 49 edm output root files, 51 DQM output files
• TriggerResults: found differences in 1 / 50 workflows

[jpata]

+reconstruction

• use rechits rather than segments for GEMMuon
• new dataformat for MuonGEMHitMatch added, but overall expected to reduce the file size due to saving less muons
• changes in muons, but also e.g. ECAL rechits which IIUC are cleaned against muons

[cmsbuild]

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, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

[CeliaFernandez]

I'm very sorry for coming up to this a little bit late (last week I was off). I have checked the code and I agree with the changes. However, I'm wondering if here, where both segments and hits are added up together, it would be better to split the sum:

https://github.com/cms-sw/cmssw/blob/b4bec60695984deefc8e01465639e22722bd3509/DataFormats/MuonReco/src/Muon.cc#L73

by adding a new type e.g. GEMHitAndTrackArbitration (or with a different approach). It is not clear to me if there is any chance that segments and hits would be mixed in this variable at some point.

[watson-ij]

Hi @CeliaFernandez, this seems like a good idea to me. The ME0 should produce Segments, so its probably clearer to separate them. I was thinking we could use the existing ME0SegmentAndTrackArbitration, but it probably makes more sense to separate GEMHits and GEMSegments, to keep it separate from the (eventually to be deprecated) ME0 objects. I will try to put together an update soon.

[watson-ij]

@CeliaFernandez I implemented the suggestion, let me know if this is okay.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38990/31642

• This PR adds an extra 44KB to repository

[cmsbuild]

Pull request #38990 was updated. @jpata, @cmsbuild, @mandrenguyen, @clacaputo can you please check and sign again.

[watson-ij]

please test