Clean up and migrating edm::RefToBase to edm::Ptr (list 2 for RNTuple) #43080

jeongeun · 2023-10-23T03:34:06Z

PR description:

This pull request is the second update to address the opened issue #42737 :

ROOT is eventually deprecating the current TTree data storage in favor of their new RNTuple which is not planned to support dynamic polymorphism. One such use case in our data formats is edm::RefToBase<T> that internally uses dynamic-polymorphism based type erasure. The straightforward migration path is towards edm::Ptr<T>.

Previous PR #43055 covered the first case on the list.

And this PR covers the second case :
It contains the set of edm::RefToBase<T> that the referenced-to class appear to be unused.
The unused code, the dictionaries and/or type aliases are removed.

As described in #42737,

Clean up the unused reco::WMuNuCandidatePtr and reco::WMuNuCandidate.
Replace edm::RefToBase<CaloRecHit> with edm::Ptr<CaloRecHit> (if no need to be kept, then I will remove).
edm::RefToBase<CaloRecHit>CaloRecHitRef used as a member data of CaloRecHitCandidate. And CaloRecHitCandidate seems to be only produced by CaloRecHitCandidateProducer<T>. Unclear if anything consumes. The template instantiations seem to be unused.

PR validation:

Tested in CMSSW_13_3_0_pre3, the basic test passed in the CMSSW PR instructions

cmsbuild · 2023-10-23T03:43:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43080/37313

This PR adds an extra 24KB to repository

cmsbuild · 2023-10-23T03:43:25Z

A new Pull Request was created by @jeongeun (JeongEun Lee) for master.

It involves the following packages:

AnalysisDataFormats/EWK (analysis)
CommonTools/RecoAlgos (reconstruction)
DataFormats/CaloRecHit (reconstruction)
DataFormats/RecoCandidate (reconstruction)

@tvami, @jfernan2, @mandrenguyen, @cmsbuild can you please review it and eventually sign? Thanks.
@missirol, @jdolen, @thomreis, @hatakeyamak, @ReyerBand, @abbiendi, @jhgoh, @ahinzmann, @gouskos, @rovere, @wang0jin, @gkasieczka, @rappoccio this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

jfernan2 · 2023-10-23T09:04:14Z

please test

cmsbuild · 2023-10-23T13:10:59Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8c65a5/35346/summary.html
COMMIT: b1af56f
CMSSW: CMSSW_13_3_X_2023-10-22-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43080/35346/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 6 lines from the logs
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357400
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3357375
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

tvami · 2023-10-23T18:47:13Z

+analysis

PR in line with the desc
tests pass, diffs in MsgLogger only

jfernan2 · 2023-10-24T07:34:49Z

+1

cmsbuild · 2023-10-24T07:35:10Z

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

makortel · 2023-10-24T13:42:00Z

DataFormats/RecoCandidate/src/classes_def.xml.generated

What is the purpose of this file?

No purpose. Sorry it happened to be included because I just used git add * command. I remove it.
(This file was created after running the scram build updateclassversion).

cmsbuild · 2023-10-27T05:13:43Z

Pull request #43080 was updated. @mandrenguyen, @tvami, @cmsbuild, @jfernan2 can you please check and sign again.

jfernan2 · 2023-10-27T08:28:41Z

please test

cmsbuild · 2023-10-27T11:07:26Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8c65a5/35457/summary.html
COMMIT: b001ace
CMSSW: CMSSW_13_3_X_2023-10-26-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43080/35457/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

4.764.76_ZMuSkim2012D/step2_ZMuSkim2012D.log

Comparison Summary

Summary:

You potentially removed 7 lines from the logs
Reco comparison results: 24 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357400
DQMHistoTests: Total failures: 1067
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3356311
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

jfernan2 · 2023-10-27T11:54:00Z

+1
Failure seems related to xrootd input in 4.76 RelVal

makortel · 2023-10-27T19:44:12Z

Thanks @jeongeun!

tvami · 2023-10-30T21:26:44Z

+analysis

cmsbuild · 2023-10-30T21:27:08Z

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

tvami · 2023-10-30T21:27:18Z

@cmsbuild , please test

cmsbuild · 2023-10-31T00:10:49Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8c65a5/35512/summary.html
COMMIT: b001ace
CMSSW: CMSSW_13_3_X_2023-10-30-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43080/35512/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 8 lines from the logs
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3362691
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3362657
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

antoniovilela · 2023-11-01T13:39:29Z

+1

cmsbuild added this to the CMSSW_13_3_X milestone Oct 23, 2023

cmsbuild added reconstruction-pending analysis-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 23, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 23, 2023

cmsbuild added tests-started and removed tests-pending labels Oct 23, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 23, 2023

cmsbuild added analysis-approved and removed analysis-pending labels Oct 23, 2023

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Oct 24, 2023

makortel reviewed Oct 24, 2023

View reviewed changes

cmsbuild removed analysis-approved reconstruction-approved fully-signed labels Oct 24, 2023

cmsbuild added the code-checks-approved label Oct 27, 2023

jeongeun force-pushed the rmRefToBase_2 branch from 481be8f to b001ace Compare October 27, 2023 05:14

cmsbuild added tests-started and removed tests-pending labels Oct 27, 2023

cmsbuild added tests-rejected and removed tests-started labels Oct 27, 2023

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Oct 27, 2023

makortel mentioned this pull request Oct 27, 2023

edm::RefToBase is deprecated #42737

Open

34 tasks

cmsbuild added analysis-approved fully-signed and removed analysis-pending pending-signatures labels Oct 30, 2023

cmsbuild added tests-started and removed tests-rejected labels Oct 30, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 31, 2023

cmsbuild added orp-approved and removed orp-pending labels Nov 1, 2023

cmsbuild merged commit d40536b into cms-sw:master Nov 1, 2023
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up and migrating edm::RefToBase to edm::Ptr (list 2 for RNTuple) #43080

Clean up and migrating edm::RefToBase to edm::Ptr (list 2 for RNTuple) #43080

jeongeun commented Oct 23, 2023

cmsbuild commented Oct 23, 2023

cmsbuild commented Oct 23, 2023

jfernan2 commented Oct 23, 2023

cmsbuild commented Oct 23, 2023

tvami commented Oct 23, 2023

jfernan2 commented Oct 24, 2023

cmsbuild commented Oct 24, 2023

makortel Oct 24, 2023

jeongeun Oct 24, 2023

cmsbuild commented Oct 27, 2023

jfernan2 commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

jfernan2 commented Oct 27, 2023

makortel commented Oct 27, 2023

tvami commented Oct 30, 2023

cmsbuild commented Oct 30, 2023

tvami commented Oct 30, 2023

cmsbuild commented Oct 31, 2023

antoniovilela commented Nov 1, 2023

Clean up and migrating edm::RefToBase to edm::Ptr (list 2 for RNTuple) #43080

Clean up and migrating edm::RefToBase to edm::Ptr (list 2 for RNTuple) #43080

Conversation

jeongeun commented Oct 23, 2023

PR description:

PR validation:

cmsbuild commented Oct 23, 2023

cmsbuild commented Oct 23, 2023

jfernan2 commented Oct 23, 2023

cmsbuild commented Oct 23, 2023

Comparison Summary

tvami commented Oct 23, 2023

jfernan2 commented Oct 24, 2023

cmsbuild commented Oct 24, 2023

makortel Oct 24, 2023

Choose a reason for hiding this comment

jeongeun Oct 24, 2023

Choose a reason for hiding this comment

cmsbuild commented Oct 27, 2023

jfernan2 commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

RelVals-INPUT

Comparison Summary

jfernan2 commented Oct 27, 2023

makortel commented Oct 27, 2023

tvami commented Oct 30, 2023

cmsbuild commented Oct 30, 2023

tvami commented Oct 30, 2023

cmsbuild commented Oct 31, 2023

Comparison Summary

antoniovilela commented Nov 1, 2023