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

Fix the MadGraph5 LO multithread utility #34437

Merged
merged 2 commits into from Jul 16, 2021

Conversation

colizz
Copy link
Contributor

@colizz colizz commented Jul 10, 2021

PR description:

The MadGraph5 LO multithread feature is a part of the GEN multithread utilities (it is not used in official production).
In this PR we fix several spotted issues for the MG LO multithread feature:

A backport to 10_6_X is also needed.

PR validation:

In the recent GEN efforts to validate on multiple multithreading modules, this utility has been tested on the LHE step produced by MG5 LO, for ~100 recent UL16/17 processes on McM.
Physics results are consistent between the default generator configuration and with the MG5 LO multithread feature applied. Please see plots in the link. See here for the configuration to activate this utility.

- apply a patch to MadGraph5 for the "readonly" gridpack mode, identified in https://answers.launchpad.net/mg5amcnlo/+question/696856 ;
- do "restore_data" manually as required by the "readonly" mode;
- use a corrected LHE merging script.
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34437/23859

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @colizz (Congqiao Li) for master.

It involves the following packages:

  • GeneratorInterface/LHEInterface (generators)

@SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks.
@alberto-sanchez, @mkirsano 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

@agrohsje
Copy link

Isn't the patch version specific? Should we really have it here instead of genproduction? We could apply patching for existing gridpacks via checking script.

@colizz
Copy link
Contributor Author

colizz commented Jul 10, 2021

Isn't the patch version specific? Should we really have it here instead of genproduction? We could apply patching for existing gridpacks via checking script.

Yes we should put it in genproduction eventually. The reason we put a patch here is to benefit all existing gridpacks with MG>=2.6.1; and that MG before version 2.7.2 will have some specific patches applied (note that we haven't moved to 2.7 yet).
The patch is flexible to handle all past versions of our gridpack.
For the last item, which checking script do you mean for specific? Is it a tool for genproduction or cmssw?

@agrohsje
Copy link

We have some 2.7 samples for polarization. Are you saying the patch works for 2.6.1, 2.6.5 and 2.7.x? Then all ultra legacy cases would be covered.

@agrohsje
Copy link

please test

@colizz
Copy link
Contributor Author

colizz commented Jul 10, 2021

We have some 2.7 samples for polarization. Are you saying the patch works for 2.6.1, 2.6.5 and 2.7.x? Then all ultra legacy cases would be covered.

sorry we need a small correction. The newly added patch is pretty new and will not be fixed until 2.9.x or 3.1.x, so I'll modify the if-else statement. Thanks for your reminder.
Then it works for all exisiting and future 2.6.1, 2.6.5, 2.7.x, and even higher version gridpacks.

When we officially move to MG 2.7.x or more, I'll implement in the genproduction repo the feature inside runcmsgrid_LO.sh and also add patches there. Then the implementation of this multithread method will become more straightforward.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34437/23865

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

Pull request #34437 was updated. @SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again.

@qliphy
Copy link
Contributor

qliphy commented Jul 12, 2021

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f2cd04/16688/summary.html
COMMIT: eb18aa4
CMSSW: CMSSW_12_0_X_2021-07-11-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34437/16688/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: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2787742
  • DQMHistoTests: Total failures: 878
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2786863
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 37 files compared)
  • 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

@qliphy
Copy link
Contributor

qliphy commented Jul 15, 2021

There are many differences from DQM/TimerService which are known and fixed: #34445
Let's re-trigger the test.

@qliphy
Copy link
Contributor

qliphy commented Jul 15, 2021

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f2cd04/16837/summary.html
COMMIT: eb18aa4
CMSSW: CMSSW_12_0_X_2021-07-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34437/16837/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: 2786302
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2786273
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • 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

@qliphy
Copy link
Contributor

qliphy commented Jul 16, 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.

None yet

4 participants