Skip to content
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

Migrate to esConsumes HIP and MillePede Alignment Algorithms #33583

Merged
merged 6 commits into from
Jun 22, 2021

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Apr 30, 2021

PR description:

Part of #31061.
Its purpose is to migrate the core base classes of Alignment/CommonAlignmentAlgorithm and percolate it down to MillePedeAlignmentAlgorithm and HIPAlignmentAlgorithm.

PR validation:

It compiles and runs successfully wf. 1001.0 (it exercises the MillePede algo via the SiPixelAli PCL worfklow).
In addition run a SiPixelAli workflow with more statistics:

cmsDriver.py milleStep --data --conditions auto:run3_data_express --triggerResultsProcess RECO --scenario pp --datatier ALCARECO --eventcontent ALCARECO --era Run2_2018 -s ALCA:PromptCalibProdSiPixelAli --dasquery='file dataset=/StreamExpress/Run2018B-TkAlMinBias-Express-v1/ALCARECO run=317320' -n 10000 --processName=ReAlCA

followed by:

cmsDriver.py pedeStep --data --conditions auto:run3_data_express --era Run2_2018 --scenario pp -s ALCAHARVEST:SiPixelAli --filein file:PromptCalibProdSiPixelAli.root 

A more thorough validation from the Tracker Alignment group is available here.

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

This is not a backport, no backport is needed.
cc:
@antoniovagnerini @vbotta

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33583/22391

  • This PR adds an extra 100KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

Alignment/CommonAlignmentAlgorithm
Alignment/CommonAlignmentProducer
Alignment/HIPAlignmentAlgorithm
Alignment/MillePedeAlignmentAlgorithm
Alignment/MuonAlignmentAlgorithms
Alignment/SurveyAnalysis

@malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks.
@pakhotin, @adewit, @abbiendi, @jhgoh, @tocheng, @tlampen, @mmusich, @trocino 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

@mmusich
Copy link
Contributor Author

mmusich commented Apr 30, 2021

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb56e5/14733/summary.html
COMMIT: 4699ebe
CMSSW: CMSSW_12_0_X_2021-04-29-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33583/14733/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb56e5/14733/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2662646
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2662623
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33583/22401

  • This PR adds an extra 100KB to repository

@cmsbuild
Copy link
Contributor

Pull request #33583 was updated. @malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33583/22406

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

Pull request #33583 was updated. @malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please check and sign again.

@mmusich mmusich marked this pull request as ready for review June 16, 2021 13:16
@mmusich
Copy link
Contributor Author

mmusich commented Jun 16, 2021

@cmsbuild, please test

to refresh the tests.

@mmusich mmusich changed the title [DRAFT] Migrate to esConsumes HIP and MillePede Alignment Algorithms Migrate to esConsumes HIP and MillePede Alignment Algorithms Jun 16, 2021
@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb56e5/15998/summary.html
COMMIT: a7f2449
CMSSW: CMSSW_12_0_X_2021-06-16-0900/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33583/15998/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS Clang-Tidy warnings: There are Clang-Tidy warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb56e5/15998/llvm-analysis/cmsclangtidy.txt for details.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2862520
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2862497
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@yuanchao
Copy link
Contributor

+1

  • migrate to esComsumes (incomplete)
  • Alignment/CommonAlignmentAlgorithm to MillePedeAlignmentAlgorithm and HIPAlignmentAlgorithm
  • wf matrix tests and unit tests passed

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

qliphy commented Jun 22, 2021

+1

@smuzaffar
Copy link
Contributor

@mmusich , there is unit test failing in last night IB [a]. I do not know ifchange in this PR has caused that test to fail, can you please check? Thanks

[a] https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-06-22-2300/unitTestLogs/Alignment/TrackerAlignment#/12-12

' looking for alignment labels ''.
Finds tag HCAL : 1
%MSG-i HCAL:  (NoModuleName) 23-Jun-2021 08:40:36 CEST pre-events
CaloTowerTopologyEP::CaloTowerTopologyEP
%MSG
' looking for alignment labels ''.
%MSG-i Alignments:  (NoModuleName) FakeAlignmentProducer()  23-Jun-2021 08:40:37 CEST pre-events
Providing data with label 'fakeForIdeal'.
%MSG
GEMGeometryESModule::initailized with flags 1:0
%MSG-i SiPixelQualityESProducer::SiPixelQualityESProducer:  (NoModuleName)  23-Jun-2021 08:40:37 CEST pre-events

%MSG
%MSG-i SiStripBackPlaneCorrectionDepESProducer:  (NoModuleName)  23-Jun-2021 08:40:37 CEST pre-events
ctor
%MSG
%MSG-i SiStripLorentzAngleDepESProducer:  (NoModuleName)  23-Jun-2021 08:40:37 CEST pre-events
ctor
%MSG
%MSG-i SiStripQualityESProducer:  (NoModuleName) 23-Jun-2021 08:40:37 CEST  pre-events
ctor
%MSG
%MSG-i Geometry:  (NoModuleName) TrackerDigiGeometryESModule()  23-Jun-2021 08:40:37 CEST pre-events
Label 'idealForDigi' IGNORING alignment labels 'fakeForIdeal'.
%MSG
%MSG-i TRACKER:  (NoModuleName) 23-Jun-2021 08:40:38 CEST pre-events
TrackerTopologyEP::TrackerTopologyEP
%MSG
----- Begin Fatal Exception 23-Jun-2021 08:40:41 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing Looper: class=AlignmentProducer label='@main_looper'
Exception Message:
A module declared it consumes an EventSetup product after its constructor.
This must be done in the contructor
The product type was: TrackerTopology in record TrackerTopologyRcd
and ESInputTag was Module label:  Data label:
----- End Fatal Exception -------------------------------------------------

@mmusich
Copy link
Contributor Author

mmusich commented Jun 23, 2021

@smuzaffar it it likely that this PR caused the problem in the unit test.
Out of curiosity from where do you read the log you pasted? It's not displayed in the IB webpage

@smuzaffar
Copy link
Contributor

@mmusich , I ran the test manually. May be you can update the test to dump the log files at

https://github.com/cms-sw/cmssw/blob/master/Alignment/TrackerAlignment/test/pixelPositions.sh#L110
https://github.com/cms-sw/cmssw/blob/master/Alignment/TrackerAlignment/test/pixelPositions.sh#L146

so that one can see the contents in case of failures

@mmusich
Copy link
Contributor Author

mmusich commented Jun 23, 2021

unfortunately I don't see immediately what's the problem (and even how to start debugging it).
Perphaps @makortel has a good suggestion.

@makortel
Copy link
Contributor

I was able to reproduce, and am taking a look.

@makortel
Copy link
Contributor

The immediate problem is the order of base classes for the multiple inheritance of AlignmentProducer

class AlignmentProducer : public AlignmentProducerBase, public edm::ESProducerLooper {

The consumesCollector() is a function via the second, edm::ESProducerLooper(), and the edm::ConsumesCollector it returns is given to the constructor of the first, AlignmentProducerBase
AlignmentProducer::AlignmentProducer(const edm::ParameterSet &config)
: AlignmentProducerBase(config, consumesCollector()),

The ConsumesCollector holds a pointer to a base class of ESProducerLooper, edm::EDConsumerBase, that checks if the consumes registration call is in the constructor. But now the EDConsumerBase has not been constructed yet, we get undefined behavior that shows up as this exception being shown.

The fix would be to change the order of the base classes. I tried that, but then I get

----- Begin Fatal Exception 23-Jun-2021 16:20:44 CEST-----------------------
An exception of category 'MakeDataException' occurred while
   [0] Using EventSetup component TrackerTopologyEP/'trackerTopology' to make data TrackerTopology/'' in record TrackerTopologyRcd
Exception Message:
Error while making data "TrackerTopology" "" in Record TrackerTopologyRcd
----- End Fatal Exception -------------------------------------------------

followed by a segfault

#3  0x00007efed087459b in (anonymous namespace)::sig_dostack_then_abort (sig=<optimized out>) at .../CMSSW_12_0_X_2021-06-22-2300/src/FWCore/Services/plugins/InitRootHandlers.cc:539
#4  <signal handler called>
#5  0x00007efea9c21e84 in AlignmentParameterStore::restoreCachedTransformations(unsigned int const&) () from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-06-22-2300/lib/slc7_amd64_gcc900/libAlignmentCommonAlignmentAlgorithm.so
#6  0x00007efea95e1cf5 in MillePedeAlignmentAlgorithm::setParametersForRunRange(std::pair<unsigned int, unsigned int> const&) () from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc900/cms/cmssw/CMSSW_12_0_X_2021-06-22-2300/lib/slc7_amd64_gcc900/pluginAlignmentMillePedeAlignmentAlgorithmAuto.so
#7  0x00007efeaa09e6f7 in AlignmentProducerBase::storeAlignmentsToDB (this=0x7efeaccd72c0) at /cvmfs/cms-ib.cern.ch/nweek-02686/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/bits/unique_ptr.h:360
#8  0x00007efeaa09eef6 in AlignmentProducerBase::finish (this=this@entry=0x7efeaccd72c0) at .../CMSSW_12_0_X_2021-06-22-2300/src/Alignment/CommonAlignmentProducer/src/AlignmentProducerBase.cc:784
#9  0x00007efeaa110577 in AlignmentProducer::endOfJob (this=0x7efeaccd6e10) at .../CMSSW_12_0_X_2021-06-22-2300/src/Alignment/CommonAlignmentProducer/plugins/AlignmentProducer.cc:40
#10 0x00007efed8cb9bda in std::function<void ()>::operator()() const (this=this@entry=0x7ffefae2f290) at /cvmfs/cms-ib.cern.ch/nweek-02686/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/bits/std_function.h:683
#11 edm::ExceptionCollector::<lambda()>::operator() (__closure=<synthetic pointer>) at .../CMSSW_12_0_X_2021-06-22-2300/src/FWCore/Utilities/src/ExceptionCollector.cc:40
#12 edm::convertException::wrap<edm::ExceptionCollector::call(std::function<void()>)::<lambda()> > (iFunc=...) at .../CMSSW_12_0_X_2021-06-22-2300/src/FWCore/Utilities/interface/ConvertException.h:21
#13 edm::ExceptionCollector::call(std::function<void ()>) (this=this@entry=0x7ffefae2f2b0, f=...) at .../CMSSW_12_0_X_2021-06-22-2300/src/FWCore/Utilities/src/ExceptionCollector.cc:40
#14 0x00007efed905159c in edm::EventProcessor::endJob (this=0x7efed2163300) at /cvmfs/cms-ib.cern.ch/nweek-02686/slc7_amd64_gcc900/external/gcc/9.3.0/include/c++/9.3.0/bits/std_function.h:87
#15 0x000000000040b83c in (anonymous namespace)::EventProcessorWithSentry::~EventProcessorWithSentry() ()
#16 0x000000000040b649 in main ()

@makortel
Copy link
Contributor

I opened an issue #34223 to follow up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants