Add GEM to muon alignment module

[hyunyong]

PR description:

• Add GEM to AlignableMuon
• Other submodules (DB converters) will be done later
• To test and develop muon alignment include GEM, we need to update the MuonAlignment module first

PR validation:

• Muon scenario builder created the GEM SQLite DB file correctly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31634/18711

• This PR adds an extra 104KB to repository

[cmsbuild]

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

It involves the following packages:

Alignment/CommonAlignment
Alignment/CommonAlignmentMonitor
Alignment/CommonAlignmentProducer
Alignment/MuonAlignment
Alignment/SurveyAnalysis
Alignment/TrackerAlignment
Geometry/CommonTopologies

@civanch, @Dr15Jones, @makortel, @cvuosalo, @christopheralanwest, @ianna, @mdhildreth, @cmsbuild, @yuanchao, @tocheng, @tlampen, @pohsun can you please review it and eventually sign? Thanks.
@pakhotin, @fabiocos, @adewit, @makortel, @mtosi, @abbiendi, @JanFSchulte, @jhgoh, @VinInn, @tocheng, @tlampen, @mschrode, @mmusich, @trocino, @ebrondol 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

[cvuosalo]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-10-01-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/9692/console Started: 2020/10/01 20:40

[cmsbuild]

+1
Tested at: 83a8ead
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e5b29/9692/summary.html
CMSSW: CMSSW_11_2_X_2020-10-01-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e5b29/9692/summary.html

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 2542225
• DQMHistoTests: Total failures: 10
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2542193
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 149 log files, 22 edm output root files, 35 DQM output files

[cvuosalo]

@ianna Do you think the changes to GeometryAligner.h in this PR will slow performance unacceptably? The applyAlignments function is called by many of the geometry builders.

[cvuosalo]

ping @ianna Do you think the changes to GeometryAligner.h in this PR will slow performance unacceptably? There is a change from picking the element out of a sorted list to repeatedly searching the list for each desired element. The applyAlignments function is called by many of the geometry builders.

[cvuosalo]

@slava77 This PR is changing GeometryAligner::applyAlignments in Geometry/CommonTopologies/interface/GeometryAligner.h, which is used by the muon detectors, Tracker, and MTD. It is replacing sequential access to sorted elements with repeated searches through the elements (see changes to GeometryAligner.h. Do you think this change could cause significant performance costs?

[slava77]

enable profiling

in response to

@slava77 This PR is changing GeometryAligner::applyAlignments in Geometry/CommonTopologies/interface/GeometryAligner.h, which is used by the muon detectors, Tracker, and MTD. It is replacing sequential access to sorted elements with repeated searches through the elements (see changes to GeometryAligner.h. Do you think this change could cause significant performance costs?

[slava77]

@cmsbuild please test

to get the profiling results

[hyunyong]

@hyunyong @bsunanda Could the two of you please coordinate about creating a PR to sort the GEM geometry by raw ID?

I've tested an alignment DB file that created with an updated muon misalignment scenario builder.
It is working without any mismatched rawId problem now.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31634/21405

• This PR adds an extra 140KB to repository

• Found files with invalid states:

  • Alignment/MuonAlignment/test/Alignments.db:
    • Added: 67aeadb
    • Deleted: 278a7b3

[cmsbuild]

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

[cvuosalo]

@cmsbuild please test

[cvuosalo]

@cmsbuild please test

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e5b29/13346/summary.html
COMMIT: 278a7b3
CMSSW: CMSSW_11_3_X_2021-03-08-1100/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/31634/13346/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-7e5b29/34634.0_TTbar_14TeV+2026D76+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal
• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-7e5b29/34834.999_TTbar_14TeV+2026D76PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU

Summary:

• No significant changes to the logs found
• Reco comparison results: 3 differences found in the comparisons
• DQMHistoTests: Total files compared: 38
• DQMHistoTests: Total histograms compared: 2849195
• DQMHistoTests: Total failures: 8
• DQMHistoTests: Total nulls: 1
• DQMHistoTests: Total successes: 2849164
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.004 KiB( 37 files compared)
• DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
• Checked 160 log files, 37 edm output root files, 38 DQM output files

[cvuosalo]

@hyunyong The PR description says:
We need to change the applyAlignments function at GeometryAligner.h (The geometry map and alignment container detID order is not the same even after sorting. Changed the loop; geometry map -> alignment container.)

I think this statement is no longer true. If no change in applyAlignments is needed, please revise the PR description.

[hyunyong]

@hyunyong The PR description says:
We need to change the applyAlignments function at GeometryAligner.h (The geometry map and alignment container detID order is not the same even after sorting. Changed the loop; geometry map -> alignment container.)

I think this statement is no longer true. If no change in applyAlignments is needed, please revise the PR description.

Yes, I deleted that PR description.

[cvuosalo]

I think this PR no longer presents any issues for Geometry.

[hyunyong]

Is there more issue to merge?

[silviodonato]

Is there more issue to merge?

@cms-sw/alca-l2

[yuanchao]

+1
As previously mentioned, getting a record from EventSetup without token is deprecated. Though many parts of the codes have been rewritten to comply. Still several parts the deprecated code remain. (iSetup.get().get(gemGeometry);) As those involve with the alignment function calls, we don't insist to complete a full migration here. Just that the migration will be needed in the future and it is up to the core software manager to enforce the migration.

[cmsbuild]

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]

+1