-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Set default HI Stage1 parameters #12364
Set default HI Stage1 parameters #12364
Conversation
A new Pull Request was created by @richard-cms (R. Alex Barbieri) for CMSSW_7_5_X. Set default HI Stage1 parameters It involves the following packages: L1Trigger/L1TCalorimeter @cmsbuild, @mulhearn can you please review it and eventually sign? Thanks. |
Talking with Salvatore, the clear preference from Alca/DB is to wait for the GT before removing the HI customizations. The combined operation should have no effect. |
please test |
The tests are being triggered in jenkins. |
@davidlange6: I failed to appreciate that PR #12183 requires these parameter updates in order to do anything sensible for HI. In for a penny in for a pound? |
Yes, just to reiterate, without this PR the HI settings currently in the release are junk. We have been manually overriding the Stage1 settings for a long time in testing, and DQM has been doing the same. If this PR and #12366 are merged, the setting will remain junk but we will get the best possible settings as soon as the GT has been updated. |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
Comparison is ready The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons |
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
Set default HI Stage1 parameters
@davidlange6: thanks! |
if hasattr(process,'caloStage1Params'): | ||
process.caloStage1Params.jetSeedThreshold = cms.double(0.) | ||
process.caloStage1Params.regionPUSType = cms.string("zeroWall") | ||
process.load('L1Trigger.L1TCalorimeter.caloStage1Params_HI_cfi') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richard-cms are these essential for prompt/express reco?
If so, it should be uncommented in Configuration/DataProcessing/python/RecoTLR.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To my knowledge this settings are not used during RECO (we don't re-emulate L1 except during DQM and MC generation).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, also, anything using online tags will be fine due to O2O, which should be up in time...
@mulhearn @mandrenguyen
This PR sets the default Stage1 Level-1 trigger parameters that we expect to use during data taking. It is meant to be merged before we start the HI MC campaign, but is not critical for data taking.
However, since we expect to get a new GT with a proper payload soon, I will also make a PR which removes the override so that we will properly pull from the GT.
Personally, I think that we should merge both PRs at once and just make the new GT quickly, because the current customization settings are junk (before this PR) and then we will get the new conditions right when the new GT shows up without having to wait for yet another PR merge. Others can comment on whether this is a good idea or not.