L1T EMTF fix for pT assignment configuration bug in 2016 and 2017

[abrinke1]

Small fix in EMTF config file to enable configuration of pT assignment by global "Era". Fixes issue where all EMTF muons were being assigned pT = 0 in UL2016 RelVals.

Also suppress a spurious warning which is generated as a result of using the correct PtAssignmentEngine.

@davignon , @BenjaminRS , @rekovic , @jiafulow

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29080/13965

• This PR adds an extra 20KB 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-29080/13965/code-format.patch
  e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29080/13965/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29080/13967

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @abrinke1 for master.

It involves the following packages:

L1Trigger/L1TMuonEndCap

@cmsbuild, @rekovic, @benkrikler can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @thomreis this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[rekovic]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4967/console Started: 2020/03/03 16:36

[srimanob]

@abrinke1 @rekovic
Thanks for this PR. I have few comments on it:

1. Do you have any plots to show that this fix is good?
2. I understand from your side that you want to make fix also 2017. Should we really push the fix of 2017 back to 10_6? I mean if someone in the future use the new release to simulate any signals, the trigger result of 2017 will not consistent with main backgrounds or any samples generated centrally.

Thx.

[cmsbuild]

+1
Tested at: 64e9f11
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7a3086/4967/summary.html
CMSSW: CMSSW_11_1_X_2020-03-03-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[srimanob]

Hi,
Would it be possible for a short update at PPD General, so that plots can go public? Thanks.

Regarding the code backport, my concern is that we change the object definition which we don't do for production release except in newer versions of MiniAOD and NanoAOD that needs for production. Like what we propose to other groups, they can do backport on code which changes efficiency/performance for their studies, but it needs to be under customization. So, by default, you should get the same definition of objects. However, now it is more complicated because we are also talking about GT. So we can have situations where

• old release + old GT
• old release + new GT (e.g. people do private signal using old release but use new GT that also update for MC MiniAOD)
• new release (in case no customization) + new GT

Can we have, i.e. customization (or new era on top of 2017, 2018) that triggers both python setting, code and specific tag to replace one in GT? So, we don't need to worry in code and GT in the same time, and user always get what they should get. It's my quick idea, I don't know if this is possible. Maybe Alca can comment if we choose to go this way.

Here is an example: --era Run2_2017, Run2_fixL1EmuLegacy
this will call all you need.

May I propose we go for 2016 first as an urgent need, and rediscuss on 2017 and 2018 parts later?
However, this part
"pt_assign_engine_->get_pt_lut_version() > 5"
should be customization/era-aware.

@silviodonato @abrinke1 @rekovic @eyigitba @davignon @BenjaminRS

[abrinke1]

Hi @srimanob , @davignon , @BenjaminRS , all,

I think there are a few misunderstandings I can clear up, and a few additional questions I have to understand Phat's concerns. I'm not familiar enough with the sequences you're talking about to know where problems from inconsistencies would arise, for example.

1. None of these changes would have any impact on an AOD, MiniAOD, or NanoAOD sequence. The EMTF emulator can only run over RAW. So changes will only occur in sequences where the L1T is emulated from RAW data/MC. We don't even have a "packer", so running on any other level of data is simply impossible.

2. "Can we have, i.e. customization (or new era on top of 2017, 2018) that triggers both python setting, code and specific tag to replace one in GT?" @rekovic , @hyejinkwon , @davignon , and/or @BenjaminRS might have to weigh in on this. To me it sounds like a hack that we might regret in the future, as it breaks the current (barely-understood) division of labor between python settings, "Era", and GT.

3. I'm perfectly happy to go with 2016 as the urgent need, and re-discuss 2017 and 2018 later. Also, I'd note the PR under discussion has no impact on 2017 or 2018 emulation. So back-porting (or not) won't affect any future re-emulation of 2017 or 2018. The thing that will affect their re-emulation (though much less dramatically than this PR affects 2016) is using the correct GT payloads for 2017 and 2018. This is clearly a separate question, though.

4. The "pt_assign_engine_->get_pt_lut_version() > 5" does not really need to be Era-aware. LUT version 5 corresponds to PtAssignmentEngine2016, and LUT versions 6 and 7 correspond to PtAssignmentEngine2017. No other LUT versions exist, and future (higher-number) LUTs will either use PtAssignmentEngine2017, or PtAssignmentEngine2021 (or some such), which would need to be added in the future.

A note on these "Engines": a question of the following sort has come up a few times, but I'll quote Olivier: "What will happen in >=2021 when the engine changes again?
Would this require an update of the python to add a line for 2018? Will this be transparent in the future?"

The answer is, this will not be transparent, and will require code changes. But there is a good reason for this: the firmware for this future configuration (which would presumably include GEM, for example) does not yet exist, and the emulation for this future firmware also doesn't exist. So when we add this new functionality, we'll need to update the emulator. When we do this, the "default" Era will presumably go to "Run3_2021", and we'll need to add a new switch for "Run3_2018". But as long as we're using "Era" at all, I don't see a way around this: we can't very well have the default era be "PhaseII_2035" and have switches already for all the non-existent firmware/emulator versions in between.

If one or more of you would like to chat over Skpe to sort this out more efficiently "in person" (virtually), I would be happy to accommodate: my Skype name is awbrinks.

Best regards,
Andrew

[jiafulow]

Regarding "Can we have, i.e. customization (or new era on top of 2017, 2018) that triggers both python setting, code and specific tag to replace one in GT ..." I think I kind of understand what Phat is getting at. He's saying that (please correct me if I'm wrong), for MC, the "era" alone should be enough to fully specify the behavior of the EMTF emulator. And I think I agree with him. Given the era parameter, there is no ambiguity in the rest of the settings (for MC), users should get what they expect to get. However, that means we don't even need GT? But we still need to supply the correct "EventSetup"?

In any case, I think this is meant for the long-term solution, it shouldn't delay this particular PR. Any suggestions for how to implement a proper long-term solution are very much welcome.

Best regards,
Jia Fu

[srimanob]

@abrinke1 @jiafulow Thanks to following up. Here is my idea, and clarification,

• Let's go with 2016 first. So you may need to add comments back to

from Configuration.Eras.Modifier_stage2L1Trigger_2017_cff import stage2L1Trigger_2017
#stage2L1Trigger_2017.toModify(simEmtfDigis, RPCEnable = cms.bool(True), Era = cms.string('Run2_2017'))	stage2L1Trigger_2017.toModify(simEmtfDigis, RPCEnable = cms.bool(True), Era = cms.string('Run2_2017'))

• OK for LUT version, it seems just to avoid warning with 2016 engine, no effect on object performance

• For 2017, and 2018; my idea is as @jiafulow explains. I would like to avoid any object performance chances (even you call a bug) because production starts already. So, I propose not to touch anything (in code, config, GT) up to RECO step (so including L1 emulation) except you put new era/customization. So in future, with future CMSSW, GT:
  + With CMSSW_10_6_100 (whatever) + MC2017_GT: ==> You get the same object performances as in today production
  + With CMSSW_10_6_100 (whatever) + MC2017_GT + era (i.e. --era Run2_2017, Run2_fixL1EmuLegacy) ==> You get update on L1 object performance.

So all your updates tags do not need to go to GT, but in the CMSSW code and reload when the specific era is used. I propose this way because production starts, I would like to avoid to have a situation in future that

• "ah, our analysis sensitive to the L1 update, how I figure out that which L1 I use"
• Or someone who still use MC turn-on (with whatever reason) gets different turn on on simulation without any idea why because of release and GT.

[jiafulow]

If I understood correctly, for this PR, we should only fix Run2_2016, but not Run2_2017?

In the future, we should also fix Run2_2017 (and Run2_fixL1EmuLegacy), when the era parameter is specified. Would the fix be the same as these two lines (and same thing for Run2_fixL1EmuLegacy):

from Configuration.Eras.Modifier_stage2L1Trigger_2017_cff import stage2L1Trigger_2017
stage2L1Trigger_2017.toModify(simEmtfDigis, RPCEnable = cms.bool(True), Era = cms.string('Run2_2017'))

or do you have something different in mind?

When you said production has already started, are you referring to a specific production? Or all the productions that have been made to date? What are the rules of fixing a bug in the future CMSSW releases (not just CMSSW_10_6_X, but also CMSSW_11_X)?

[srimanob]

Hi @jiafulow
Correct, that is what I have in mind.

This PR should fix only what affects 2016, then backport can be done without any era modifier.

When we agree on how to fix 2017, we bring
from Configuration.Eras.Modifier_stage2L1Trigger_2017_cff ...
back under era modifier or customization (together with payload in GT, ... etc).

Regarding the production, I mean that we already have 3B MC samples of Legacy 2017 + ~2B of Legacy 2018. Bugfix at this point needs to be considered carefully:

• Can it be done at MiniAOD level ==> If yes, great. For example, we misse info on L1 unprefiring bit which can be added, or update on tau id.

• If not, how serious is it? Affect SF, JEC, Do we need to restart everything? Then we decide from that. In this L1 EMTF case, it should not affect SF/JEC but to avoid on mixing of object performances in analysis level, so I propose to go with era-modifier here. I think it's case-by-case.

Thx.

[srimanob]

Dear All, @abrinke1 @jiafulow @rekovic
Could you please update on this PR?

If we focus on 2016 only, I think it's straightforward to fix this PR (just by commenting on 2017 part).
Anything else? So we can test and discuss (if need) at the release meeting.

Thanks in advance.

FYI
@silviodonato @BenjaminRS @davignon

[abrinke1]

Hi @srimanob ,

Is the 2016 UL happening in a particular release? If so, I would prefer to port this PR into that release, and then comment-out the 2017 config lines, rather than commenting-out the 2017 lines in the "master" PR. But if you want us to just comment out the 2017 lines in this PR, I can do that as well.

Best,
Andrew

[srimanob]

Hi,

CMSSW_10_6_X is the branch that PR should go for UL. We can leave this PR as it is for now but the PR subject should be changed, as it's mentioned for 2016 only.

I propose to make a little cleaner but comment out 2017 for now, so this PR can backport will be exactly the same to handle 2016. Then they can be merged easily. You can have other PR for 2017 where backport will difference. This will be easier to manage, I think.

[abrinke1]

Chatting with @srimanob , we would like to continue with this PR into the master branch, so that all future versions of CMSSW will have the Era bug-fixes for both 2016 and 2017.

[silviodonato]

@benkrikler @rekovic do you have any objections?

[rekovic]

As long as PPD (@srimanob ) is fine with having different default behaviour with using Eras between 10_6 and 11_1, I am ok too.
So we abandon the idea of providing these changes via customizatons ?

[srimanob]

Hi,
To be clear, the customization idea is for 10_6 only, to avoid mixing of production. 11_1 is fine to have as it's, considered as bug-fix.

[rekovic]

+1

[cmsbuild]

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

[silviodonato]

+1