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

Remove ESProducers for ECAL Mustache parameters from configuration - 11_2_X #33184

Conversation

thomreis
Copy link
Contributor

PR description:

Since the records should now be provided with the GT this PR removes the ESSource and ESProducers for EcalMustacheSCParameters and EcalSCDynamicDPhiParameters from the default configuration.

PR validation:

Passes the limited matrix tests.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Backport of #33183
This is needed for MC production with some special ECAL GTs.

…lSCDynamicDPhiParameters from cfi to use GT parameters instead.
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 15, 2021

A new Pull Request was created by @thomreis (Thomas Reis) for CMSSW_11_2_X.

It involves the following packages:

RecoEcal/EgammaClusterProducers

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @lecriste, @rchatter, @jainshilpi, @rovere, @lgray, @sobhatta, @thomreis, @afiqaize, @simonepigazzini, @argiro, @varuns23, @ram1123 this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

@thomreis if this PR behaves as the one submitted in the master, outputs for run1 and run2 workflows will get modified wrt to the ones in the baseline 11_2 release. This cannot be accepted, in principle, as it would affect run1 and run2 productions.

@silviodonato , I don't remember: does the "no change" rule apply for 11_2 production release? Or, since the productions with 11_2 are Run3 DPG oriented, we can relax that rules if it only cause changes in run1 and run2 workflows?

@perrotta
Copy link
Contributor

backport of #33183

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-47f80e/13546/summary.html
COMMIT: 1877ac3
CMSSW: CMSSW_11_2_X_2021-03-16-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33184/13546/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: 14366 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2527501
  • DQMHistoTests: Total failures: 8655
  • DQMHistoTests: Total nulls: 6
  • DQMHistoTests: Total successes: 2518818
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): 0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 136.731 ): -0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 151 log files, 37 edm output root files, 36 DQM output files
  • TriggerResults: found differences in 1 / 35 workflows

@silviodonato
Copy link
Contributor

If I understand correctly, looking at https://cms-pdmv.cern.ch/mcm/campaigns?cmssw_release=CMSSW_11_2_*&page=0&shown=63, no reco campaigns are ongoing in 11_2.

@silviodonato
Copy link
Contributor

assign alca
(after ORP meeting discussion)
@christopheralanwest could you comment on the differences observed? Thank you

@cmsbuild
Copy link
Contributor

New categories assigned: alca

@christopheralanwest,@tlampen,@pohsun,@yuanchao,@francescobrivio,@malbouis you have been requested to review this Pull request/Issue and eventually sign? Thanks

@thomreis
Copy link
Contributor Author

The WFs with differences are the same than for the 11_3 version. The WFs that now get the Run 2 parameters from the GT (instead of the previously hardcoded Run 3 parameters).

@yuanchao
Copy link
Contributor

+1

  • modification on python config only
  • Regardless the WF comparison fails, matrix tests and unit tests are all passed.

@perrotta
Copy link
Contributor

+1

  • Removing the usage of the ESProducers forces the usage of the parameters stored in the DB: they are otpimized separately for all eras, while with the ESProducers the run3 config was adopted also for Run1 and Run2.
  • This leads to changes in the reco outputs for the Run1 and Run2 workflows, which are understood and revert to the situation when their dedicated configs were still considered instead of the Run3 ones
  • Since "no reco campaigns are ongoing in 11_2", the no change rule for the production 11_2 release can be relaxed here,

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 6e91f3d into cms-sw:CMSSW_11_2_X Mar 19, 2021
@thomreis thomreis deleted the ecal_mustache_params_remove_esproducers_112x branch March 19, 2021 09:07
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

5 participants