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

[HBHE] Always use 8 pulses in MAHI without 1-pulse to 8-pulse switch for Run3 and beyond #29617

Merged
merged 6 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 18 additions & 1 deletion HLTrigger/Configuration/python/customizeHLTforCMSSW.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ def synchronizeHCALHLTofflineRun3on2018data(process):
producer.setNoiseFlagsQIE8 = cms.bool( True )
producer.setPulseShapeFlagsQIE8 = cms.bool( True )

#----------------------------------------------------------
# Use 1+8p fit (PR29617) and apply HB- correction (PR26177)
for producer in producers_by_type(process, "HBHEPhase1Reconstructor"):
producer.algorithm.applyLegacyHBMCorrection = cms.bool( True )
producer.algorithm.chiSqSwitch = cms.double(15.0)

return process

def synchronizeHCALHLTofflineRun2(process):
Expand Down Expand Up @@ -166,7 +172,7 @@ def customiseFor2017DtUnpacking(process):

def customiseFor29512(process):
"""Refresh configuration of ElectronNHitSeedProducer instances.

Loops over some parameters of ElectronNHitSeedProducer instances and changes their type from string to ESInputTag.

For PR https://github.com/cms-sw/cmssw/pull/29512 "ElectronNHitSeedProducer modernization"
Expand Down Expand Up @@ -196,12 +202,23 @@ def customiseFor29658(process):

return process

# Use 8p fit (PR29617) and don't apply HB- correction (PR26177)
def customiseFor29617(process):
for producer in producers_by_type(process, "HBHEPhase1Reconstructor"):
producer.algorithm.applyLegacyHBMCorrection = cms.bool( False )
from Configuration.Eras.Modifier_run2_HE_2017_cff import run2_HE_2017
from Configuration.Eras.Modifier_run3_HB_cff import run3_HB
#--- >= Run3 modification:
(run3_HB & run2_HE_2017).toModify(producer.algorithm, chiSqSwitch = -1)
return process
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was puzzled at first by the change in the plots in the HI Run2 workflow
https://cmsweb.cern.ch/dqm/dev/start?runnr=1;dataset%3D/RelVal_wf158_0_pr/CMSSW_11_1_X-PRcmssw_29617-6107/DQMIO;referenceshow%3Dall;referencenorm=False;referenceobj1%3Dother::/RelVal_wf158_0_base/CMSSW_11_1_X-PRcmssw_29617-6107/DQMIO::;sampletype%3Doffline_relval;workspace%3DEverything;

The changes in this file are not here to patch the config to keep the HLT running.
Instead these are to change its default behavior. Isn't this a wrong way to do it?
If the goal is to change existing parameters in the existing tables, shouldn't there be a JIRA ticket instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slava77

  • synchronizeHCALHLTofflineRun3on2018data and synchronizeHCALHLTofflineRun2 are function to let now play and testing in old data/menu
  • customiseFor29617: is for the Run3 menu

For a modification in the confDB we usually propose a change in a customization function like this, then @Martin-Grunewald/TSG import the modification in the ConfDB in the Run3 menu and dump the menu the release. A JIRA is also needed to activate TSG. Will do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customiseFor29617: is for the Run3 menu

however, it apparently applies to the Run2 menu used in wf 158.0


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This subroutine should be called (see end of the file).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for comment. It's now called in customizeHLTforCMSSW.

# CMSSW version specific customizations
def customizeHLTforCMSSW(process, menuType="GRun"):

# add call to action function in proper order: newest last!
# process = customiseFor12718(process)
process = customiseFor29512(process)
process = customiseFor29658(process)
process = customiseFor29617(process)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like the menuType can be passed to the customiseForNNNN, which would apparently be the better solution in the context of not using eras

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember, this customisation code is just temporary. Eventually, we'll move the customisations directly into the menu in ConfDB and re-generate the HLT menu dumps.
The development menus in CMSSW_11_1 now must correspond to run3.
Workflow 158.0 is using HLT:HIon, so either it is updated to/replaced by a Run3 workflow (it is MC after all), OR it should be modified to use the HLT fake2 menu.


return process
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,8 @@
deltaChiSqThresh = cms.double(1e-3),
nnlsThresh = cms.double(1e-11)
)

from Configuration.Eras.Modifier_run2_HE_2017_cff import run2_HE_2017
from Configuration.Eras.Modifier_run3_HB_cff import run3_HB
#--- >= Run3 modification:
(run3_HB & run2_HE_2017).toModify(mahiParameters, chiSqSwitch = -1)