Run3 sim83 Attempt to unify the file definitions between FullSim and FastSim #33233

bsunanda · 2021-03-19T23:55:57Z

PR description:

Attempt to unify the file definitions between FullSim and FastSim

PR validation:

Use the runTheMatrix test workflows

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

Nothing special

cmsbuild · 2021-03-20T00:01:34Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21677

This PR adds an extra 40KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21677/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21677/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-03-20T00:09:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21678

This PR adds an extra 40KB to repository

cmsbuild · 2021-03-20T00:09:24Z

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

FastSimulation/Calorimetry
Geometry/HcalSimData
SimG4CMS/Calo
SimG4Core/Application

@ssekmen, @Dr15Jones, @lveldere, @makortel, @cvuosalo, @civanch, @ianna, @mdhildreth, @cmsbuild, @sbein can you please review it and eventually sign? Thanks.
@makortel, @cvuosalo, @rovere, @matt-komm, @fabiocos, @slomeo 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

makortel · 2021-03-20T00:22:30Z

FastSimulation/Calorimetry/python/Calorimetry_cff.py

-            ProbMax          = cms.double(1.0),
-            CFibre           = cms.double(0.5),
-            OnlyLong          = cms.bool(True)
+            HFShowerBlock     = HFShowerBlock


This leads to "long distance interaction" that is very confusing in most cases. If anyone does e.g. g4SimHits.HFShower.HFShowerBlock.ProbMax = 0.1, that change will be visible here as well (because the HFShowerBlock PSet object is the same).

We typically advise against that behavior and do

Suggested change

HFShowerBlock = HFShowerBlock

HFShowerBlock = HFShowerBlock.clone()

instead.

If you want to be able to set value in HFShowerBlock PSet once and have the change to propagate to both here and g4SimHits, best would be to do

Suggested change

HFShowerBlock = HFShowerBlock

HFShowerBlock = cms.PSet(refToPSet_ = cms.string("HFShowerBlock"))

and make sure the top-level HFShowerBlock gets attached to cms.Process (as it current does). Then the intention is clear, and the connection is visible e.g. edmConfigDump. The C++ side doesn't need any change.

@bsunanda , let us do like this

cmsbuild · 2021-03-20T13:04:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21684

This PR adds an extra 40KB to repository

cmsbuild · 2021-03-20T13:05:19Z

Pull request #33233 was updated. @ssekmen, @Dr15Jones, @lveldere, @makortel, @cvuosalo, @civanch, @ianna, @mdhildreth, @cmsbuild, @sbein can you please check and sign again.

bsunanda · 2021-03-20T15:41:38Z

@cmsbuild Please test

cmsbuild · 2021-03-22T12:53:29Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33233/21698

This PR adds an extra 44KB to repository

cmsbuild · 2021-03-22T12:53:50Z

Pull request #33233 was updated. @ssekmen, @Dr15Jones, @lveldere, @makortel, @cvuosalo, @civanch, @ianna, @mdhildreth, @cmsbuild, @sbein can you please check and sign again.

bsunanda · 2021-03-22T13:05:14Z

@cmsbuild Please test

cmsbuild · 2021-03-22T15:37:07Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-34b748/13663/summary.html
COMMIT: 0de7d4d
CMSSW: CMSSW_11_3_X_2021-03-21-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33233/13663/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: 9 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2639935
DQMHistoTests: Total failures: 18
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2639894
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 36 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

cvuosalo · 2021-03-22T21:30:30Z

+1

civanch · 2021-03-23T06:28:35Z

@sbein , your sign should be first

sbein · 2021-03-23T09:03:05Z

+1

civanch · 2021-03-23T14:14:16Z

+1

cmsbuild · 2021-03-23T14:14:42Z

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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2021-03-23T14:28:39Z

+1

Sunanda added 2 commits March 19, 2021 23:38

Attempt to have common files

71fe0cd

Add one missing cff file

4f7cfcb

cmsbuild added this to the CMSSW_11_3_X milestone Mar 19, 2021

cmsbuild added code-checks-pending fastsim-pending geometry-pending orp-pending pending-signatures simulation-pending tests-pending labels Mar 19, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 20, 2021

Code Check

9286126

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 20, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 20, 2021

makortel reviewed Mar 20, 2021

View reviewed changes

As suggested by Matti

8df7fd7

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 20, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 20, 2021

cmsbuild removed the tests-pending label Mar 20, 2021

cmsbuild added fastsim-pending tests-pending labels Mar 22, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 22, 2021

cmsbuild added tests-started and removed tests-pending labels Mar 22, 2021

cmsbuild added tests-approved and removed tests-started labels Mar 22, 2021

cmsbuild added geometry-approved and removed geometry-pending labels Mar 22, 2021

cmsbuild added fastsim-approved and removed fastsim-pending labels Mar 23, 2021

cmsbuild added fully-signed simulation-approved and removed pending-signatures simulation-pending labels Mar 23, 2021

cmsbuild added orp-approved and removed orp-pending labels Mar 23, 2021

cmsbuild merged commit 28d9f7a into cms-sw:master Mar 23, 2021

cmsbuild mentioned this pull request Mar 23, 2021

Utilities/StaticAnalyzers: Update ESR::get report to remove incorrectly flagged functions from partial match #33251

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run3 sim83 Attempt to unify the file definitions between FullSim and FastSim #33233

Run3 sim83 Attempt to unify the file definitions between FullSim and FastSim #33233

bsunanda commented Mar 19, 2021

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

makortel Mar 20, 2021

civanch Mar 21, 2021

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

bsunanda commented Mar 20, 2021

cmsbuild commented Mar 22, 2021

cmsbuild commented Mar 22, 2021

bsunanda commented Mar 22, 2021

cmsbuild commented Mar 22, 2021

cvuosalo commented Mar 22, 2021

civanch commented Mar 23, 2021

sbein commented Mar 23, 2021

civanch commented Mar 23, 2021

cmsbuild commented Mar 23, 2021

silviodonato commented Mar 23, 2021

	HFShowerBlock = HFShowerBlock
	HFShowerBlock = HFShowerBlock.clone()

	HFShowerBlock = HFShowerBlock
	HFShowerBlock = cms.PSet(refToPSet_ = cms.string("HFShowerBlock"))

Run3 sim83 Attempt to unify the file definitions between FullSim and FastSim #33233

Run3 sim83 Attempt to unify the file definitions between FullSim and FastSim #33233

Conversation

bsunanda commented Mar 19, 2021

PR description:

PR validation:

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

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

makortel Mar 20, 2021

Choose a reason for hiding this comment

civanch Mar 21, 2021

Choose a reason for hiding this comment

cmsbuild commented Mar 20, 2021

cmsbuild commented Mar 20, 2021

bsunanda commented Mar 20, 2021

cmsbuild commented Mar 22, 2021

cmsbuild commented Mar 22, 2021

bsunanda commented Mar 22, 2021

cmsbuild commented Mar 22, 2021

Comparison Summary

cvuosalo commented Mar 22, 2021

civanch commented Mar 23, 2021

sbein commented Mar 23, 2021

civanch commented Mar 23, 2021

cmsbuild commented Mar 23, 2021

silviodonato commented Mar 23, 2021