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

reduce verbosity of reHLT customisation in SKIM step #40599

Merged
merged 1 commit into from
Jan 29, 2023

Conversation

missirol
Copy link
Contributor

PR description:

This PR is a follow-up of #40481, addressing #40481 (comment).

It reduces the verbosity of cmsDriver.py when the re-HLT customisation is applied to the SKIM step. In this case, it produces only a 1-line message, instead of 1 line for every affected Sequence of the SKIM step.

Merely technical. No changes expected, except for a reduction of printouts from cmsDriver.py.

PR validation:

Some basic manual checks with runTheMatrix.py and cmsDriver.py.

If this PR is a backport, please specify the original PR and why you need to backport that PR. If this PR will be backported, please specify to which release cycle the backport is meant for:

N/A

self.additionalCommands.append('process.%s.visit(ConfigBuilder.MassSearchReplaceProcessNameVisitor("%s", "%s", whitelist = ("subSystemFolder",)))'% (sequence,HLTprocess, proc))

self.additionalCommands.append(
'process.%s.visit(ConfigBuilder.MassSearchReplaceProcessNameVisitor("%s", "%s", whitelist = ("subSystemFolder",), verbose = %s))'% (sequence, HLTprocess, proc, verboseVisit))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a bit of text to (some of) the cfgs produced by cmsDriver.py. Specifically, it adds a , verbose = False in the reHLT-related customisations inside the cfg.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40599/33855

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Configuration/Applications (operations)

@cmsbuild, @perrotta, @rappoccio, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@missirol
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-208f86/30150/summary.html
COMMIT: b3800af
CMSSW: CMSSW_13_0_X_2023-01-24-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40599/30150/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: 11 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3555495
  • DQMHistoTests: Total failures: 9
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3555464
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 211 log files, 162 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

test parameters:

  • workflows = 4.17

@perrotta
Copy link
Contributor

please test

@missirol
Copy link
Contributor Author

hold

Still reviewing the outputs of the tests (specifically, the logs).

@cmsbuild
Copy link
Contributor

Pull request has been put on hold by @missirol
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@cmsbuild cmsbuild added the hold label Jan 25, 2023
@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-208f86/30161/summary.html
COMMIT: b3800af
CMSSW: CMSSW_13_0_X_2023-01-24-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40599/30161/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3613452
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3613425
  • DQMHistoTests: Total skipped: 24
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 216 log files, 166 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@missirol
Copy link
Contributor Author

There is something I don't understand about the logs from runTheMatrix.py. Take wf 136.731, step 3:

  1. latest tests of this PR
  2. baseline (from IB)
  3. latest tests of another, unrelated, PR

I don't understand why 1. does not include printouts from cmsDriver.py like Step: RECO Spec:. It might be due to this PR, but I do not see how.

@missirol
Copy link
Contributor Author

No significant changes to the logs found

@smuzaffar, I have a couple of questions. In this PR we expect a reduction in the logs of the matrix tests.

  • Does the bot only report increases in the logs, or also reductions? (here?)

  • In the outputs of PR tests (e.g. here), can one find some 'global' stats on the logs? (total number of lines, or similar)

@cmsbuild
Copy link
Contributor

Pull request #40599 was updated. @perrotta, @rappoccio, @fabiocos, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-208f86/30231/summary.html
COMMIT: a2127b3
CMSSW: CMSSW_13_0_X_2023-01-28-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40599/30231/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 9 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3613452
  • DQMHistoTests: Total failures: 9
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3613419
  • DQMHistoTests: Total skipped: 24
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 216 log files, 166 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@missirol
Copy link
Contributor Author

missirol commented Jan 29, 2023

The RelVals-INPUT error is unrelated (the tests passed in #40599 (comment), and there were really no changes since then).

No significant changes to the logs found

As noted in #40599 (comment), I'm not sure the bot reports reductions in the logs.

One can now see the reduction in the logs due to this PR, e.g. this vs baseline.

I don't have other updates planned for this PR.

@perrotta
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 (but tests are reportedly failing).

@perrotta
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 562a8e8 into cms-sw:master Jan 29, 2023
@missirol missirol deleted the devel_lessVerboseReHLTOfSkim branch January 29, 2023 15:11
@smuzaffar
Copy link
Contributor

smuzaffar commented Jan 29, 2023

  • Does the bot only report increases in the logs, or also reductions? (here?)

@missirol , yes bot was only reporting the increase in logs output but I now have updated it to report any reduction too. From the comparison tests log I see that 1286 log lines were removed.

Logs: -1286 False 216 299 0
Events: True 166 0.0 49 [] []
SUMMARY No significant changes to the logs found
  • In the outputs of PR tests (e.g. here), can one find some 'global' stats on the logs? (total number of lines, or similar)

you can find some information by going to Comparison with the baseline and then look into validateJR/logRootQA-events.log file

@missirol
Copy link
Contributor Author

great, thanks @smuzaffar !

@smuzaffar
Copy link
Contributor

There is something I don't understand about the logs from runTheMatrix.py. Take wf 136.731, step 3:

  1. latest tests of this PR
  2. baseline (from IB)
  3. latest tests of another, unrelated, PR

I don't understand why 1. does not include printouts from cmsDriver.py like Step: RECO Spec:. It might be due to this PR, but I do not see how.

I was also looking in to it and now after reading this comment I think the problem is that bot might have set PYTHONUNBUFFERED only for IB tests. This explains why we see extra printouts in IB logs and not in PR logs. I will update bot to set PYTHONUNBUFFERED for PR too.

@missirol
Copy link
Contributor Author

missirol commented Feb 1, 2023

Thanks for having a look. Indeed, that's the only explaination I could think of, but I could not see where PYTHONUNBUFFERED is set for the IB tests. Out of curiosity, where is that done?

@smuzaffar
Copy link
Contributor

all IB tests use https://github.com/cms-sw/cms-bot/blob/master/run-ib-testbase.sh to create dev area and this scripts sets export PYTHONUNBUFFERED=1

@smuzaffar
Copy link
Contributor

by the way, bot pr cms-sw/cms-bot#1927 should fix this for PR tests too

@missirol
Copy link
Contributor Author

missirol commented Feb 1, 2023

Sounds good. Thanks for the info!

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.

4 participants