Remove unused headers in DataFormats package #35071

yuanchao · 2021-08-30T11:24:34Z

PR description:

Remove unused header for DataFormats packages #4

Following the issue #31505, the following headers are unused in DataFormats package:

DataFormats/BTauReco/interface/PFCombinedTauTagInfo.h
DataFormats/BTauReco/interface/PFIsolatedTauTagInfo.h
DataFormats/BTauReco/interface/TrackIPData.h
DataFormats/Common/interface/NewPolicy.h
DataFormats/Common/interface/OwnArray.h
DataFormats/Common/interface/TransientDataFrame.h
DataFormats/Common/interface/debugging_allocator.h
DataFormats/Common/test/OwnArray_t.cpp
DataFormats/Common/test/testOwnArray.cc
DataFormats/GeometrySurface/interface/private/extTkRotation.h
DataFormats/GeometrySurface/interface/private/newTkRotation.h
DataFormats/GeometryVector/interface/OnePiRange.h
DataFormats/GeometryVector/interface/private/extBasic2DVector.h
DataFormats/METReco/interface/PFClusterMETCollection.h
DataFormats/Math/interface/SSEArray.h
DataFormats/Math/interface/private/AVXVec.h
DataFormats/ParticleFlowReco/interface/PFBlockElementFwd.h
DataFormats/ParticleFlowReco/interface/PFBlockElementSuperClusterFwd.h
DataFormats/ParticleFlowReco/interface/PFClusterShapeAssociation.h
DataFormats/ParticleFlowReco/interface/PFParticleFwd.h
DataFormats/ParticleFlowReco/interface/PFSuperClusterFwd.h
DataFormats/ParticleFlowReco/interface/PFTrajectoryPointFwd.h
DataFormats/Provenance/interface/BranchDescriptionIndex.h
DataFormats/Provenance/interface/TypeInBranchType.h
DataFormats/RPCDigi/interface/RPCDigiL1Linkfwd.h
DataFormats/SiPixelDigi/interface/PixelDigiCollectionfwd.h
DataFormats/SiStripCommon/interface/ConstantsForSummaryPlots.h
DataFormats/SiStripDigi/interface/SiStripDigifwd.h
DataFormats/TrackerRecHit2D/interface/SiPixelRecHitfwd.h
DataFormats/TrajectorySeed/interface/BasicTrajectorySeed.h

Though the followings are listed in #31505 , they are found to be used in other packages. So not included.

DataFormats/GeometrySurface/interface/private/oldTkRotation.h
DataFormats/GeometryVector/interface/private/oldBasic2DVector.h
DataFormats/GeometryVector/interface/private/oldBasic3DVector.h
DataFormats/SiStripCluster/interface/SiStripClusterfwd.h

PR validation:

Code compiles. Run local limited runTheMatrix.py tests.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Not a backport and no backport forseen.

cmsbuild · 2021-08-30T11:31:14Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35071/24952

This PR adds an extra 28KB to repository

cmsbuild · 2021-08-30T11:31:42Z

A new Pull Request was created by @yuanchao (Yuan CHAO) for master.

It involves the following packages:

DataFormats/BTauReco (reconstruction)
DataFormats/Common (core)
DataFormats/GeometrySurface (simulation)
DataFormats/GeometryVector (simulation)
DataFormats/METReco (reconstruction)
DataFormats/Math (reconstruction)
DataFormats/ParticleFlowReco (reconstruction)
DataFormats/Provenance (core)
DataFormats/RPCDigi (simulation)
DataFormats/SiPixelDigi (simulation)
DataFormats/SiStripCommon (reconstruction)
DataFormats/SiStripDigi (simulation)
DataFormats/TrackerRecHit2D (reconstruction)
DataFormats/TrajectorySeed (reconstruction)

@smuzaffar, @civanch, @Dr15Jones, @makortel, @mdhildreth, @cmsbuild, @slava77, @jpata can you please review it and eventually sign? Thanks.
@erikbutz, @rappoccio, @gouskos, @echabert, @felicepantaleo, @hatakeyamak, @robervalwalsh, @emilbols, @OzAmram, @ahinzmann, @demuller, @threus, @seemasharmafnal, @mmarionncern, @makortel, @JanFSchulte, @lgray, @jdolen, @ferencek, @pieterdavid, @rovere, @VinInn, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @alesaggio, @mmusich, @mtosi, @fabiocos, @clelange, @AlexDeMoor, @wddgit, @JyothsnaKomaragiri, @dkotlins, @cbernet, @gpetruc, @mariadalfonso, @andrzejnovak, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

yuanchao · 2021-08-30T11:49:10Z

@cmsbuild please test

cmsbuild · 2021-08-30T15:57:08Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-186496/18130/summary.html
COMMIT: 1c23b82
CMSSW: CMSSW_12_1_X_2021-08-29-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35071/18130/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 11
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3000318
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

makortel · 2021-08-30T16:20:49Z

+core

civanch · 2021-08-30T20:19:06Z

+1

jpata · 2021-09-01T07:31:46Z

@VinInn you may be familiar if SSEArray.h, AVXVec.h are really outdated, or perhaps they may still be needed?

@hatakeyamak @bendavid @kdlong @juska can you recall if the forward defs in PF (PFBlockElementFwd.h, PFBlockElementSuperClusterFwd.h, ...) are needed for some reason?

This is not in reco as such, but extBasic2DVector.h is removed, while extBasic3DVector.h is kept/used. Perhaps extBasic2DVector.h might be needed at some point?

yuanchao · 2021-09-01T10:13:41Z

@jpata Surely if you think extBasic2DVector.h would be needed and wants to keep it. I'll restore it and re-commit.

smuzaffar · 2021-09-01T10:31:22Z

@yuanchao , the following header are needed

DataFormats/Math/interface/private/AVXVec.h is used https://cmssdt.cern.ch/lxr/source/DataFormats/Math/interface/SSEVec.h#0385
DataFormats/GeometryVector/interface/private/extBasic2DVector.h is used https://cmssdt.cern.ch/lxr/source/DataFormats/GeometryVector/interface/Basic2DVector.h#0006

jpata · 2021-09-01T13:51:04Z

@cmsbuild please test

cmsbuild · 2021-09-01T17:20:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-186496/18211/summary.html
COMMIT: 2c3e3e8
CMSSW: CMSSW_12_1_X_2021-08-31-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35071/18211/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: 39
DQMHistoTests: Total histograms compared: 3000404
DQMHistoTests: Total failures: 11
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3000370
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 38 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

civanch · 2021-09-01T19:23:29Z

+1

jpata · 2021-09-02T11:18:11Z

@hatakeyamak @bendavid @kdlong @juska kind ping, let me know if you see any reason to keep the PF-related forward defs (PFBlockElementFwd.h, ...)

hatakeyamak · 2021-09-02T11:25:08Z

Oops. Sorry.
I don't. Probably they not needed particularly if compile still goes and jenkins run without them.
Others, agree?

jpata · 2021-09-02T11:54:00Z

OK, apart from jenkins, I was wondering if there's some untested/nonstandard reason why they exist. But thanks for confirming!

jpata · 2021-09-02T11:55:04Z

+reconstruction

technical, removes unused headers
no differences in tests

perrotta · 2021-09-07T07:00:05Z

@cms-sw/core-l2 you already signed once: do you have any issue with the updated version of this PR?

makortel · 2021-09-07T13:25:59Z

+core

cmsbuild · 2021-09-07T13:26:27Z

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

perrotta · 2021-09-07T13:35:10Z

+1

makortel · 2021-09-08T01:18:17Z

DataFormats/GeometrySurface/interface/private/extTkRotation.h

@@ -1,366 +0,0 @@
-#ifndef Geom_newTkRotation_H


The ppc64le build is failing because this file was removed. It gets #included here

cmssw/DataFormats/GeometrySurface/interface/TkRotation.h

Lines 6 to 12 in 51c9398

#if defined(USE_EXTVECT)

#include "private/extTkRotation.h"

#elif defined(USE_SSEVECT)

#include "private/sseTkRotation.h"

#else

#include "private/oldTkRotation.h"

#endif

Thank you @makortel
Indeed USE_EXTVECT is currently used in several places in cmssw, and I think that DataFormats/GeometrySurface/interface/private/extTkRotation.h should be restored: do you agree @yuanchao @cms-sw/geometry-l2?

yes it should be restored. It will also fail when we build with avx flags on

perrotta · 2021-09-08T08:07:45Z

@makortel @smuzaffar @yuanchao I restored that file in #35190

Remove unused headers in DataFormats package.

1c23b82

cmsbuild added this to the CMSSW_12_1_X milestone Aug 30, 2021

cmsbuild added code-checks-pending core-pending orp-pending pending-signatures reconstruction-pending simulation-pending tests-pending labels Aug 30, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 30, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 30, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 30, 2021

cmsbuild added core-approved and removed core-pending labels Aug 30, 2021

cmsbuild added simulation-approved and removed simulation-pending labels Aug 30, 2021

restore AVXVec.h and extBasic2DVector.h

5618cd0

cmsbuild removed the core-approved label Sep 1, 2021

cmsbuild added tests-started and removed tests-pending labels Sep 1, 2021

cmsbuild added tests-approved and removed tests-started labels Sep 1, 2021

cmsbuild added simulation-approved and removed simulation-pending labels Sep 1, 2021

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 2, 2021

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Sep 7, 2021

cmsbuild added orp-approved and removed orp-pending labels Sep 7, 2021

cmsbuild merged commit 3ce0b94 into cms-sw:master Sep 7, 2021

makortel reviewed Sep 8, 2021

View reviewed changes

cmsbuild mentioned this pull request Sep 8, 2021

Updated root to tip of branch v6-24-00-patches cms-sw/cmsdist#7271

Closed

perrotta mentioned this pull request Sep 8, 2021

Restore DataFormats/GeometrySurface/interface/private/extTkRotation.h #35190

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unused headers in DataFormats package #35071

Remove unused headers in DataFormats package #35071

yuanchao commented Aug 30, 2021

cmsbuild commented Aug 30, 2021

cmsbuild commented Aug 30, 2021

yuanchao commented Aug 30, 2021 •

edited

Loading

cmsbuild commented Aug 30, 2021

makortel commented Aug 30, 2021

civanch commented Aug 30, 2021

jpata commented Sep 1, 2021

yuanchao commented Sep 1, 2021

smuzaffar commented Sep 1, 2021

jpata commented Sep 1, 2021

cmsbuild commented Sep 1, 2021

civanch commented Sep 1, 2021

jpata commented Sep 2, 2021

hatakeyamak commented Sep 2, 2021 •

edited

Loading

jpata commented Sep 2, 2021

jpata commented Sep 2, 2021

perrotta commented Sep 7, 2021

makortel commented Sep 7, 2021

cmsbuild commented Sep 7, 2021

perrotta commented Sep 7, 2021

makortel Sep 8, 2021

perrotta Sep 8, 2021

smuzaffar Sep 8, 2021 •

edited

Loading

perrotta commented Sep 8, 2021

	#if defined(USE_EXTVECT)
	#include "private/extTkRotation.h"
	#elif defined(USE_SSEVECT)
	#include "private/sseTkRotation.h"
	#else
	#include "private/oldTkRotation.h"
	#endif

Remove unused headers in DataFormats package #35071

Remove unused headers in DataFormats package #35071

Conversation

yuanchao commented Aug 30, 2021

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Aug 30, 2021

cmsbuild commented Aug 30, 2021

yuanchao commented Aug 30, 2021 • edited Loading

cmsbuild commented Aug 30, 2021

Comparison Summary

makortel commented Aug 30, 2021

civanch commented Aug 30, 2021

jpata commented Sep 1, 2021

yuanchao commented Sep 1, 2021

smuzaffar commented Sep 1, 2021

jpata commented Sep 1, 2021

cmsbuild commented Sep 1, 2021

Comparison Summary

civanch commented Sep 1, 2021

jpata commented Sep 2, 2021

hatakeyamak commented Sep 2, 2021 • edited Loading

jpata commented Sep 2, 2021

jpata commented Sep 2, 2021

perrotta commented Sep 7, 2021

makortel commented Sep 7, 2021

cmsbuild commented Sep 7, 2021

perrotta commented Sep 7, 2021

makortel Sep 8, 2021

Choose a reason for hiding this comment

perrotta Sep 8, 2021

Choose a reason for hiding this comment

smuzaffar Sep 8, 2021 • edited Loading

Choose a reason for hiding this comment

perrotta commented Sep 8, 2021

yuanchao commented Aug 30, 2021 •

edited

Loading

hatakeyamak commented Sep 2, 2021 •

edited

Loading

smuzaffar Sep 8, 2021 •

edited

Loading