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

GE21-ME21 segment reconstruction #39441

Merged

Conversation

slowmoyang
Copy link
Contributor

@slowmoyang slowmoyang commented Sep 19, 2022

PR description:

  • This PR implements the GE21-ME21 segment reconstruction in the same way as the GE11-ME11 segment reconstruction.
  • Add GEMCSCSegmentProducer::fillDescription
  • Add GEMCSCCoincidenceRateAnalyzer for the GEM-CSC coincidence rate study
  • Add runGEMCSCCoincidenceRateAnalyzer_cfg.py and plotGEMCSCCoincidenceRate.py to validate GEMCSCSegment reconstruction codes. The instruction is shown in the PR validation part.

PR validation:

Because GEMCSCSegmentProducer is not included in the standard reconstruction sequence, this PR cannot be validated by runTheMatrix.py.
Therefore, wf 42021.0 was modified and used for the validation. The result was presented in the GEM DPG meeting.

via online DQM unittest

Also, because this PR touches DQM/GEM, this PR was valiated with the online DQM unit test. Logs and output files are here.

cmsRun ${CMSSW_BASE}/src/DQM/Integration/python/clients/gem_dqm_sourceclient-live_cfg.py unitTest=True

via runGEMCSCCoincidenceRateAnalyzer_cfg.py and plotGEMCSCCoincidenceRate.py

This PR can be validated with runGEMCSCCoincidenceRateAnalyzer_cfg.py and plotGEMCSCCoincidenceRate.py. runGEMCSCCoincidenceRateAnalyzer_cfg.py takes RECO-tier files as input and then runs GEMCSCSegmentProducer and GEMCSCCoincidenceRateAnalyzer. GEMCSCCoincidenceRateAnalyzer produces a root file containing TTree. Then, plotGEMCSCCoincidenceRate.pytakes the resulting root file as input and produces efficiency plots.

Results can be found here

$ cmsRun runGEMCSCCoincidenceRateAnalyzer_cfg.py -o ./output.root
$ python3 plotGEMCSCCoincidenceRate.py -i ./output.root

The help message of runGEMCSCCoincidenceRateAnalyzer_cfg.py is as below. The era, global tag and geometry can be configured using arguments.

$ python3 runGEMCSCCoincidenceRateAnalyzer_cfg.py -h
sys.argv=['runGEMCSCCoincidenceRateAnalyzer_cfg.py', '-h']
['runGEMCSCCoincidenceRateAnalyzer_cfg.py'] are interpreted as arguments for cmsRun.
parsing argumnets from ['-h'].
usage: runGEMCSCCoincidenceRateAnalyzer_cfg.py [-h] [-e ERA] [-t GLOBAL_TAG] [-g GEOMETRY] [-m MAX_EVENTS] [-i INPUT_FILES [INPUT_FILES ...]] [-o OUTPUT_FILE]

optional arguments:
  -h, --help            show this help message and exit
  -e ERA, --era ERA     era (default: Phase2C17I13M9)
  -t GLOBAL_TAG, --global-tag GLOBAL_TAG
                        global tag (default: auto:phase2_realistic_T21)
  -g GEOMETRY, --geometry GEOMETRY
                        geometry (default: GeometryExtended2026D88Reco)
  -m MAX_EVENTS, --max-events MAX_EVENTS
                        max events (default: -1)
  -i INPUT_FILES [INPUT_FILES ...], --input-files INPUT_FILES [INPUT_FILES ...]
                        input files (default: ['file:/eos/cms/store/relval/CMSSW_12_6_0_pre2/RelValSingleMuPt1000/GEN-SIM-
                        RECO/125X_mcRun4_realistic_v2_2026D88noPU-v1/2580000/412ca5d0-4533-477d-ae92-411f5a532937.root', 'file:/eos/cms/store/relval/CMSSW_12_6_0_pre2/RelValSingleMuPt1000/GEN-SIM-
                        RECO/125X_mcRun4_realistic_v2_2026D88noPU-v1/2580000/817a15f8-1355-4606-a069-4e2d9e4f7939.root', 'file:/eos/cms/store/relval/CMSSW_12_6_0_pre2/RelValSingleMuPt1000/GEN-SIM-
                        RECO/125X_mcRun4_realistic_v2_2026D88noPU-v1/2580000/d1801dfb-b4e3-489b-b04c-5125cc35bc2e.root', 'file:/eos/cms/store/relval/CMSSW_12_6_0_pre2/RelValSingleMuPt1000/GEN-SIM-
                        RECO/125X_mcRun4_realistic_v2_2026D88noPU-v1/2580000/d21fa76d-6a29-4455-8476-cc04800a4b49.root', 'file:/eos/cms/store/relval/CMSSW_12_6_0_pre2/RelValSingleMuPt1000/GEN-SIM-
                        RECO/125X_mcRun4_realistic_v2_2026D88noPU-v1/2580000/f25378b1-c009-41b0-9a1b-1a043c4cc599.root'])
  -o OUTPUT_FILE, --output-file OUTPUT_FILE
                        output file (default: output.root)

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

This PR is not a backport but will be backported.

- Implement the GE21-ME21 segment reconstruction the same way as the GE11-ME11
segment reconstruction.
- Add `fillDescription` to `GEMCSCSegmentProducer`.
- Add `GEMCSCCoincidenceRateAnalyzer` for GEM-CSC coincidence rate study
@slowmoyang
Copy link
Contributor Author

@jshlee

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39441/32148

  • This PR adds an extra 28KB to repository

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39441/32152

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slowmoyang (slowmoyang) for master.

It involves the following packages:

  • DQM/GEM (dqm)
  • DQM/Integration (dqm)
  • RecoLocalMuon/GEMCSCSegment (upgrade, reconstruction)

@micsucmed, @emanueleusai, @ahmad3213, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @clacaputo, @syuvivida, @pmandrik, @mandrenguyen, @rvenditti can you please review it and eventually sign? Thanks.
@batinkov, @bellan, @battibass, @watson-ij, @missirol, @jhgoh, @trtomei, @threus, @beaucero, @francescobrivio this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@mandrenguyen
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-774acf/27649/summary.html
COMMIT: 4b893a2
CMSSW: CMSSW_12_6_X_2022-09-18-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39441/27649/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3621956
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3621929
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@mandrenguyen
Copy link
Contributor

Hi @slowmoyang can you provide a recipe to test this PR?
I'm not even finding a relval wf with the number 42021.0 in 12_6_0_pre2.
Why do we need to have GEMCSCCoincidenceRateAnalyzer in cmssw instead of some private repo?
Is there a reason to put it in test instead of plugins?

@slowmoyang
Copy link
Contributor Author

slowmoyang commented Sep 20, 2022

Hi @mandrenguyen,

I tested this PR using cfg files here The step1-3 cfg files are the output of wf 42021.0 with a bit modification (removing redundant steps like PAT and extending output contents). Then, step4_GEMCSCSegmentReco.py calls GEMRecHitProducer with ge21Off of false (ge21Off is true in the standard sequence) and GEMCSCSegmentProducer. Finally, step5_Analysis.py calls GEMCSCCoincidenceRateAnalyzer, which produces TTree using TFileService.

cmsrel CMSSW_12_6_0_pre2
cd CMSSW_12_6_0_pre2/src
cmsenv
git-cms-merge-topic slowmoyang:ge21-me21-seg-reco__from-CMSSW_12_6_0_pre2
scram b
curl -OJ https://raw.githubusercontent.com/slowmoyang/GEM-Workspace/main/SW-Dev/220722_GE21-ME21-Segment-Reco/test/CMSSW_12_6_0_pre2/TenMu_phase2_realistic_T21/step1_GEN_SIM.py
curl -OJ https://raw.githubusercontent.com/slowmoyang/GEM-Workspace/main/SW-Dev/220722_GE21-ME21-Segment-Reco/test/CMSSW_12_6_0_pre2/TenMu_phase2_realistic_T21/step2_DIGI_L1TrackTrigger_L1_DIGI2RAW_HLT.py
curl -OJ https://raw.githubusercontent.com/slowmoyang/GEM-Workspace/main/SW-Dev/220722_GE21-ME21-Segment-Reco/test/CMSSW_12_6_0_pre2/TenMu_phase2_realistic_T21/step3_RAW2DIGI_RECO.py
curl -OJ https://raw.githubusercontent.com/slowmoyang/GEM-Workspace/main/SW-Dev/220722_GE21-ME21-Segment-Reco/test/CMSSW_12_6_0_pre2/TenMu_phase2_realistic_T21/step4_GEMCSCSegmentRECO.py
curl -OJ https://raw.githubusercontent.com/slowmoyang/GEM-Workspace/main/SW-Dev/220722_GE21-ME21-Segment-Reco/test/CMSSW_12_6_0_pre2/TenMu_phase2_realistic_T21/step5_Analysis.py
cmsRun step1_GEN_SIM.py
cmsRun step2_DIGI_L1TrackTrigger_L1_DIGI2RAW_HLT.py
cmsRun step3_RAW2DIGI_RECO.py
cmsRun step4_GEMCSCSegmentRECO.py
cmsRun step5_Analysis.py

First of all, I'm okay to drop RecoLocalMuon/GEMCSCSegment/test/GEMCSCCoincidenceAnalyzer.cc because it is not used in the unit test. Anyways, I added GEMCSCCoincidenceRateAnalyezr to replace RecoLocalMuon/GEMCSCSegment/test/TestGEMCSCSegmentAnalyzer.cc. TestGEMCSCSegmentAnalyzer seems to be used for GEMCSCSegmentProducer validation and produces more than 200 histograms only for GE11-ME11 segments. It is straightforward to modify TestGEMCSCSegmentAnalyzer for GE21-ME21 segments. But I think it is more useful to produce TTree instead of histograms for validating GEMCSCSegmentProducer, although the current GEMCSCCoincidenceAnalyzer does not cover all output of TestGEMCSCSegmentAnalyzer.

@slowmoyang
Copy link
Contributor Author

Also, wf42021.0 exists in the upgrade set.

$ runTheMatrix.py --what upgrade --showMatrix | grep '42021\.0'
42021.0 2026D94PU+TenMuE_0_200_pythia8_GenSimHLBeamSpot+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU

@mandrenguyen
Copy link
Contributor

I'm still trying to understand why this isn't tested in a relval workflow. Is there a plan to introduce GEMCSCSegmentProducer into the standard reco sequence?

@slowmoyang
Copy link
Contributor Author

slowmoyang commented Nov 10, 2022

@perrotta

if this new feature is not testable with standard workflows (also data workflows) why was it ported to online DQM?

GEMCSCSegment is being used in the online DQM for monitoring GEM efficiency, and this PR implements GE21-ME21 segment reconstruction for upcoming GE21 monitoring.

could you please confirm that the tests run fine even after last changes (modifier, etc) and provide a recipe to replicate them?
Because I removed files used in the original test, I tested current version of codes with RelVal data. Then, results looks consistent.

I added scripts for validation to this PR. The instruction can be found in the PR description.
Since data files used in the original test have been removed, I tested the current version of codes using a relval dataset. The new validation results are consistent with the original one.

could you please edit the PR description and update it with all latest developments?

I updated it.

@cmsbuild
Copy link
Contributor

@emanueleusai
Copy link
Member

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-774acf/28979/summary.html
COMMIT: 447d039
CMSSW: CMSSW_12_6_X_2022-11-11-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39441/28979/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

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

@mandrenguyen
Copy link
Contributor

+1
resign

@emanueleusai
Copy link
Member

+1

  • resign

@srimanob
Copy link
Contributor

+Upgrade
re-sign

@perrotta
Copy link
Contributor

+1

  • PR tests are a bit old, but this PR shouldn't touch anything in the normal workflows

@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 be automatically merged.

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.

None yet

6 participants