Cleanup, RecoJets/JetAnalyzers, Motivated by JetCorrector Migration

[wddgit]

PR description:

Cleanup in RecoJets/JetAnalyzers. Removes archaic example modules. This only deletes files.

The Core group is attempting to complete a migration started in 2014 that moves from using the old ::JetCorrector class to using the reco::JetCorrector class. We are trying to centrally complete this migration. In RecoJets, the old JetCorrector class is only used in the example modules deleted by this PR. This PR is one of many PRs related to this migration. At this point, there are PRs submitted removing all usage of the old JetCorrector. Four of these PRs are still under review (including this one) and the rest already merged. This is related to the consumes migration and multi-threading performance.

All of the deleted example modules are 10 to 15 years old and so full of deprecated non-functional code that it would be harmful for someone to read as an example. 10 to 15 years ago these might have been useful, but at this point deleting them is a positive thing independent of the JetCorrector migration. We should not be using resources to distribute, build, and maintain code like this.

Note that if someone believes these examples are valuable, the code could be restored from the git history and fully updated at some point in the future.

I'll also note that I only looked at code using the deprecated JetCorrector class. There is more code that could be updated or deleted in this package.

Some of the problems:

• Don't call consumes and use getByLabel (fails).
• Use deprecated JetCorrector (fails).
• No unit tests or relVal tests.
• Nothing references any of these other than test configs also deleted
• Configs load cff files that no longer exist
• Use corrections that no longer exist
• Ancient input files that probably no longer exist
• There is nothing deep and useful that really needs saving

PR validation:

Only deletions. Nothing to test. Nothing could be using them because they don't work anymore.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39864/32767

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

• RecoJets/JetAnalyzers (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@rappoccio, @jdolen, @ahinzmann, @jdamgov, @missirol, @nhanvtran, @gkasieczka, @clelange, @schoef, @mariadalfonso, @seemasharmafnal 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

[wddgit]

please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a27743/28543/summary.html
COMMIT: 97c452b
CMSSW: CMSSW_12_6_X_2022-10-26-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39864/28543/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: 13 differences found in the comparisons
• Reco comparison had 6 failed jobs
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3384029
• DQMHistoTests: Total failures: 12
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3383995
• 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

[clacaputo]

Hi @wddgit , could you please update the PR title? IIUC there is no "migration" but a clean-up. I suppose the removal as been discussed with @cms-sw/jetmet-pog-l2

[wddgit]

@clacaputo I modified the title and the description at the top. Hopefully that is more clear.

This has not been discussed recently at the jetmet pog. There are a few reasons that shouldn't be necessary. First, the JetCorrection migration was discussed long ago and has been approved for years. Almost all reconstruction code has already been migrated to use the new JetCorrector and not use the old JetCorrector. Second, I am a core developer and I've been doing this migration work in many different packages across CMSSW. If I gave a presentation for each package, it would take me a very long time to get it done. Third, there is no new functionality here. This PR just deletes dead code. There is not much to present or discuss.

If it is necessary, could you take care of presenting this to the pog? I know very little about jet or met reconstruction code or how it is documented and what examples exist...

Note that if the deleted example files are actually useful, then when it is convenient someone could restore them from git history and update them to be good examples of modern code.

[wddgit]

New info related to this: The PR that deletes the deprecated header file (JetCorrector.h) was submitted, #39953. I tested it on top of this PR (and 3 other similar ones), all PR tests pass. Further, I ran runTheMatrix.py (the full version with all 1000+ tests) and that passes. It successfully compiles and builds.

@clacaputo I saw your thumbs up to my last response. Are we waiting on the jet-met pog before approving this PR? When does that occur? Are there any other questions?

[clacaputo]

please test

just refreshing the test results

[cmsbuild]

-1

Failed Tests: UnitTests RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a27743/28833/summary.html
COMMIT: 97c452b
CMSSW: CMSSW_12_6_X_2022-11-07-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39864/28833/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 test-das-selected-lumis had ERRORS

RelVals-INPUT

• 1328.0DAS Error

Comparison Summary

Summary:

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

[wddgit]

please test

Try again. The latest failures don't appear related to the PR. Note this PR was included when I tested #39953 on Saturday and that passed.

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a27743/28843/summary.html
COMMIT: 97c452b
CMSSW: CMSSW_12_6_X_2022-11-07-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39864/28843/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

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

[wddgit]

please test

Try again, timeouts in the RelVals this time, is there any way to avoid those? Is it just too many things trying to run on our build machines at the same time? This appears unrelated to the PR.

Note that the PR tests have passed twice since the last time the code was modified. Given this is 100% file deletions, it is unlikely some underlying code change could break something. The only possibility would be someone adding a new test or adding to an existing so that it uses one of the deleted modules... And that wouldn't be a timeout.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a27743/28884/summary.html
COMMIT: 97c452b
CMSSW: CMSSW_12_6_X_2022-11-08-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39864/28884/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: 5 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3416402
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3416374
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 206 log files, 48 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[wddgit]

Hooray! Third time is the charm. Tests are green again.

[clacaputo]

+reconstruction

• the PR remove unused code
• no reco changes are introduced

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

[perrotta]

+1