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

PPS: DQM update #28742

Merged
merged 3 commits into from Jan 16, 2020
Merged

PPS: DQM update #28742

merged 3 commits into from Jan 16, 2020

Conversation

jan-kaspar
Copy link
Contributor

@jan-kaspar jan-kaspar commented Jan 15, 2020

PR description:

  • better structure of PPS DQM sequences
  • introduced flag to enable/disable creation of proton-reconstruction plots
  • adjusted range of proton time histogram
  • strip-RP plots removed from the online and offline sequences since these RPs will not be used in Run 3 (the plots still remain in the "calibration" sequence for special runs)

PR validation:

Running

runTheMatrix.py --ibeos -l 136.731,136.7611,136.788,136.8311,136.85

gave

5 5 5 3 tests passed, 0 0 0 0 failed

This PR doesn't change the results of the reconstruction - see the plot below - blue = before the PR, red = with this PR.

make_cmp

@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-28742/13373

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jan-kaspar for master.

It involves the following packages:

DQM/CTPPS
DQM/Integration
DQMOffline/Configuration

@andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@forthommel, @threus, @batinkov, @rociovilar this is something you requested to watch as well.
@davidlange6, @silviodonato, @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 Jan 16, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4242/console Started: 2020/01/16 09:54

@jfernan2
Copy link
Contributor

@jan-kaspar since these changes affect Online DQM which is not tested in githug, have you tested the PR in a Online DQM configuration?
runTheMatrix does NOT check Online DQM in any workflow
Thanks

@jan-kaspar
Copy link
Contributor Author

@jan-kaspar since these changes affect Online DQM which is not tested in githug, have you tested the PR in a Online DQM configuration?
runTheMatrix does NOT check Online DQM in any workflow

Sort of. I tried directly by putting True here:
https://github.com/CTPPS/cmssw/blob/pps_dqm_update/DQM/Integration/python/clients/ctpps_dqm_sourceclient-live_cfg.py#L6
but it didn't work out of the box: there was a "list index out of range" when quering DAS, then PPSTimingCalibrationRcd was not found (so I had to use a different GT) ... so I had to modify the config a little to get it work. For sure, the reorganisation of the PPS DQM sequences introduced in this PR doesn't seem to pose any problem.

@cmsbuild
Copy link
Contributor

+1
Tested at: 8426162
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-57d4b1/4242/summary.html
CMSSW: CMSSW_11_1_X_2020-01-15-2300
SCRAM_ARCH: slc7_amd64_gcc820

@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-57d4b1/4242/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2697090
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 2
  • DQMHistoTests: Total successes: 2696741
  • DQMHistoTests: Total skipped: 346
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -24571.02 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -1636.734 KiB CTPPS/TrackingStrip
  • DQMHistoSizes: changed ( 10024.0,... ): -1.334 KiB CTPPS/DAQ
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@jfernan2
Copy link
Contributor

@jan-kaspar despite you made makeProtonRecoPlots = True in DQMOffline, you are removing 1k plots, can you please check?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_1_X_2020-01-15-2300+57d4b1/34765/dqm-histo-comparison-summary.html
They are mostly empty but due to stats and type of dataset if understood correctly from previous PRs...

@jan-kaspar
Copy link
Contributor Author

@jan-kaspar despite you made makeProtonRecoPlots = True in DQMOffline, you are removing 1k plots, can you please check?

This PR removes the plots from tracking-strip RPs as these will not be used in Run 3 (in normal runs). I checked your link and this is exactly what happens for WFs 136.731 and 136.788.

I realise that I didn't mention this in the PR description, will fix it.

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

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 3290d00 into cms-sw:master Jan 16, 2020
@jan-kaspar jan-kaspar deleted the pps_dqm_update branch January 24, 2020 17:23
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