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

Remove BX comparisons for CLCT and LCT in CSC L1T DQM [11_3_X] #33381

Merged
merged 2 commits into from Apr 13, 2021
Merged

Remove BX comparisons for CLCT and LCT in CSC L1T DQM [11_3_X] #33381

merged 2 commits into from Apr 13, 2021

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Apr 9, 2021

PR description:

Remove the BX comparisons for CLCT and LCT in CSC L1T DQM. The data CLCTs and LCTs have their BX information truncated. I had previously added this PR #33335 to simulate this truncation in the emulator. Even with function, there will always be substantial differences because the BX's are defined differently in data and emulator. For that reason it is best to remove the BX comparisons for CLCT and LCT altogether. The ALCT BX is not truncated, so those comparisons still make sense.

PR validation:

Code compiles.

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

To be back-ported once #33357 is merged.

Before submitting your pull requests, make sure you followed this checklist:

@dildick
Copy link
Contributor Author

dildick commented Apr 9, 2021

@jfernan2 This simple PR should pass all tests. I checked it previously when I was working on #33335. I'm running all matrix tests and will post the output when they are done.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33381/21974

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

A new Pull Request was created by @dildick (Sven Dildick) for master.

It involves the following packages:

DQM/L1TMonitor

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@dildick dildick changed the title Fix BX for CLCT and LCT in CSC L1T DQM Fix BX for CLCT and LCT in CSC L1T DQM [11_3_X] Apr 9, 2021
@jfernan2
Copy link
Contributor

jfernan2 commented Apr 9, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-87b232/14131/summary.html
COMMIT: e993042
CMSSW: CMSSW_11_3_X_2021-04-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33381/14131/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2865506
  • DQMHistoTests: Total failures: 459
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2865025
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@dildick dildick changed the title Fix BX for CLCT and LCT in CSC L1T DQM [11_3_X] Use BXData for CLCT and LCT in CSC L1T DQM comparisons [11_3_X] Apr 9, 2021
@jfernan2
Copy link
Contributor

jfernan2 commented Apr 9, 2021

@dildick
Copy link
Contributor Author

dildick commented Apr 9, 2021

I don't see any "_data" histograms. Why is that?

In principle, the code will run fine on real data. But on MC relval, it won't make sense. Because one type of TPs will have their BXs compressed, whereas the other won't. An example where this happens:

Screen Shot 2021-04-09 at 8 59 30 AM

@dildick
Copy link
Contributor Author

dildick commented Apr 9, 2021

I suppose I could add a check if the module is running on real data or not. If data, use getBXData. If MC, use getBX. That would fix the failures.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33381/21994

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

Pull request #33381 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again.

@dildick dildick changed the title Use BXData for CLCT and LCT in CSC L1T DQM comparisons [11_3_X] Remove BX comparisons for CLCT and LCT CSC L1T DQM comparisons [11_3_X] Apr 12, 2021
@dildick dildick changed the title Remove BX comparisons for CLCT and LCT CSC L1T DQM comparisons [11_3_X] Remove BX comparisons for CLCT and LCT BX in CSC L1T DQM [11_3_X] Apr 12, 2021
@dildick dildick changed the title Remove BX comparisons for CLCT and LCT BX in CSC L1T DQM [11_3_X] Remove BX comparisons for CLCT and LCT in CSC L1T DQM [11_3_X] Apr 12, 2021
@dildick
Copy link
Contributor Author

dildick commented Apr 12, 2021

@jfernan2 It turns out that BX comparisons of the CLCT and LCT do not make much sense, even with the bit masking. To avoid confusion for shifters it is probably best to remove these plots from the DQM. The ALCT BX comparison does make sense, so that one is kept.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33381/22060

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #33381 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again.

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-87b232/14210/summary.html
COMMIT: 1e4945d
CMSSW: CMSSW_11_3_X_2021-04-12-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33381/14210/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2864426
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2864397
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -132.664 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): -2.412 KiB L1TEMU/L1TdeCSCTPG
  • DQMHistoSizes: changed ( 10024.0,... ): -7.236 KiB L1TEMU/L1TdeCSCTPG
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1

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

@dildick
Copy link
Contributor Author

dildick commented Apr 13, 2021

@jfernan2 Thanks! After this, I'll wait a bit with DQM dev until I have received feedback from this week's MWGR. And then perhaps 1 more PR to get it into good shape for Run-3

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 5980dee into cms-sw:master Apr 13, 2021
@dildick dildick deleted the from-CMSSW_11_3_X_2021-04-08-2300-get-clct-lct-bx-data branch April 15, 2021 01:46
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