Removing the PhysicsTools/MVATrainer package

[guitargeek]

PR description:

This PR aims to provide an alternative to the recently opened PR #27870.

After seeing #27870 it occurred to me that it might be reasonable to remove the PhysicsTools/MVATrainer package so nobody has to spend time supporting and fixing compiler warnings in this code in the future.

The MVATrainer [1] is a framework that wraps around TMVA to train MVAs directly on EDM data and interfacing with the CondDB. It's over 10 years old. Today, people either use TVMA outside of CMSSW on NanoAOD or private "ntuples" or go for machine learning in the python ecosystem.

The only code where the MVATrainer is referenced to in CMSSW is located in the TopQuarkAnalysis subsystem, which seems to be abandoned itself so I suppose removing the code where the MVATrainer is referenced there does not do any harm.

What do you think? Who is responsible for this package now? Is it maybe still used by someone?

[1] https://indico.cern.ch/event/25702/contributions/575840/attachments/451161/625567/plenary-b-tag.pdf

PR validation:

CMSSW compiles and local matrix tests pass.

if this PR is a backport please specify the original PR:

No backport intended.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27888/11669

• This PR adds an extra 36KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File PhysicsTools/MVATrainer/src/LeastSquares.cc modified in PR(s): Fix 5 warnings in PhysicsTools/MVATrainer #27870
  • File PhysicsTools/MVATrainer/src/MVATrainer.cc modified in PR(s): Fix 5 warnings in PhysicsTools/MVATrainer #27870
  • File PhysicsTools/MVATrainer/src/ProcLikelihood.cc modified in PR(s): Fix 5 warnings in PhysicsTools/MVATrainer #27870
  • File PhysicsTools/MVATrainer/src/ProcNormalize.cc modified in PR(s): Fix 5 warnings in PhysicsTools/MVATrainer #27870

[cmsbuild]

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

PhysicsTools/MVATrainer
RecoEgamma/EgammaTools
RecoTauTag/RecoTau
TopQuarkAnalysis/TopEventSelection
TopQuarkAnalysis/TopJetCombination
Utilities/ReleaseScripts

@perrotta, @smuzaffar, @Dr15Jones, @cmsbuild, @slava77, @santocch can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @emilbols, @Sam-Harper, @kreczko, @smoortga, @jainshilpi, @varuns23, @wddgit, @lgray, @HeinerTholen, @JyothsnaKomaragiri, @mverzett, @ferencek, @andrzejnovak, @pvmulder, @acaudron this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2234/console Started: 2019/08/28 14:27

[slava77]

@ferencek @pvmulder @arizzi
please check and clarify it if is OK to remove.
Looking back in my inbox I see only BTV use cases mentioned, at least for some more recent changes from 2013 or so.

[guitargeek]

I figured out it was also used for the Taus in the past (not only BTV), but that code got already removed here #26816.

[cmsbuild]

-1

Tested at: 95c326a

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d22888/2234/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

• Unit Tests:

I found errors in the following unit tests:

---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafTopJetCombination had ERRORS

[cmsbuild]

Comparison job queued.

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2497/console Started: 2019/09/12 15:52

[cmsbuild]

+1
Tested at: 6ea71cf
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d22888/2497/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d22888/2497/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 34
• DQMHistoTests: Total histograms compared: 2957224
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2956882
• DQMHistoTests: Total skipped: 341
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
• Checked 145 log files, 15 edm output root files, 34 DQM output files

[slava77]

+1

for #27888 6ea71cf

• mostly obsolete code removal to reduce maintenance load. No planned/future use cases were reported based on the call for comments in the RECO/AT/ML HNs.
• jenkins tests pass

[fabiocos]

+1

@santocch please have a look

[fabiocos]

merge

[santocch]

+1

[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 be automatically merged.