Remove 'TSGForOI' module #39726

andrea21z · 2022-10-13T15:12:33Z

PR description:

This PR is created to remove a module that are not used anymore in production:
TSGForOI from RecoMuon/TrackerSeedGenerator.

The issue is described in #37973.
The deprecated module was replaced by TSGForOIFromL2 in 2018, and this one was replaced in April by TSGForOIDNN (RecoMuon/TrackerSeedGenerator/plugins/TSGForOIDNN.h).
Files related with this module have been removed.
We update the L3MuonProducer test to run TSGForOIDNN instead of TSGForOI.

PR validation:

The proposed changes passed all code format and runTheMatrix tests.

cmsbuild · 2022-10-13T15:18:55Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39726/32563

This PR adds an extra 20KB to repository

cmsbuild · 2022-10-13T15:19:17Z

A new Pull Request was created by @andrea21z for master.

It involves the following packages:

RecoMuon/L3MuonProducer (hlt, reconstruction)
RecoMuon/TrackerSeedGenerator (reconstruction)

@cmsbuild, @missirol, @mandrenguyen, @clacaputo, @Martin-Grunewald can you please review it and eventually sign? Thanks.
@bellan, @silviodonato, @andrea21z, @abbiendi, @JanFSchulte, @Fedespring, @missirol, @HuguesBrun, @jhgoh, @CeliaFernandez, @trocino, @cericeci, @rociovilar 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

missirol · 2022-10-13T15:31:00Z

please test

@khaosmos93 @wonpoint4 @cms-sw/muon-pog-l2

Could you please confirm that this producer can be removed?

cmsbuild · 2022-10-13T19:13:54Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-77775a/28236/summary.html
COMMIT: 37c3f89
CMSSW: CMSSW_12_6_X_2022-10-13-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39726/28236/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/20834.0_TTbar_14TeV+2026D88+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/20834.75_TTbar_14TeV+2026D88_HLT75e33+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HLT75e33+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/20896.0_CloseByPGun_CE_E_Front_120um+2026D88+CE_E_Front_120um_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/20900.0_CloseByPGun_CE_H_Coarse_Scint+2026D88+CE_H_Coarse_Scint_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/21034.999_TTbar_14TeV+2026D88PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/23234.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3392309
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3392287
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 49 edm output root files, 49 DQM output files
TriggerResults: no differences found

missirol · 2022-10-14T09:31:56Z

RecoMuon/L3MuonProducer/test/newL3.py

-	        maxSeeds = cms.uint32(1),
-	        minEtaForTEC = cms.double(0.9),
-	        src = cms.InputTag("hltL2Muons","UpdatedAtVtx")
+    process.hltIterL3OISeedsFromL2Muons = cms.EDProducer("TSGForOIDNN",


This indentation is incorrect. The file is actually broken in the first place (try python3 newL3.py), b/c of mixed use of tabs and whitespaces. Please fix it by removing tabs, and reformatting (or, remove the file altogether if the MUO POG prefers that).

I have fixed the indentation problem, now it should run correctly. For the tabs/whitespaces part, I will wait to see if we want to remove the file or keep it.

Hi @andrea21z, this customizer is obsolete and does not compatible with the Run 3 menu. So I suggest you remove the file.

I agree, I will remove the test file.

missirol · 2022-10-14T09:33:28Z

assign muon-pog

See question in #39726 (comment).

cmsbuild · 2022-10-14T09:33:47Z

New categories assigned: muon-pog

@JanFSchulte,@gkaratha you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild · 2022-10-14T10:11:50Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39726/32571

This PR adds an extra 20KB to repository

khaosmos93 · 2022-10-17T12:22:39Z

Hi @missirol, we confirm that it can be safely removed, sorry for the delayed answer.

missirol · 2022-10-17T12:28:51Z

Thanks, @khaosmos93 . There is a pending item, #39726 (comment). The newL3.py file is currently broken (python3 newL3.py does not work). I suggest to fix it, unless you prefer to remove it. Please let @andrea21z know your feedback.

(I wasn't sure whether 'can be removed' refers only to the plugin, or also to the python test config.)

cmsbuild · 2022-10-17T14:02:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39726/32596

This PR adds an extra 12KB to repository

cmsbuild · 2022-10-17T14:03:04Z

Pull request #39726 was updated. @Martin-Grunewald, @gkaratha, @JanFSchulte, @clacaputo, @cmsbuild, @missirol, @mandrenguyen can you please check and sign again.

missirol · 2022-10-17T15:13:29Z

unassign muon-pog

I take #39726 (comment) as a sign-off of this PR by the MUO POG.

missirol · 2022-10-17T15:15:17Z

+hlt

Since the only difference is the removal of a file that wasn't used anywhere, the upcoming tests should pass just like they did before.

cmsbuild · 2022-10-17T18:41:03Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-77775a/28305/summary.html
COMMIT: 854f31f
CMSSW: CMSSW_12_6_X_2022-10-17-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39726/28305/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-77775a/2500.601_mc126X+TTBarMINIAOD12.6+NANO_mc12.6+HRV_NANO_mc

Summary:

No significant changes to the logs found
Reco comparison results: 10 differences found in the comparisons
Reco comparison had 6 failed jobs
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3391158
DQMHistoTests: Total failures: 90
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3391046
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 201 log files, 48 edm output root files, 48 DQM output files
TriggerResults: no differences found

mandrenguyen · 2022-10-20T06:28:56Z

+1

cmsbuild · 2022-10-20T06:29:21Z

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)

perrotta · 2022-10-20T06:39:17Z

+1

removing TSGForOI module

37c3f89

cmsbuild added this to the CMSSW_12_6_X milestone Oct 13, 2022

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Oct 13, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2022

cmsbuild added tests-started and removed tests-pending labels Oct 13, 2022

cmsbuild added tests-approved and removed tests-started labels Oct 13, 2022

missirol reviewed Oct 14, 2022

View reviewed changes

cmsbuild added the muon-pog-pending label Oct 14, 2022

fixing indentation

8a8053b

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Oct 14, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 14, 2022

removing obsolete test file

854f31f

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 17, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 17, 2022

cmsbuild removed the muon-pog-pending label Oct 17, 2022

cmsbuild added hlt-approved tests-started and removed hlt-pending tests-pending labels Oct 17, 2022

cmsbuild added tests-approved and removed tests-started labels Oct 17, 2022

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending pending-signatures labels Oct 20, 2022

cmsbuild added orp-approved and removed orp-pending labels Oct 20, 2022

cmsbuild merged commit ed4c9be into cms-sw:master Oct 20, 2022

andrea21z mentioned this pull request Oct 24, 2022

Outdated muon code to be removed #37973

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove 'TSGForOI' module #39726

Remove 'TSGForOI' module #39726

andrea21z commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

missirol commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

missirol Oct 14, 2022

andrea21z Oct 14, 2022

khaosmos93 Oct 17, 2022

andrea21z Oct 17, 2022

missirol commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

khaosmos93 commented Oct 17, 2022

missirol commented Oct 17, 2022 •

edited

cmsbuild commented Oct 17, 2022

cmsbuild commented Oct 17, 2022

missirol commented Oct 17, 2022

missirol commented Oct 17, 2022

cmsbuild commented Oct 17, 2022

mandrenguyen commented Oct 20, 2022

cmsbuild commented Oct 20, 2022

perrotta commented Oct 20, 2022

Remove 'TSGForOI' module #39726

Remove 'TSGForOI' module #39726

Conversation

andrea21z commented Oct 13, 2022

PR description:

PR validation:

cmsbuild commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

missirol commented Oct 13, 2022

cmsbuild commented Oct 13, 2022

Comparison Summary

missirol Oct 14, 2022

Choose a reason for hiding this comment

andrea21z Oct 14, 2022

Choose a reason for hiding this comment

khaosmos93 Oct 17, 2022

Choose a reason for hiding this comment

andrea21z Oct 17, 2022

Choose a reason for hiding this comment

missirol commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

cmsbuild commented Oct 14, 2022

khaosmos93 commented Oct 17, 2022

missirol commented Oct 17, 2022 • edited

cmsbuild commented Oct 17, 2022

cmsbuild commented Oct 17, 2022

missirol commented Oct 17, 2022

missirol commented Oct 17, 2022

cmsbuild commented Oct 17, 2022

Comparison Summary

mandrenguyen commented Oct 20, 2022

cmsbuild commented Oct 20, 2022

perrotta commented Oct 20, 2022

missirol commented Oct 17, 2022 •

edited