Reconfigure particle flow cfi #27782

hatakeyamak · 2019-08-15T13:35:47Z

PR description:

Reconfigure particleFlow_cfi and make clear the parameters used for PFEGammaFilters, PFMuonAlgo, i.e. different components of PFAlgo. Also used this opportunity to remove obsolete config parameters.

PR validation:

Compiles and run a few matrix tests with success (scenarios for 2017, 2018, and phase2)

if this PR is a backport please specify the original PR:

This is not a backport PR.

@bendavid @jpata @guitargeek

cmsbuild · 2019-08-15T13:36:15Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-15T13:42:06Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27782/11468

This PR adds an extra 72KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoParticleFlow/PFProducer/python/particleFlow_cfi.py modified in PR(s): Light by light modifier 1100 pre1 #27633

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-27782/11468/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27782/11468/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

slava77 · 2019-08-15T14:19:20Z

RecoParticleFlow/PFProducer/interface/PFAlgo.h

-                                   double minSignificanceReduction,
-                                   double maxDeltaPhiPt,
-                                   double minDeltaMet);
+				   const edm::ParameterSet& pfHFCleaningParams);


there is a preference to reduce dependence on ParameterSet this is going against that

Hmm. What is the reason behind reducing the dependence on ParameterSet?

This PFHFCleaningParameters and also PFBadHcalMitigationParameters are less important, and we don't have to introduce these PSets, but I think it's a good idea to isolate parameters for PFEGammaFilters and PFMuonAlgo, which I am hoping that EGamma and Muon POGs keep eyes on and manage (factorizing parameter sets to factorize responsibilities).

I'm using this #27010 (comment) as a reference.
Perhaps it does not fully apply in this case: there the tool is delivered via the event setup, while here the tool is created by the ED producer directly.

@Dr15Jones
is it worth to reduce dependence on the ParameterSet in the helper algorithms used by ED producers/analyzers?
The factory pattern [not the case here, but somewhat related] even enforces this dependence.

Okay. I perfer to leave the PR as it is, but let's see what @Dr15Jones would say. Thank you.

In general, the framework group encourages people to reduce dependencies on the ParameterSet class. It can make it harder to reuse code and the calls to getParameter always do parsing so are not super efficient.

cmsbuild · 2019-08-15T14:21:47Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-15T14:29:05Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27782/11469

This PR adds an extra 72KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoParticleFlow/PFProducer/python/particleFlow_cfi.py modified in PR(s): Light by light modifier 1100 pre1 #27633

cmsbuild · 2019-08-15T14:29:37Z

A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master.

It involves the following packages:

RecoParticleFlow/PFProducer

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @lgray, @seemasharmafnal, @bachtis, @cbernet this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

perrotta · 2019-08-16T05:43:08Z

please test

cmsbuild · 2019-08-16T05:43:39Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2050/console Started: 2019/08/16 10:08

cmsbuild · 2019-08-16T10:05:30Z

-1

Tested at: ccd41d9

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b790fa/2050/summary.html

I found follow errors while testing this PR

Failed tests: RelVals AddOn

RelVals:

When I ran the RelVals I found an error in the following workflows:
140.56 step2

runTheMatrix-results/140.56_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18/step2_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18.log

158.0 step2

runTheMatrix-results/158.0_HydjetQ_B12_5020GeV_2018_ppReco+HydjetQ_B12_5020GeV_2018_ppReco+DIGIHI2018PPRECO+RECOHI2018PPRECO+ALCARECOHI2018PPRECO+HARVESTHI2018PPRECO/step2_HydjetQ_B12_5020GeV_2018_ppReco+HydjetQ_B12_5020GeV_2018_ppReco+DIGIHI2018PPRECO+RECOHI2018PPRECO+ALCARECOHI2018PPRECO+HARVESTHI2018PPRECO.log

11634.0 step2

runTheMatrix-results/11634.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021/step2_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2021_GenSimFull+DigiFull_2021+RecoFull_2021+HARVESTFull_2021+ALCAFull_2021.log

12434.0 step2

runTheMatrix-results/12434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023_GenSimFull+DigiFull_2023+RecoFull_2023+HARVESTFull_2023+ALCAFull_2023/step2_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023_GenSimFull+DigiFull_2023+RecoFull_2023+HARVESTFull_2023+ALCAFull_2023.log

AddOn:

I found errors in the following addon tests:

cmsRun /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_11_0_X_2019-08-15-2300/src/HLTrigger/Configuration/test/OnLine_HLT_HIon.py realData=True globalTag=@ inputFiles=@ : FAILED - time: date Fri Aug 16 11:42:23 2019-date Fri Aug 16 11:41:11 2019 s - exit: 18688
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018_pp_on_AA --processName=HLTRECO --filein file:RelVal_Raw_HIon_DATA.root --fileout file:RelVal_Raw_HIon_DATA_HLT_RECO.root : FAILED - time: date Fri Aug 16 11:42:23 2019-date Fri Aug 16 11:41:11 2019 s - exit: 256
cmsRun /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_11_0_X_2019-08-15-2300/src/HLTrigger/Configuration/test/OnLine_HLT_GRun.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Fri Aug 16 11:48:35 2019-date Fri Aug 16 11:41:16 2019 s - exit: 18688
cmsDriver.py RelVal -s HLT:GRun,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run2_mc_GRun --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3 --processName=HLTRECO --filein file:RelVal_Raw_GRun_MC.root --fileout file:RelVal_Raw_GRun_MC_HLT_RECO.root : FAILED - time: date Fri Aug 16 11:48:35 2019-date Fri Aug 16 11:41:16 2019 s - exit: 18688
cmsRun /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_11_0_X_2019-08-15-2300/src/HLTrigger/Configuration/test/OnLine_HLT_HIon.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Fri Aug 16 11:49:50 2019-date Fri Aug 16 11:45:37 2019 s - exit: 18688
cmsDriver.py RelVal -s HLT:HIon,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run2_mc_HIon --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018_pp_on_AA --processName=HLTRECO --filein file:RelVal_Raw_HIon_MC.root --fileout file:RelVal_Raw_HIon_MC_HLT_RECO.root : FAILED - time: date Fri Aug 16 11:49:50 2019-date Fri Aug 16 11:45:37 2019 s - exit: 256
cmsRun /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_11_0_X_2019-08-15-2300/src/HLTrigger/Configuration/test/OnLine_HLT_GRun.py realData=True globalTag=@ inputFiles=@ : FAILED - time: date Fri Aug 16 11:51:55 2019-date Fri Aug 16 11:48:42 2019 s - exit: 18688
cmsDriver.py RelVal -s HLT:GRun,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_GRun --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2018 --processName=HLTRECO --filein file:RelVal_Raw_GRun_DATA.root --fileout file:RelVal_Raw_GRun_DATA_HLT_RECO.root : FAILED - time: date Fri Aug 16 11:51:55 2019-date Fri Aug 16 11:48:42 2019 s - exit: 18688

cmsbuild · 2019-08-16T10:05:33Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

slava77 · 2019-08-16T11:59:22Z

@hatakeyamak
can we introduce proper parameter set validation?
https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideConfigurationValidationAndHelp
This is a recommended way.
I see that this PR is introducing more "existsAs" cases, which are discouraged and may be OK if there is a good reason for an exception.

cmsbuild · 2019-10-03T14:19:22Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-10-03T14:26:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27782/12124

This PR adds an extra 208KB to repository
Found files with invalid states:
- MagneticField/GeomBuilder/plugins/dd4hep/MagGeoBuilder.cc:
  - Added: 957d80d, 6aa8543
  - Modified: e6d1e35, 9cc0070
  - Deleted: 7865712, b650320
- MagneticField/GeomBuilder/plugins/dd4hep/volumeHandle.h:
  - Added: 957d80d, 6aa8543
  - Deleted: 7865712, b650320
- MagneticField/GeomBuilder/plugins/dd4hep/VolBasedMagFieldESProducerNewDD.cc:
  - Added: 957d80d, 6aa8543
  - Modified: eaaed4b, 78d71af
  - Deleted: 7865712, b650320
- MagneticField/GeomBuilder/plugins/dd4hep/MagGeoBuilder.h:
  - Added: 957d80d, 6aa8543
  - Deleted: 7865712, b650320
- RecoEgamma/EgammaHLTProducers/interface/Poison.h:
  - Added: 327b1ac
  - Deleted: 2bae36b
- RecoEgamma/EgammaHLTProducers/src/Poison.cc:
  - Added: 327b1ac
  - Deleted: 2bae36b
- MagneticField/GeomBuilder/plugins/dd4hep/volumeHandle.cc:
  - Added: 957d80d, 6aa8543
  - Modified: eaaed4b, 78d71af
  - Deleted: 7865712, b650320

cmsbuild · 2019-10-03T14:27:37Z

Pull request #27782 was updated. @SiewYan, @andrius-k, @lveldere, @chayanit, @emeschi, @sbein, @schneiml, @ianna, @kpedro88, @Martin-Grunewald, @rekovic, @fioriNTU, @tlampen, @alberto-sanchez, @pohsun, @santocch, @peruzzim, @perrotta, @civanch, @zhenhu, @cmsbuild, @agrohsje, @fwyzard, @davidlange6, @smuzaffar, @Dr15Jones, @cvuosalo, @ssekmen, @mdhildreth, @jfernan2, @tocheng, @slava77, @ggovi, @qliphy, @fabiocos, @mkirsano, @benkrikler, @efeyazgan, @kmaeshima, @christopheralanwest, @pgunnell, @franzoni, @fgolf, @mommsen can you please check and sign again.

cmsbuild added this to the CMSSW_11_0_X milestone Aug 15, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 15, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Aug 15, 2019

slava77 reviewed Aug 15, 2019

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-rejected labels Aug 15, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 15, 2019

cmsbuild added tests-started and removed tests-pending labels Aug 16, 2019

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Aug 16, 2019

hatakeyamak closed this Oct 3, 2019

hatakeyamak mentioned this pull request Oct 3, 2019

Reconfigure particle flow config files #28110

Merged

cmsbuild removed the code-checks-pending label Oct 3, 2019

cmsbuild added the code-checks-approved label Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reconfigure particle flow cfi #27782

Reconfigure particle flow cfi #27782

hatakeyamak commented Aug 15, 2019 •

edited

Loading

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

slava77 Aug 15, 2019

hatakeyamak Aug 15, 2019 •

edited

Loading

slava77 Aug 15, 2019

hatakeyamak Aug 15, 2019 •

edited

Loading

Dr15Jones Aug 16, 2019

hatakeyamak Aug 16, 2019

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

perrotta commented Aug 16, 2019

cmsbuild commented Aug 16, 2019 •

edited

Loading

cmsbuild commented Aug 16, 2019

cmsbuild commented Aug 16, 2019

slava77 commented Aug 16, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

Reconfigure particle flow cfi #27782

Reconfigure particle flow cfi #27782

Conversation

hatakeyamak commented Aug 15, 2019 • edited Loading

PR description:

PR validation:

if this PR is a backport please specify the original PR:

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

slava77 Aug 15, 2019

Choose a reason for hiding this comment

hatakeyamak Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

slava77 Aug 15, 2019

Choose a reason for hiding this comment

hatakeyamak Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

Dr15Jones Aug 16, 2019

Choose a reason for hiding this comment

hatakeyamak Aug 16, 2019

Choose a reason for hiding this comment

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

cmsbuild commented Aug 15, 2019

perrotta commented Aug 16, 2019

cmsbuild commented Aug 16, 2019 • edited Loading

cmsbuild commented Aug 16, 2019

cmsbuild commented Aug 16, 2019

slava77 commented Aug 16, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

cmsbuild commented Oct 3, 2019

hatakeyamak commented Aug 15, 2019 •

edited

Loading

hatakeyamak Aug 15, 2019 •

edited

Loading

hatakeyamak Aug 15, 2019 •

edited

Loading

cmsbuild commented Aug 16, 2019 •

edited

Loading