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

Adapt uGMT DQM for new readout menu following the disabling of the zero suppression #38115

Merged
merged 6 commits into from
Jun 3, 2022

Conversation

dinyar
Copy link
Contributor

@dinyar dinyar commented May 29, 2022

PR description:

This PR updates the uGMT DQM following changes made to the readout menu after we disabled the zero suppression. It also includes some changes to add plots for the new hadronic showers.

In particular:

  • Removed zero suppression folder from uGMT DQM
  • Ignored BX range mismatches in uGMT muon copy comparisons. (We now read out only the central BX for the copies but +/-2 for the main outputs.)
  • Ignore BX range mismatches for the data-emulator comparisons for the same reason.
  • Move the main uGMT DQM plots behind the fat events filter as otherwise the BX distribution plots would be heavily biased towards the central BX. (Because only the central BX is read out in standard events for the inputs.)
  • Enable uGMT muon copy comparisons for all events (not just the validation events).
  • Remove alerting on incorrect zero suppression and disable BX range checks.
  • Add hadronic shower comparisons between the EMTF outputs and uGMT inputs
  • Add uGMT muon showers to DQM quality tests

PR validation:

Ran the usual runTheMatrix and scram checks.

dinyar added 4 commits May 28, 2022 19:58
The following changes were done to cope with the uGMT readout menu
following the removal of the zero suppression of the uGMT:
- Removed zero suppression folder from uGMT DQM
- Ignore BX range mismatches in uGMT muon copy comparisons as we read
out only the central BX for the copies but +/-2 for the main outputs.
- Enable uGMT muon copy comparisons for all events (not just the
validation events).
- Ignore BX range mismathes for the data-emulator comparisons for the
same reason.
- Move the main uGMT DQM plots behind the fat events filter as otherwise
the BX distribution plots would be heavily biased towards the central
BX. (Because only the central BX is read out in standard events for the
inputs.)
This enables comparisons of hadronic showers at the EMTF outputs with
the ones found at the uGMT inputs.
Remove alerting on incorrect zero suppression and disable BX range
checks.
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38115/30230

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dinyar (Dinyar Rabady) for master.

It involves the following packages:

  • DQM/L1TMonitor (dqm)
  • DQM/L1TMonitorClient (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@dinyar
Copy link
Contributor Author

dinyar commented May 29, 2022

attn @vukasinmilosevic for L1T DQM

@dinyar dinyar marked this pull request as draft May 29, 2022 20:21
@dinyar
Copy link
Contributor Author

dinyar commented May 29, 2022

Will have another look at this, might have removed the uGMT standard plots entirely by mistake..

@dinyar
Copy link
Contributor Author

dinyar commented May 29, 2022

So, it looks like those plots are just not generated because no events passed the fat events filter (see [1]). What's not clear to me is whether this is an issue e.g., for RelVals or other offline workflows? Removing the draft state to indicate that expert feedback would be useful.

-d

[1]

HLTPaths = ['HLT_L1FatEvents_v*', 'HLT_Physics_v*']

@dinyar dinyar marked this pull request as ready for review May 29, 2022 20:42
@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38115/30243

ERROR: Build errors found during clang-tidy run.

DQM/L1TMonitor/src/L1TStage2uGMTBxDistributions.cc:215:13: error: use of undeclared identifier 'trackFinderType' [clang-diagnostic-error]
        if (trackFinderType == l1t::omtf_neg) {
            ^
DQM/L1TMonitor/src/L1TStage2uGMTBxDistributions.cc:234:13: error: use of undeclared identifier 'trackFinderType' [clang-diagnostic-error]
        if (trackFinderType == l1t::emtf_neg) {
            ^
Suppressed 1642 warnings (1641 in non-user code, 1 with check filters).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38115/30251

  • This PR adds an extra 44KB to repository

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

@cmsbuild
Copy link
Contributor

Pull request #38115 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

@pmandrik
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7b2ba5/25124/summary.html
COMMIT: fbd9c63
CMSSW: CMSSW_12_5_X_2022-05-31-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38115/25124/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: 50
  • DQMHistoTests: Total histograms compared: 3647705
  • DQMHistoTests: Total failures: 86
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3647596
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1318.6840000000002 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -7.011 KiB L1T/L1TStage2uGMT
  • DQMHistoSizes: changed ( 11634.0,... ): 138.794 KiB L1T/L1TStage2uGMT
  • DQMHistoSizes: changed ( 11634.0,... ): 0.085 KiB L1TEMU/L1TdeStage2uGMT
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@pmandrik
Copy link
Contributor

pmandrik commented Jun 1, 2022

Hi, this one affect l1t_dqm_sourceclient-*_cfg.py clients. We need a backport to CMSSW_12_3_X to test it at P5

@dinyar
Copy link
Contributor Author

dinyar commented Jun 1, 2022

Hi @pmandrik,

Ok, done (#38172). Shall I also create one for 12_4_X as I'd like to have it deployed to P5?

Cheers,
Dinyar

@dinyar
Copy link
Contributor Author

dinyar commented Jun 1, 2022

Speculatively created also #38174.

@pmandrik
Copy link
Contributor

pmandrik commented Jun 1, 2022

Hi @pmandrik,

Ok, done (#38172). Shall I also create one for 12_4_X as I'd like to have it deployed to P5?

Cheers, Dinyar

Hi, at the moment CMSSW_12_3_4_patch2 is used at the DQM production machines, please check DQM playback and let me know if it is ready to be deployed at P5 for operation, the 12_4_X is probably also useful but not at the moment

@emanueleusai
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 2, 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)

@qliphy
Copy link
Contributor

qliphy commented Jun 2, 2022

Discussions are still ongoing in 12_3_X on P5 tests:
#38172

@qliphy
Copy link
Contributor

qliphy commented Jun 3, 2022

+1

@cmsbuild cmsbuild merged commit 9341d87 into cms-sw:master Jun 3, 2022
dinyar added a commit to dinyar/deployment that referenced this pull request Jun 7, 2022
These plots were removed from the DQM in
cms-sw/cmssw#38115.
muhammadimranfarooqi pushed a commit to dmwm/deployment that referenced this pull request Jun 9, 2022
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