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

Created GlobalEvFOutputModule #34621

Merged
merged 5 commits into from Jul 27, 2021
Merged

Conversation

Dr15Jones
Copy link
Contributor

@Dr15Jones Dr15Jones commented Jul 26, 2021

PR description:

  • The GlobalEvFOutputModule creates the same file format as EvFOutputModule but allows concurrent processing of the different cmsRun streams. This allows substantially better thread scaling.
  • To make it easier to reason about the change, some code in EventFilter/Utilities had its const correctness improved.

PR validation:

Code compiles and a locally run test program runs fine.

No checks were made on the contents of the created file.

resolves cms-sw/framework-team#190

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34621/24205

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

  • EventFilter/Utilities (daq, reconstruction)

@perrotta, @emeschi, @cmsbuild, @slava77, @jpata, @smorovic can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

please test

@Dr15Jones
Copy link
Contributor Author

image

Here is the comparison results for event throughput as a function of number of threads (where number of concurrent streams == number of threads). The job was a combination of the data creation from

https://github.com/cms-sw/cmssw/blob/master/EventFilter/Utilities/test/startBU.py

with the path filtering and file writing from

https://github.com/cms-sw/cmssw/blob/master/EventFilter/Utilities/test/startFU.py

Different OutputModules were substituted for each run of the test.

  • The use of AsciiOutputModule shows the highest possible throughput scaling of the configuration as the module allows all streams to run independently and the module itself does nearly no work each event.
  • The job with the regular EvFOutputModule shows the least amount of scaling which is expected for a one:: module.
  • Two different implementation of GlobalEvFOutputModule are shown
    • the with locks variant used a blocking mutex at the point where the module needs to write to the output file
    • the external work variant uses the ExternalWork ability to run object serialization and compression in parallel and then use a edm::SerialTaskQueue to guarantee that only one stream at a time can write to the file. This variant requires more infrastructure but is non blocking and will scale better than the locking variant when writing is slow.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-32a7f8/17202/summary.html
COMMIT: 010333b
CMSSW: CMSSW_12_0_X_2021-07-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34621/17202/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 11601.011601.0_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt10+2021+SingleElectronPt10_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
  • 11602.011602.0_SingleElectronPt35+2021+SingleElectronPt35_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt35+2021+SingleElectronPt35_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
  • 11603.011603.0_SingleElectronPt1000+2021+SingleElectronPt1000_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA/step2_SingleElectronPt1000+2021+SingleElectronPt1000_pythia8_GenSimINPUT+Digi+Reco+HARVEST+ALCA.log
Expand to see more relval errors ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 3 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998564
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2998535
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 38 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: no differences found

@Dr15Jones
Copy link
Contributor Author

This code change can not be the cause of the PR test problems as the touched classes are not even USED in any RelVals.

@smorovic
Copy link
Contributor

+1

@slava77
Copy link
Contributor

slava77 commented Jul 26, 2021

+reconstruction

for #34621 010333b

  • code changes in EventFilter/Utilities are in files not related to reco
    • as we discussed with @jpata , it's worth to reconsider if this package still needs to be in reco category; it is in only via the TcdsRawToDigi
  • jenkins tests pass and comparisons with the baseline show no (relevant) differences

@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). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Jul 27, 2021

merge

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.

Add more parallelization in streamer output module
5 participants