New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[91X] Unify alignment producers #18138
[91X] Unify alignment producers #18138
Conversation
A new Pull Request was created by @ghellwig (Gregor Mittag) for master. It involves the following packages: Alignment/CommonAlignment @ghellwig, @arunhep, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 75138fa The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: Build
I found an error when building: >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PhysicsToolsObjects/test/TGraphWriter.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PhysicsToolsObjects/test/SiStripDeDxMipReader.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PhysicsToolsObjects/test/SiStripDeDx2DReader.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PhysicsToolsObjects/test/SiStripDeDxMipBuilder.cc >> Compiling edm plugin /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PhysicsToolsObjects/test/TFormulaWriter.cc /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PCLConfig/plugins/AlignPCLThresholdsReader.cc:14:63: error: expected template-name before '<' token class AlignPCLThresholdsReader : public edm::one::EDAnalyzer<> { ^ /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PCLConfig/plugins/AlignPCLThresholdsReader.cc:14:63: error: expected '{' before '<' token /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PCLConfig/plugins/AlignPCLThresholdsReader.cc:14:63: error: expected unqualified-id before '<' token /build/cmsbld/jenkins-workarea/workspace/ib-any-integration/CMSSW_9_1_X_2017-03-30-2300/src/CondFormats/PCLConfig/plugins/AlignPCLThresholdsReader.cc:31:80: error: invalid use of incomplete type 'class edmtest::AlignPCLThresholdsReader' The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison not run due to Build errors (RelVals and Igprof tests were also skipped) |
ok, let's wait until #18137 is merged... |
- needed for unification of alignment producers - in addition: - `EventSetup` was nowhere used - in case of zero processed events it would be undefined
…neric `AlignmentProducerAsAnalyzer_cff`.
-1 Tested at: 76ed399 You can see the results of the tests here: I found follow errors while testing this PR Failed tests: AddOn
I found errors in the following addon tests: cmsDriver.py RelVal -s L1REPACK:Full2015Data --data --scenario=HeavyIons -n 10 --conditions auto:run2_hlt_HIon --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016,Run2_HI --magField 38T_PostLS1 --fileout file:RelVal_Raw_HIon_DATA.root --filein /store/hidata/HIRun2015/HIHardProbes/RAW-RECO/HighPtJet-PromptReco-v1/000/263/689/00000/1802CD9A-DDB8-E511-9CF9-02163E0138CA.root : FAILED - time: date Mon Apr 24 23:33:57 2017-date Mon Apr 24 23:25:00 2017 s - exit: 23552 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 76ed399 You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals AddOn
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log
I found errors in the following addon tests: cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_PRef --relval 9000,50 --datatier "RAW" --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2017 --magField 38T_PostLS1 --eventcontent RAW --fileout file:RelVal_Raw_PRef_DATA.root --filein /store/data/Run2016B/JetHT/RAW/v1/000/272/762/00000/C666CDE2-E013-E611-B15A-02163E011DBE.root : FAILED - time: date Tue Apr 25 08:25:44 2017-date Tue Apr 25 08:16:27 2017 s - exit: 23552 |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
+1 |
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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar |
Previously two parallel code infrastructures in place for offline and PCL alignment
AlignmentProducer
→edm::ESProducerLooper
PCLTrackerAlProducer
→edm::EDAnalyzer
(legacy!)Most of the code is just duplication with more than 1000 lines of redundant code, i.e. it is hard to maintain.
In addition, only the PCL code is routinely tested during CMSSW software integration in workflow 1001.
This PR unifies the two alignment producers by means of a common base class
AlignmentProducerBase
which provides the common implementation.AlignmentProducer
now inherits fromAlignmentProducerBase
andedm::ESProducerLooper
.PCLTrackerAlProducer
is renamed to the more genericAlignmentProducerAsAnalyzer
which now inherits fromAlignmentProducerBase
andedm::one::EDAnalyzer
.This means easier maintenance and multi-threading friendliness. The latter applies not only to the PCL workflow, but also the offline MillePede workflow which is migrated the
edm::one::EDAnalyzer
version of the alignment producer.As a bonus, the main part of the offline alignment code is now routinely exercised during integration tests.
This changes has also been presented at the last Tracker Alignment meeting.
This PR includes also some minor fixes regarding the transition to phase-1.