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

First attempt to remove DQM HLT warnings from logs caused by HLT Fake menus #27467

Merged
merged 13 commits into from Jul 16, 2019

Conversation

jfernan2
Copy link
Contributor

@jfernan2 jfernan2 commented Jul 9, 2019

PR description:

This PR is a First attempt to remove DQM HLT warnings from logs caused by HLT Fake menus, flooding logs from RelVals not using GRun HLT menu (currently only Upgrade menus)

Basically I replaced every instance of @standardDQM+@ExtraHLT by @standardDQMFakeHLT in all Run1+Run2 relvals defined in Configuration/PyReleaseValidation/python/relval_steps.py

I left untouched "mother" steps definitions like *FullHEfail, HARVESTD or dataReco which may be used in the future as usual templates, and are in principle not used in current Relvals.

I am not sure if there exists a more clever way to do this.

I also removed a resilient HLT sequence in @standardDQMFakeHLT which was still causing messages in logs despite being warned a Fake HLT menu was being used.

This PR is connected to the recent change to Fake menus in 2018 Relvals #27331 and #21815
Besides issue #27372 should move PR tests to 2021 wf in CMSSW_11

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

It is NOT a backport, though I am not sure if it needs a backport

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27467/10795

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

A new Pull Request was created by @jfernan2 for master.

It involves the following packages:

Configuration/PyReleaseValidation
DQMOffline/Configuration
DQMOffline/Trigger

@cmsbuild, @andrius-k, @kmaeshima, @zhenhu, @schneiml, @prebello, @kpedro88, @pgunnell, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@battibass, @makortel, @threus, @jhgoh, @Martin-Grunewald, @calderona, @HuguesBrun, @mtosi, @trocino, @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 Author

jfernan2 commented Jul 9, 2019

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1378/console Started: 2019/07/09 20:08

@fabiocos
Copy link
Contributor

fabiocos commented Jul 9, 2019

@jfernan2 thank you for this attempt as discussed at last meeting. We have too many parallel developments pending on PyRelVal, this PR needs to be rebased after the integration of #27415

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 9, 2019

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

Comparison Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 56 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 2821103
  • DQMHistoTests: Total failures: 22
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2820747
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -635621.24 KiB( 32 files compared)
  • DQMHistoSizes: changed ( 136.85,... ): -12396.665 KiB HLT/JME
  • DQMHistoSizes: changed ( 136.85,... ): -10388.740 KiB HLT/SiStrip
  • DQMHistoSizes: changed ( 136.85,... ): -8697.605 KiB HLT/Pixel
  • DQMHistoSizes: changed ( 136.85,... ): -5951.869 KiB HLT/BTV
  • DQMHistoSizes: changed ( 136.85,... ): -5151.953 KiB HLT/EXO
  • DQMHistoSizes: changed ( 136.85,... ): -4788.865 KiB HLT/HIG
  • DQMHistoSizes: changed ( 136.85,... ): -4417.707 KiB HLT/TOP
  • DQMHistoSizes: changed ( 136.85,... ): -3675.566 KiB HLT/EGM
  • DQMHistoSizes: changed ( 136.85,... ): -3553.723 KiB HLT/SUSY
  • DQMHistoSizes: changed ( 136.85,... ): -3498.134 KiB HLT/Tracking
  • DQMHistoSizes: changed ( 136.85 ): ...
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@jfernan2
Copy link
Contributor Author

@fabiocos this PR is almost done. Still pending:

  • ecalDQM: ECAL team has promised to give an answer on it
  • HLTTauDQM: it is "hidden" somewhere into some deep validation which I was not able trace back
  • PV HLT: same as Tau
  • Some AlCaDQM in AlCa workflows.

I can dedicate some time next week to the last three ones and include them here or in another PR as you prefer.
Thanks

@fabiocos
Copy link
Contributor

@jfernan2 as said at the meeting, I believe that this PR as it is is already a good step forward, and fixes the part of the issues deriving from the change in the HLT menu. I would as to @prebello @zhenhu @kpedro88 to check it, and possibly merge it, adding further cleaning in a subsequent PR, which will probably need to address a different kind of issue.

@kpedro88
Copy link
Contributor

+upgrade

@prebello
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

@jfernan2 do you agree to merge this PR as it is?

@jfernan2
Copy link
Contributor Author

+1
yes of course

@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

+1

@cmsbuild cmsbuild merged commit 361a915 into cms-sw:master Jul 16, 2019
@fabiocos
Copy link
Contributor

@jfernan2 from the ongoing IB CMSSW_11_0_X_2019-07-16-2300 where this PR is merged it looks like it works properly for most workflows, but it causes a failure in HARVESTING where the @rerecoZeroBiasFakeHLT sequence is invoked. This is the recent update in #27415 by @mmusich .

@mmusich
Copy link
Contributor

mmusich commented Jul 17, 2019

@fabiocos I did not introduce any @rerecoZeroBiasFakeHLT in #27415

@mmusich
Copy link
Contributor

mmusich commented Jul 17, 2019

@jfernan2 failed to update the file DQMOffline/Configuration/python/DQMOffline_Certification_cff.py in order to apply this instruction https://github.com/cms-sw/cmssw/blob/master/DQMOffline/Configuration/python/DQMOffline_Certification_cff.py#L34

phase1Pixel.toReplaceWith(DQMCertCommon, DQMCertCommon.copyAndExclude([ # FIXME
    sipixelCertification # segfaults when included
]))

also to the newly created DQMCertCommonFakeHLT sequence.
It is somehow disappointing these changes were not fully tested.
Having said this I can provide a fix in a few minutes...

@fabiocos
Copy link
Contributor

@mmusich sorry, I was not clear, what I mean is that the failing histograms are those recently introduced in #27415 using @rerecoZeroBias , that were updated by @jfernan2 to the FakeHLT version

@mmusich
Copy link
Contributor

mmusich commented Jul 17, 2019

Having said this I can provide a fix in a few minutes...

@fabiocos @jfernan2 I offer a quick fix in #27540.

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

6 participants