Skip to content

Conversation

@seungjin-yang
Copy link
Contributor

@seungjin-yang seungjin-yang commented Mar 8, 2022

PR description:

This PR adds DQM modules to monitor GE1/1 detection efficiency using GEMCSCSegment in the online DQM. It adds 36 MonitorElements for all eras and all scenarios in the online DQM. Please see this slides for more details.

PR validation:

This PR is tested with the muon gun simulation. Please check the above slides.

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

none

@seungjin-yang
Copy link
Contributor Author

@jshlee @quark2

@seungjin-yang
Copy link
Contributor Author

I checked the ORP meeting report and so I don't need a backport for April 8 collision runs, right?

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2022

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37178/28746

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2022

A new Pull Request was created by @seungjin-yang (Seungjin Yang) for master.

It involves the following packages:

  • DQM/GEM (dqm)
  • DQM/Integration (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@battibass, @threus, @batinkov, @watson-ij, @francescobrivio this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

jfernan2 commented Mar 8, 2022

please test

@jfernan2
Copy link
Contributor

jfernan2 commented Mar 8, 2022

@seungjin-yang Since this PR involves Online DQM, as usual we need a backport to 12_2_X to test it at P5
FYI @pmandrik

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2022

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-76b532/22948/summary.html
COMMIT: dbd376f
CMSSW: CMSSW_12_3_X_2022-03-08-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37178/22948/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-gem_dqm_sourceclient had ERRORS

Comparison Summary

Summary:

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

@seungjin-yang
Copy link
Contributor Author

I'm running the unittest locally and I'm able to report tomorrow.

@seungjin-yang
Copy link
Contributor Author

@seungjin-yang Since this PR involves Online DQM, as usual we need a backport to 12_2_X to test it at P5 FYI @pmandrik

Thanks @jfernan2, I will make a backport PR after fixing errors in the unittest.

@jfernan2
Copy link
Contributor

jfernan2 commented Apr 7, 2022

+1
@seungjin-yang @jshlee please also backport to 12_3_X

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 7, 2022

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

@perrotta
Copy link
Contributor

perrotta commented Apr 7, 2022

please test
(28 days old test must be refreshed before proceeding)

@perrotta
Copy link
Contributor

perrotta commented Apr 7, 2022

@seungjin-yang @jfernan2 could you please explain which is the difference between this master PR and the 12_2_X backport #37476 that you tested at P5 in order to approve this one? At least the number of modified lines of code is different...

@seungjin-yang
Copy link
Contributor Author

Hi @perrotta,
As mentioned in #37476 (comment), only the GEMEffByGEMCSCSegmentSource module has differences between the original PR and the 12_2_X backport. So, there are differences in the following two files:

  • DQM/GEM/plugins/GEMEffByGEMCSCSegmentSource.h
  • DQM/GEM/plugins/GEMEffByGEMCSCSegmentSource.cc

In the backport, GEMEffByGEMCSCSegmentSource is directly derived from DQMEDAnalyzer, not GEMOffflineDQMBase, and instead contains GEMOffflineDQMBase’s several methods.

So, here is a pseudo diff output for the header file between the original PR and the backport (diff OriginalPR Backport).

< class GEMEffByGEMCSCSegmentSource : public GEMOfflineDQMBase {
---
> class GEMEffByGEMCSCSegmentSource : public DQMEDAnalyzer {
>   // NOTE
>   using MEMap = std::map<GEMDetId, dqm::impl::MonitorElement*>;
>   bool hasMEKey(const MEMap&, const GEMDetId&);
>   void fillME(dqm::impl::MonitorElement*, const double);
>   void fillME(MEMap& me_map, const GEMDetId& key, const double);
>   void fillMEWithinLimits(dqm::impl::MonitorElement*, const double);
>   void fillMEWithinLimits(MEMap&, const GEMDetId&, const double);
>   template <typename T>
>   inline bool checkRefs(const std::vector<T*>&);
>   inline GEMDetId getReStLaKey(const GEMDetId&);
>   const double kEps_ = std::numeric_limits<double>::epsilon();

Also, diffstat says the 86 new lines were added and 10 lines deleted.

@seungjin-yang
Copy link
Contributor Author

I found that my PR is behind from master for DQM/Integration/python/clients/gem_dqm_sourceclient-live_cfg.py. #37101 has updated the config file but my one is outdated. Can you please let me know how to proceed?

#37101

- from Configuration.StandardSequences.Eras import eras
- process = cms.Process('GEMDQM', eras.Run3)
+ from Configuration.Eras.Era_Run3_cff import Run3
+ process = cms.Process('GEMDQM', Run3)

My PR

from Configuration.StandardSequences.Eras import eras
process = cms.Process('GEMDQM', eras.Run3)

@perrotta
Copy link
Contributor

perrotta commented Apr 7, 2022

@seungjin-yang thank you for having checked the compatibility with the HEADER
The mismatch that you are pointing out is in a part of the code (L4) which is untouched by your PR, and as your changes will not overwrite it. git is smart enough to recognize that you did not planned to touch those lines, and therefore it won't do that.
Of course it was good for you to check, just in case the updates in the master could have affected anyhow the part of the code modified by you: this is actually not the case here, and therefore we can proceed and merge it.

@seungjin-yang
Copy link
Contributor Author

@perrotta Thank you for your kind explanation!

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-76b532/23718/summary.html
COMMIT: 3e4c09b
CMSSW: CMSSW_12_4_X_2022-04-06-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37178/23718/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3593039
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3593009
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 200 log files, 45 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

perrotta commented Apr 8, 2022

+1

@cmsbuild cmsbuild merged commit af6eb52 into cms-sw:master Apr 8, 2022
cmsbuild added a commit that referenced this pull request Apr 8, 2022
…rom-CMSSW_12_3_X_2022-04-06-2300

Add GE1/1 detection efficiency monitor using GEMCSCSegment (backport of #37178, 12_3_X)
cmsbuild added a commit that referenced this pull request Apr 8, 2022
…rom-CMSSW_12_2_X_2022-04-03-2300

 Add GE1/1 detection efficiency monitor using GEMCSCSegment (backport of #37178, 12_2_X)
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.

6 participants