Remove nearly all calls to Modifier.isChosen() from L1T configurations #25577

makortel · 2019-01-02T21:01:55Z

This PR includes a suggestion on how to modify bunch of L1T configuration files to remove nearly all calls to cms.Modifier.isChosen(). It includes also some "modernization" of the customization style on the modified parts.

Tested in 10_4_0_pre4 (full matrix runs), no changes expected.

…sen()

…alls to isChosen()

…Chosen()

…isChosen()

…sen()

cmsbuild · 2019-01-02T21:03:04Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-01-02T21:09:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25577/7815

cmsbuild · 2019-01-02T21:09:35Z

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

Configuration/StandardSequences
L1Trigger/Configuration
L1Trigger/L1TCalorimeter
L1Trigger/L1TCommon
L1Trigger/L1TGlobal
L1Trigger/L1TMuon
L1Trigger/L1TNtuples

@cmsbuild, @nsmith-, @rekovic, @franzoni, @thomreis, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@kreczko, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @thomreis, @dgulhan 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

makortel · 2019-01-02T21:11:53Z

@cmsbuild, please test

cmsbuild · 2019-01-02T21:12:16Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32398/console Started: 2019/01/02 22:13

makortel · 2019-01-02T21:13:45Z

Configuration/StandardSequences/python/SimL1EmulatorRepack_CalouGT_cff.py

@@ -11,118 +11,118 @@
 else:
    print("L1T INFO:  L1REPACK:CalouGT (intended for 2016/2017 data), reemulates the Calo part, uses unpacked Muons, and reemulates uGT.")


Are these printouts still needed (also in other SimL1EmulatorRepack_* files)? If no, I'd remove them, if yes, there is likely a (bit hacky) way to recast them as calls to Modifier.toModify().

These comments are useful to remind the user of intricacies of the expert level workflow. I'd like to keep them, please.

makortel · 2019-01-02T21:17:04Z

Is this customization file

cmssw/L1Trigger/Configuration/python/customise_ReEmulateL1_cff.py

Lines 8 to 12 in 315e069

    
           def getSubsystemsToEmulate(subsys): 
        
               if not stage2L1Trigger.isChosen(): 
        
                   if 'ECAL' in subsys: 
        
                       subsys.append('RCT')

still needed (git grep would indicate "no")? If not, I'd remove it, if yes, the customizations can likely be recast to calls to Modifier.toModify().

makortel · 2019-01-02T21:20:41Z

L1Trigger/L1TCalorimeter/python/simDigis_cff.py

+from L1Trigger.L1TCalorimeter.simCaloStage2Digis_cfi import simCaloStage2Digis
+stage2L1Trigger.toReplaceWith(SimL1TCalorimeter, cms.Sequence( simCaloStage2Layer1Digis + simCaloStage2Digis ))
+
+def _modifyStage2L1TriggerCaloParams(process):
    from CondCore.CondDB.CondDB_cfi import CondDB
    CondDB.connect = cms.string("frontier://FrontierProd/CMS_CONDITIONS")


This line modifies the global CondCore.CondDB.CondDB_cfi.CondDB object. It would be better to modify a local clone. I can do that either in this PR or separately.

OK. Either way fine with me.

@makortel so I suggest that we move forward with this PR and your suggestion is implemented in a further PR

cmsbuild · 2019-01-03T00:26:05Z

-1

Tested at: de3f27a

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

I found follow errors while testing this PR

Failed tests: AddOn

AddOn:

I found errors in the following addon tests:

cmsDriver.py TTbar_Tauola_8TeV_TuneCUETP8M1_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run1_mc_Fake --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --fileout file:RelVal_Raw_Fake_MC.root : FAILED - time: date Thu Jan 3 01:08:29 2019-date Thu Jan 3 00:52:40 2019 s - exit: 35584
cmsRun /cvmfs/cms-ib.cern.ch/week1/slc7_amd64_gcc700/cms/cmssw-patch/CMSSW_10_5_X_2019-01-02-1100/src/HLTrigger/Configuration/test/OnLine_HLT_Fake.py realData=False globalTag=@ inputFiles=@ : FAILED - time: date Thu Jan 3 01:08:29 2019-date Thu Jan 3 00:52:40 2019 s - exit: 21504
cmsDriver.py RelVal -s HLT:Fake,RAW2DIGI,L1Reco,RECO --mc --scenario=pp -n 10 --conditions auto:run1_mc_Fake --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --processName=HLTRECO --filein file:RelVal_Raw_Fake_MC.root --fileout file:RelVal_Raw_Fake_MC_HLT_RECO.root : FAILED - time: date Thu Jan 3 01:08:29 2019-date Thu Jan 3 00:52:40 2019 s - exit: 21504

cmsbuild · 2019-01-03T00:26:10Z

Comparison job queued.

cmsbuild · 2019-01-31T17:02:11Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/32934/console Started: 2019/01/31 18:03

cmsbuild · 2019-01-31T19:09:17Z

+1
Tested at: 3f63752
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25577/32934/summary.html

cmsbuild · 2019-01-31T19:09:22Z

Comparison job queued.

cmsbuild · 2019-01-31T20:28:57Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25577/32934/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 3097440
DQMHistoTests: Total failures: 148
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3097095
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 133 log files, 14 edm output root files, 32 DQM output files

rekovic · 2019-02-01T09:39:34Z

#25577 (comment)
#25577 (comment)

Addressed.
And , Yes, can remove L1Trigger/Configuration/python/customise_ReEmulateL1_cff.py in this PR. Thanks.

rekovic · 2019-02-01T09:48:42Z

Ok.
No RECO differences this time.
The DQM differences are from the known instability of Tau, that need to be addressed separately.

rekovic · 2019-02-01T09:48:46Z

+1

fabiocos · 2019-02-01T19:20:06Z

+operations

@makortel if you agree we may move forward with this PR and possible additional updates are done separately. Please let me know

cmsbuild · 2019-02-01T19:20:36Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

makortel · 2019-02-01T19:22:03Z

@fabiocos Sounds good, thanks.

fabiocos · 2019-02-04T21:18:07Z

+1

makortel added 8 commits December 21, 2018 22:19

Reorganize modifier usage in L1TRawToDigi_cff to remove call to isCho…

670d2dc

…sen()

Remove no-longer-needed copy

f4f16e9

Reorganize modifier usage in L1TDigiToRaw_cff to remove call to isCho…

dfcb2fc

…sen()

Reorganize modifier usage in L1 simulation configurations to reduce c…

2bd96ff

…alls to isChosen()

Reorganize modifier usage in customiseReEmul.py to remove calls to is…

1a9f5af

…Chosen()

Reorganize modifier usage in hackConditions_cff's to remove calls to …

a4fadad

…isChosen()

Reorganize modifier usage in reEmul.py to remove calls to isChosen()

4bb48da

Reorganize modifier usage in L1NtupleEMU_cff to remove calls to isCho…

de3f27a

…sen()

cmsbuild added this to the CMSSW_10_5_X milestone Jan 2, 2019

cmsbuild added code-checks-pending comparison-pending l1-pending operations-pending orp-pending pending-signatures tests-pending labels Jan 2, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jan 2, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 2, 2019

makortel commented Jan 2, 2019

View reviewed changes

cmsbuild added comparison-pending tests-pending and removed tests-approved labels Jan 31, 2019

cmsbuild added tests-started and removed tests-pending labels Jan 31, 2019

cmsbuild added tests-approved and removed tests-started labels Jan 31, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jan 31, 2019

cmsbuild added l1-approved and removed l1-pending labels Feb 1, 2019

cmsbuild added fully-signed operations-approved and removed operations-pending pending-signatures labels Feb 1, 2019

cmsbuild added orp-approved and removed orp-pending labels Feb 4, 2019

cmsbuild merged commit 06a3bfb into cms-sw:master Feb 4, 2019

makortel mentioned this pull request Feb 12, 2019

Remove last Modifier.isChosen() calls from L1T configuration #25924

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove nearly all calls to Modifier.isChosen() from L1T configurations #25577

Remove nearly all calls to Modifier.isChosen() from L1T configurations #25577

makortel commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

makortel commented Jan 2, 2019

cmsbuild commented Jan 2, 2019 •

edited

makortel Jan 2, 2019

rekovic Feb 1, 2019

makortel commented Jan 2, 2019

makortel Jan 2, 2019

rekovic Feb 1, 2019

fabiocos Feb 1, 2019

cmsbuild commented Jan 3, 2019

cmsbuild commented Jan 3, 2019

cmsbuild commented Jan 31, 2019 •

edited

cmsbuild commented Jan 31, 2019

cmsbuild commented Jan 31, 2019

cmsbuild commented Jan 31, 2019

rekovic commented Feb 1, 2019

rekovic commented Feb 1, 2019

rekovic commented Feb 1, 2019

fabiocos commented Feb 1, 2019

cmsbuild commented Feb 1, 2019

makortel commented Feb 1, 2019

fabiocos commented Feb 4, 2019

		@@ -11,118 +11,118 @@
		else:
		print("L1T INFO: L1REPACK:CalouGT (intended for 2016/2017 data), reemulates the Calo part, uses unpacked Muons, and reemulates uGT.")

Remove nearly all calls to Modifier.isChosen() from L1T configurations #25577

Remove nearly all calls to Modifier.isChosen() from L1T configurations #25577

Conversation

makortel commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

cmsbuild commented Jan 2, 2019

makortel commented Jan 2, 2019

cmsbuild commented Jan 2, 2019 • edited

makortel Jan 2, 2019

Choose a reason for hiding this comment

rekovic Feb 1, 2019

Choose a reason for hiding this comment

makortel commented Jan 2, 2019

makortel Jan 2, 2019

Choose a reason for hiding this comment

rekovic Feb 1, 2019

Choose a reason for hiding this comment

fabiocos Feb 1, 2019

Choose a reason for hiding this comment

cmsbuild commented Jan 3, 2019

cmsbuild commented Jan 3, 2019

cmsbuild commented Jan 31, 2019 • edited

cmsbuild commented Jan 31, 2019

cmsbuild commented Jan 31, 2019

cmsbuild commented Jan 31, 2019

rekovic commented Feb 1, 2019

rekovic commented Feb 1, 2019

rekovic commented Feb 1, 2019

fabiocos commented Feb 1, 2019

cmsbuild commented Feb 1, 2019

makortel commented Feb 1, 2019

fabiocos commented Feb 4, 2019

cmsbuild commented Jan 2, 2019 •

edited

cmsbuild commented Jan 31, 2019 •

edited