Move commons.h to DataFormats/HGCalReco to break cyclic dep #45208

smuzaffar · 2024-06-12T20:06:12Z

This PR proposes to move RecoHGCal/TICL/interface/commons.h to DataFormats/HGCalReco/interface/commons.h to break the cyclic dependency. RecoHGCal/TICL depends DataFormats/HGCalReco via [a]. Moving the commons.h in to DataFormats/HGCalReco break the cyclic deps and should also fix the FWLite build errors [b] as RecoHGCal/TICL is not part of fwlite build

[a]

RecoHGCal/TICL/plugins/TICLCandidateFromTrackstersProducer.cc:#include "DataFormats/HGCalReco/interface/Trackster.h"
RecoHGCal/TICL/plugins/PFTICLProducer.cc:#include "DataFormats/HGCalReco/interface/TICLCandidate.h"
RecoHGCal/TICL/plugins/HGCGraph.cc:#include "DataFormats/HGCalReco/interface/Common.h"
RecoHGCal/TICL/plugins/SeedingRegionAlgoBase.h:#include "DataFormats/HGCalReco/interface/Common.h"
RecoHGCal/TICL/plugins/SeedingRegionAlgoBase.h:#include "DataFormats/HGCalReco/interface/TICLSeedingRegion.h"
RecoHGCal/TICL/plugins/TICLDumper.cc:#include "DataFormats/HGCalReco/interface/Trackster.h"
RecoHGCal/TICL/plugins/TICLDumper.cc:#include "DataFormats/HGCalReco/interface/TICLCandidate.h"
RecoHGCal/TICL/plugins/TICLDumper.cc:#include "DataFormats/HGCalReco/interface/Common.h"
RecoHGCal/TICL/plugins/TICLDumper.cc:#include "DataFormats/HGCalReco/interface/commons.h"
RecoHGCal/TICL/plugins/TICLLayerTileProducer.cc:#include "DataFormats/HGCalReco/interface/TICLLayerTile.h"
RecoHGCal/TICL/plugins/LinkingAlgoByDirectionGeometric.h:#include "DataFormats/HGCalReco/interface/commons.h"
RecoHGCal/TICL/plugins/LinkingAlgoByDirectionGeometric.h:#include "DataFormats/HGCalReco/interface/TICLLayerTile.h"
RecoHGCal/TICL/plugins/LinkingAlgoByDirectionGeometric.h:#include "DataFormats/HGCalReco/interface/Trackster.h"
RecoHGCal/TICL/plugins/TrackstersPCA.h:#include "DataFormats/HGCalReco/interface/Trackster.h"

[b]

In file included from input_line_7:70:
src/DataFormats/HGCalReco/interface/TICLCandidate.h:10:10: fatal error: 'RecoHGCal/TICL/interface/commons.h' file not found
#include "RecoHGCal/TICL/interface/commons.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

cmsbuild · 2024-06-12T20:06:31Z

cms-bot internal usage

smuzaffar · 2024-06-12T20:09:54Z

@felicepantaleo , your PR #44785 had introduced this dependency. please feel free to propose a better change ( e.g. move commons.h to some other common base package)

cmsbuild · 2024-06-12T20:12:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45208/40559

This PR adds an extra 48KB to repository

cmsbuild · 2024-06-12T20:13:04Z

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

It involves the following packages:

DataFormats/HGCalReco (upgrade, reconstruction)
RecoHGCal/TICL (upgrade, reconstruction)
Validation/HGCalValidation (dqm)

@rvenditti, @nothingface0, @antoniovagnerini, @tjavaid, @cmsbuild, @subirsarkar, @jfernan2, @syuvivida, @mandrenguyen, @srimanob can you please review it and eventually sign? Thanks.
@sobhatta, @vandreev11, @cseez, @rovere, @felicepantaleo, @youyingli, @forthommel, @missirol, @sameasy, @lecriste, @apsallid, @sethzenz, @hatakeyamak, @lgray, @pfs, @bsunanda this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

felicepantaleo · 2024-06-12T21:16:09Z

Thanks @smuzaffar DataFormats/HGCalReco is a good location.

felicepantaleo · 2024-06-12T21:16:38Z

@cmsbuild please test

cmsbuild · 2024-06-13T00:23:54Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9bdfeb/39857/summary.html
COMMIT: 416f488
CMSSW: CMSSW_14_1_X_2024-06-12-1900/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45208/39857/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 4 lines from the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3345018
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3344989
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

smuzaffar · 2024-06-13T08:51:05Z

hold

@felicepantaleo , there is already DataFormats/HGCalReco/interface/Common.h , Any objections on merging both DataFormats/HGCalReco/interface/Common.h and DataFormats/HGCalReco/interface/commons.h ?

cmsbuild · 2024-06-13T08:51:23Z

Pull request has been put on hold by @smuzaffar
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

felicepantaleo · 2024-06-13T12:17:26Z

@smuzaffar Since there is already a PR fixing pending stuff in TICLv5, maybe we can make the fix you propose here #45215 and close this PR so that we avoid conflicts, rebase, and retriggering of tests.

cmsbuild · 2024-06-13T19:24:02Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9bdfeb/39869/summary.html
COMMIT: 5e23d50
CMSSW: CMSSW_14_1_X_2024-06-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45208/39869/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3345018
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3344992
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

cmsbuild · 2024-06-14T06:39:31Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9bdfeb/39869/summary.html
COMMIT: 5e23d50
CMSSW: CMSSW_14_1_X_2024-06-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45208/39869/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3345018
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3344992
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

mandrenguyen · 2024-06-17T01:49:28Z

+1

smuzaffar · 2024-06-17T10:10:44Z

@cms-sw/dqm-l2 @cms-sw/upgrade-l2 can you please review this PR . This should fix the cyclic source level dependency between RecoHGCal/TICL and DataFormats/HGCalReco and also should fix the FWLite build error

smuzaffar · 2024-06-17T10:11:49Z

unhold

tjavaid · 2024-06-18T10:18:35Z

+1

srimanob · 2024-06-20T10:49:06Z

+Upgrade

cmsbuild · 2024-06-20T10:49:24Z

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

antoniovilela · 2024-06-20T22:49:43Z

+1

Move commons.h to DataFormats/HGCalReco to break cyclic dep

416f488

cmsbuild added this to the CMSSW_14_1_X milestone Jun 12, 2024

cmsbuild added reconstruction-pending dqm-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending labels Jun 12, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 12, 2024

cmsbuild added tests-started and removed tests-pending labels Jun 12, 2024

cmsbuild added tests-approved and removed tests-started labels Jun 13, 2024

cmsbuild added the hold label Jun 13, 2024

cmsbuild mentioned this pull request Jun 13, 2024

TICLv5 - Fix TracksterLinkingbySkeletons Logic #45215

Merged

merge commons.h in to Common.h

5e23d50

cmsbuild removed tests-approved code-checks-approved labels Jun 13, 2024

cmsbuild added tests-started and removed tests-pending labels Jun 13, 2024

cmsbuild added tests-rejected tests-started and removed tests-started tests-rejected labels Jun 13, 2024

cmsbuild added tests-approved and removed tests-started labels Jun 14, 2024

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jun 17, 2024

cmsbuild removed the hold label Jun 17, 2024

cmsbuild added dqm-approved and removed dqm-pending labels Jun 18, 2024

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Jun 20, 2024

cmsbuild added orp-approved and removed orp-pending labels Jun 20, 2024

cmsbuild merged commit 3028de7 into cms-sw:master Jun 20, 2024
11 checks passed

cmsbuild mentioned this pull request Jun 21, 2024

Heterogeneous Workflow (CLUE) in Phase2 HLT Simplified Menu #45178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move commons.h to DataFormats/HGCalReco to break cyclic dep #45208

Move commons.h to DataFormats/HGCalReco to break cyclic dep #45208

smuzaffar commented Jun 12, 2024

cmsbuild commented Jun 12, 2024 •

edited

Loading

smuzaffar commented Jun 12, 2024 •

edited

Loading

cmsbuild commented Jun 12, 2024

cmsbuild commented Jun 12, 2024

felicepantaleo commented Jun 12, 2024

felicepantaleo commented Jun 12, 2024

cmsbuild commented Jun 13, 2024

smuzaffar commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

felicepantaleo commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

cmsbuild commented Jun 14, 2024

mandrenguyen commented Jun 17, 2024

smuzaffar commented Jun 17, 2024

smuzaffar commented Jun 17, 2024

tjavaid commented Jun 18, 2024

srimanob commented Jun 20, 2024

cmsbuild commented Jun 20, 2024

antoniovilela commented Jun 20, 2024

Move commons.h to DataFormats/HGCalReco to break cyclic dep #45208

Move commons.h to DataFormats/HGCalReco to break cyclic dep #45208

Conversation

smuzaffar commented Jun 12, 2024

cmsbuild commented Jun 12, 2024 • edited Loading

smuzaffar commented Jun 12, 2024 • edited Loading

cmsbuild commented Jun 12, 2024

cmsbuild commented Jun 12, 2024

felicepantaleo commented Jun 12, 2024

felicepantaleo commented Jun 12, 2024

cmsbuild commented Jun 13, 2024

Comparison Summary

smuzaffar commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

felicepantaleo commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

RelVals-INPUT

Comparison Summary

cmsbuild commented Jun 14, 2024

Comparison Summary

mandrenguyen commented Jun 17, 2024

smuzaffar commented Jun 17, 2024

smuzaffar commented Jun 17, 2024

tjavaid commented Jun 18, 2024

srimanob commented Jun 20, 2024

cmsbuild commented Jun 20, 2024

antoniovilela commented Jun 20, 2024

cmsbuild commented Jun 12, 2024 •

edited

Loading

smuzaffar commented Jun 12, 2024 •

edited

Loading