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

update to TopMonitor plugin in DQMOffline/Trigger/ (+simple fixes to some DQM outputs) #28067

Merged
merged 14 commits into from Oct 2, 2019

Conversation

missirol
Copy link
Contributor

PR description:

This PR only affects the DQMOffline/Trigger package. It includes a couple of improvements to the source code, and a few simple fixes in the python configuration files (mainly, fixes to the paths of DQM outputs, or Harvesting inputs, to correctly produce the relevant plots).

The two main updates are in TriggerDQMBase.h (the fill method) and TopMonitor.cc (a simple generalization of the code to handle multiple InputTags for values of b-tagging discriminants).

Some changes are expected in the comparisons, mainly due to the addition of plots that were not getting produced before.

More detailed descriptions of the changes are provided at the bottom of this message.

PR validation:

Local tests suggest the changes work as expected (but only compared outputs with limited stats).

runTheMatrix.py: workflow 250202.181 ran successfully.

if this PR is a backport please specify the original PR:

Not a backport (and the PR itself is not planned to be backported).


  • TriggerDQMBase.h:

    • added ObjME::fill method in TriggerDQMBase,
      to fill denominator and numerator MEs with one call,
      in order to limit the repetition of code
      (for filling denominator and numerator plots) in the plugins;
      see TopMonitor::analyze() for an example of how this is applied.
  • TopMonitor.cc:

    • removed hard-coded reference to "pfDeepCSVJetTags",
      and replaced members btagalgo and bbtagalgo (two InputTags) with btagAlgos (VInputTag):
      the b-tag discriminator of a jet is now computed as the sum of the values from btagAlgos
      (use case: sum outputs of a multi-class discriminator, like DeepCSV).
      The logic (i.e. summing the values) is the same as before, only the implementation is a bit more generic.
      The default values in fillDescriptions have been updated (to DeepCSV).

    • the change from btagalgo (InputTag) to btagAlgos (VInputTag) required
      to update the syntax in other configuration files that use the TopMonitor plugin
      (these are the only changes to HiggsMonitoring_cf*, MssmHbbMonitoring_cf* and SusyMonitor_cfi)

    • removed internal modification of enableMETPlot flag based on value of metSelection string

    • other small updates (mostly polishing code, e.g. removing unused data members)

  • TopMonitor_cfi.py:

    • removed explicit types for the parameters of cloned module hltTOPmonitoring
      (no changes to the actual values)
  • TopMonitoring_Client_cff.py:

    • small fixes to plot labels (small change which unfortunately results in large diffs)
  • SusyMonitoring_Client_cff.py:

    • fixed values of subDirs parameters -> efficiency plots are now produced
      (-> difference expected in the comparisons: additional plots)

    • added module triple_muon_dca_mupt_efficiency to the Harvesting sequence
      (the module existed already, it just wasn't included in the sequence)
      (-> difference expected in the comparisons: additional plots)

  • SusyMonitoring_cff.py:

    • fixed typos in paths to DQM outputs, to make them consistent with Harvesting inputs defined in SusyMonitoring_Client_cff
      (this renaming will create differences in the comparisons,
      and now the new folders will contain the efficiency plots from the Harvesting step)
  • BTVHLTOfflineSource_cfi.py:

    • fixed path name -> two new sub-folders will now be created under HLT/BTV/
      (-> difference expected in the comparisons: additional plots)
  • BTVHLTOfflineSource_cff.py:

    • removed, and moved these sequences (unmodified) to BTaggingMonitoring_cff to simplify
  • BTaggingMonitoring_Client_cff.py:

    • fix in BTVEfficiency_TurnOnCurves, efficiency plots will now be produced
      (-> difference expected in the comparisons: additional plots)
  • HEP17Monitoring_Client_cff.py:

    • fixed path of Harvesting inputs of hep17Efficiency
      (-> difference expected in the comparisons: additional plots in HLT/EGM/HEP17/)
  • TrackingMonitoring_Client_cff.py:

    • fixed path to Harvesting inputs of trackingForElectronsEffFromHitPatternHLT,
      but the client itself is not included in any sequence
      (so no changes in the outputs are expected from this)

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28067/12023

  • This PR adds an extra 216KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @missirol (Marino Missiroli) for master.

It involves the following packages:

DQMOffline/Trigger

@andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@battibass, @trocino, @jhgoh, @calderona, @HuguesBrun, @mtosi, @folguera, @rociovilar this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 25, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2677/console Started: 2019/09/25 17:51

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-238b96/2677/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 14 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2957392
  • DQMHistoTests: Total failures: 129
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2956922
  • DQMHistoTests: Total skipped: 341
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 2339.292 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 10042.0,... ): 262.245 KiB HLT/SUSY
  • DQMHistoSizes: changed ( 12434.0,... ): 112.137 KiB HLT/BTV
  • DQMHistoSizes: changed ( 12434.0,... ): 107.720 KiB HLT/EGM
  • DQMHistoSizes: changed ( 1000.0 ): 63.863 KiB HLT/EGM
  • Checked 145 log files, 15 edm output root files, 34 DQM output files

@jfernan2
Copy link
Contributor

@missirol the PR is adding many histograms in some dedicated workflows, but some onf them are empty (SUSY folder mainly).
Can you check that the desired output is produced across all workflows?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_0_X_2019-09-24-2300+238b96/33600/dqm-histo-comparison-summary.html

@missirol
Copy link
Contributor Author

Hi @jfernan2 , as far as I can see, the diffs in the comparisons are consistent with the changes in the PR (and the output of my local tests).

In my understanding, the SUSY/ plots are added for the workflows where the corresponding DQM/Harvesting sequences are included (and not, for example, when DQM:@standardDQMFakeHLT is used), for example wf 10824; in addition, for the 2021 workflows (example: wf 11634), additional plots in EGM/ are created as well (see fix in this commit) because the HLT Menu is not the Fake one (and these plots are automatically generated checking the content of the menu, so they don't get added in 2018 workflows, since the HLT Menu there is now the Fake one).

I realized the additional plots (in SUSY/ and EGM/) are mostly empty, and I haven't looked into the reason why yet; it is now on my to-do list. In general, I plan to add to the existing HLT Offline-DQM plugins (wherever possible) the switch to not produce plots when the trigger paths are incorrect (functionality introduced in #27772), and this should help limit the appearance of empty plots. One of the next steps is also the clean-up of unused folders/plugins, and we are trying to gather info from the different POG/PAG contacts in TSG about this.

If possible, I would like to keep these additional updates for the next PR; in this PR, I just tried to fix simple inconsistencies in the paths of the different DQM/Harvesting modules, found while going through the cff files.

@jfernan2
Copy link
Contributor

+1
Looking forward to additional updates including non empty plots in a next PR

@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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Oct 2, 2019

+1

@cmsbuild cmsbuild merged commit b3365e6 into cms-sw:master Oct 2, 2019
@missirol missirol deleted the devel_DQMOffline_Trigger_11_0_0_pre7 branch March 27, 2022 13:43
@srimanob srimanob mentioned this pull request Sep 18, 2023
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

4 participants