Removed CalibTracker/SiStripCommon dependency from CalibFormats/SiStripObjects #33888

Dr15Jones · 2021-05-29T02:12:33Z

PR description:

CalibFormat objects should not be reading files to get intermediate data as this violates one of the guiding principles of CMSSW. This change moves the needed information into a separate class, SiStripDetInfo and requires code that constructs a SiStripQuality or a SiStripGain to pass in the appropriate SiStripDetInfo.

The class SiStripDetInfoFileReader now returns the SiStripDetInfo.

This now makes CalibFormats/SiStripObjects independent from algorithmic code.

PR validation:

Code compiles.

The information needed is now in SiStripDetInfo. It is up to the code that constructs a SiStripQuality to get access to a SiStripDetInfo.

This allows dropping dependency on CalibTracker/SiStripCommon.

Dr15Jones · 2021-05-29T02:13:09Z

@davidlange6 this should help with the ROOT modules build.

cmsbuild · 2021-05-29T02:20:42Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33888/22931

This PR adds an extra 88KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQMOffline/CalibTracker/plugins/SiStripQualityHotStripIdentifierRoot.cc modified in PR(s): Introduce AlCaHarvesting / PCLMetadataWriter unit test #33880

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33888/22931/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33888/22931/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

cmsbuild · 2021-05-29T03:30:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33888/22936

This PR adds an extra 88KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DQMOffline/CalibTracker/plugins/SiStripQualityHotStripIdentifierRoot.cc modified in PR(s): Introduce AlCaHarvesting / PCLMetadataWriter unit test #33880

cmsbuild · 2021-05-29T03:30:40Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

CalibFormats/SiStripObjects
CalibTracker/SiStripCommon
CalibTracker/SiStripESProducers
CalibTracker/SiStripHitEfficiency
CalibTracker/SiStripQuality
CondCore/SiStripPlugins
DQM/SiStripMonitorSummary
DQMOffline/CalibTracker
OnlineDB/SiStripESSources
RecoLocalTracker/SiStripClusterizer

@perrotta, @malbouis, @andrius-k, @yuanchao, @kmaeshima, @tlampen, @ErnestaP, @ahmad3213, @cmsbuild, @jpata, @jfernan2, @slava77, @ggovi, @pohsun, @rvenditti, @francescobrivio can you please review it and eventually sign? Thanks.
@erikbutz, @echabert, @felicepantaleo, @pieterdavid, @robervalwalsh, @fioriNTU, @threus, @venturia, @hdelanno, @makortel, @JanFSchulte, @rociovilar, @yduhm, @sroychow, @GiacomoSguazzoni, @rovere, @VinInn, @tocheng, @jandrea, @alesaggio, @idebruyn, @mmusich, @mtosi, @arossi83, @gbenelli, @gpetruc this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

jfernan2 · 2021-05-29T08:37:22Z

please test

cmsbuild · 2021-05-29T09:16:08Z

-1

Failed Tests: Build ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-59ed9d/15406/summary.html
COMMIT: fa8eceb
CMSSW: CMSSW_12_0_X_2021-05-28-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33888/15406/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation error when building:

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/Phase2TrackerCablingCfgESSource.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripApvGainFakeESSource.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripBackPlaneCorrectionFakeESSource.cc
In file included from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-05-28-2300/src/Geometry/TrackerNumberingBuilder/interface/utils.h:4,
                 from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripBackPlaneCorrectionFakeESSource.cc:26:
/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-05-28-2300/src/Geometry/TrackerNumberingBuilder/interface/GeometricDet.h:12:10: fatal error: DD4hep/Shapes.h: No such file or directory
   12 | #include 
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripBadModuleConfigurableFakeESSource.cc
gmake: *** [tmp/slc7_amd64_gcc900/src/CalibTracker/SiStripESProducers/plugins/CalibTrackerSiStripFakeESProducersPlugins/fake/SiStripBackPlaneCorrectionFakeESSource.cc.o] Error 1

Clang Build

I found compilation error while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripPedestalsFakeESSource.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripQualityFakeESSource.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripThresholdFakeESSource.cc
>> Compiling edm plugin /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/modules.cc
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripHashedDetIdFakeESSource.cc:6:
/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-05-28-2300/src/Geometry/TrackerNumberingBuilder/interface/GeometricDet.h:12:10: fatal error: 'DD4hep/Shapes.h' file not found
#include 
         ^~~~~~~~~~~~~~~~~
In file included from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_0_X_2021-05-28-2300/src/CalibTracker/SiStripESProducers/plugins/fake/SiStripLorentzAngleFakeESSource.cc:36:
/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-05-28-2300/src/Geometry/TrackerNumberingBuilder/interface/GeometricDet.h:12:10: fatal error: 'DD4hep/Shapes.h' file not found
#include

Dr15Jones · 2021-05-29T11:25:59Z

@smuzaffar why did the <use name="dd4hep"/> in Geometry/TrackerNumberingBuilder/BuildFile.xml not get applied to the plugins in CalibTracker/SiStripESProducers?

I figured it out. I removed a <use ../> directive higher up in the hierarchy.

jfernan2 · 2021-06-03T13:45:41Z

+1

slava77 · 2021-06-03T18:06:27Z

RecoLocalTracker/SiStripZeroSuppression/BuildFile.xml

@@ -8,6 +8,7 @@
 <use name="CondFormats/SiStripObjects"/>
 <use name="CalibFormats/SiStripObjects"/>
 <use name="CalibTracker/Records"/>
+<use name="Geometry/TrackerGeometryBuilder"/>


nevermind, I removed the comment; there's no problem here

slava77 · 2021-06-03T18:09:44Z

+reconstruction

for #33888 f589717

code changes in reco are rather trivial:
- adapting to the new interface in RecoLocalTracker/SiStripClusterizer/test
- update/fix of some missing package dependencies
jenkins tests pass and comparisons with the baseline show no (relevant) differences

civanch · 2021-06-03T21:08:29Z

+1

yuanchao · 2021-06-04T18:39:27Z

+1

remove CalibTracker/SiStripCommon dependency
matrix tests and unit tests passed

qliphy · 2021-06-07T00:38:51Z

kindly ping @cms-sw/db-l2

Dr15Jones · 2021-06-08T13:50:17Z

Ping @cms-sw/db-l2

ggovi · 2021-06-08T18:38:38Z

+1

cmsbuild · 2021-06-08T18:39:06Z

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

qliphy · 2021-06-09T00:27:27Z

+1

Dr15Jones added 2 commits May 28, 2021 21:06

Removed file reading from SiStripQuality

015b950

The information needed is now in SiStripDetInfo. It is up to the code that constructs a SiStripQuality to get access to a SiStripDetInfo.

Removed file reading from SiStripGain

2112353

This allows dropping dependency on CalibTracker/SiStripCommon.

cmsbuild added this to the CMSSW_12_0_X milestone May 29, 2021

cmsbuild added alca-pending code-checks-pending db-pending dqm-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 29, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 29, 2021

code checks

fa8eceb

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2021

cmsbuild added tests-started tests-rejected and removed tests-pending tests-started labels May 29, 2021

Updated BuildFile.xml files to new dependencies

4b5e977

slava77 reviewed Jun 3, 2021

View reviewed changes

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Jun 3, 2021

cmsbuild added simulation-approved and removed simulation-pending labels Jun 3, 2021

cmsbuild added alca-approved and removed alca-pending labels Jun 4, 2021

cmsbuild mentioned this pull request Jun 6, 2021

Introduce Quality for PixelTracks. Improve Patatrack duplicate & ambiguity resolution #33889

Merged

cmsbuild added db-approved fully-signed and removed db-pending pending-signatures labels Jun 8, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 9, 2021

cmsbuild merged commit 6c6436e into cms-sw:master Jun 9, 2021

cmsbuild mentioned this pull request Jun 9, 2021

initialize all of pat Muon sim info #34056

Merged

Dr15Jones deleted the improveCalibFormatsSiStripObjects branch June 9, 2021 14:37

cmsbuild mentioned this pull request Jun 9, 2021

Use LFN instead of hardcoded cms-xrd-global site #34057

Merged

mmusich mentioned this pull request Jun 10, 2021

Fix SiStripQuality merging with FED errors from a DQM file #34082

Merged

pieterdavid mentioned this pull request Jun 30, 2021

SiStripDetInfoFileReader refactoring and cleanup #34285

Merged

mmusich mentioned this pull request Jul 13, 2021

Introduce SiPixelDynamicInefficiency payload inspector #34457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed CalibTracker/SiStripCommon dependency from CalibFormats/SiStripObjects #33888

Removed CalibTracker/SiStripCommon dependency from CalibFormats/SiStripObjects #33888

Dr15Jones commented May 29, 2021

Dr15Jones commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

jfernan2 commented May 29, 2021

cmsbuild commented May 29, 2021

Dr15Jones commented May 29, 2021 •

edited

jfernan2 commented Jun 3, 2021

slava77 Jun 3, 2021

slava77 commented Jun 3, 2021

civanch commented Jun 3, 2021

yuanchao commented Jun 4, 2021

qliphy commented Jun 7, 2021

Dr15Jones commented Jun 8, 2021

ggovi commented Jun 8, 2021

cmsbuild commented Jun 8, 2021

qliphy commented Jun 9, 2021

Removed CalibTracker/SiStripCommon dependency from CalibFormats/SiStripObjects #33888

Removed CalibTracker/SiStripCommon dependency from CalibFormats/SiStripObjects #33888

Conversation

Dr15Jones commented May 29, 2021

PR description:

PR validation:

Dr15Jones commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

cmsbuild commented May 29, 2021

jfernan2 commented May 29, 2021

cmsbuild commented May 29, 2021

Build

Clang Build

Dr15Jones commented May 29, 2021 • edited

jfernan2 commented Jun 3, 2021

slava77 Jun 3, 2021

Choose a reason for hiding this comment

slava77 commented Jun 3, 2021

civanch commented Jun 3, 2021

yuanchao commented Jun 4, 2021

qliphy commented Jun 7, 2021

Dr15Jones commented Jun 8, 2021

ggovi commented Jun 8, 2021

cmsbuild commented Jun 8, 2021

qliphy commented Jun 9, 2021

Dr15Jones commented May 29, 2021 •

edited