-
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
Add PPS and EGM tags to dataRun3 GTs #37557
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37557/29289
|
A new Pull Request was created by @malbouis for master. It involves the following packages:
@cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild , please test |
Mh.. I guess I need a clarification on the EGM update (@cms-sw/egamma-pog-l2). This comment from @francescobrivio includes
The 'flag' refers to what is also mentioned in the summary slide of the EGM talk:
Assuming the flag is changed in the source code, what happens then for MC? Since the HLT config is unchanged, wouldn't we need the new EGM-HLT tags in the Run-3 MC GTs? Also, what about |
I will let @cms-sw/egamma-pog-l2 reply about the MC GTs as we have not been requested to update them. Concerning the offline data GTs, we thought they could be updated at a later stage, once the FTV is finished and the new payload validated. The same goes for the flag that is not set to In case you think we should be updating the offline GTs right away due to the HLT offline tests, please let us know. |
It's not clear to me why we are updating the HLT GTs if the new payload is not yet validated (the fast-track validation based on the HLT menu for Cosmics is unlikely to exercise this update of the EGM HLT supercluster regression tags). In any case, my understanding based on comments from others is that updating the tag without updating the flag in the source code leads to incorrect results, so I was expecting the changes to be done at the same time, as written in #37491 (review).
Regarding the (HLT tags in the) offline GTs, the same reasoning applies: the tags would have to change as soon as the source code is changed, if HLT is to produce the intended results. EGM experts might correct me. |
As of now, the GTs are updated with the new tag but that tag does not contain the new payload, so it is basically the "old" tag.
Indeed. The flag could be changed when the new payloads will be appended to the tags. As mentioned above, the new payload is not appended yet.
Understood, thanks. I will update the description of the PR to make it clear that the current EGM tags in the GTs do not yet have the new payloads. That will be appended once the FTV has validated it. And then, when doing the append, the Flag should be changed as well. |
Okay, thanks for the clarification; clearly I didn't notice that the payload was the same. Opening a short parenthesis for my own understanding: what's the advantage of updating/changing the tags first without changing the payloads? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-596180/23888/summary.html Comparison SummarySummary:
|
I would say it is so that we have autoCond.py in line with the data-taking GTs that will soon be used at P5. |
(sorry I'm travelling, and can engage in this properly only early next week.. )
yes we missed to request for this, my fault. Thanks Helena for clarifying. And Saumya will be requesting this soon. So HLT regression tags need to go to MC GT also, so that HLT can be properly simulated. I think I also saw a comment in this thread about HLT tags in offline data reco GT.. why is that needed? About the flag, we added a new flag in the .cc file which is set to False now, |
Hi @missirol just to add to Helena's comment here (#37557 (comment) ) what we did here is to introduce a multi-iov tag to which it is possible to append the new payload from a future run (from a run when the new HLT menu should be used) Regarding changing the code or changing the menu: arent we running HLT add-on tests (and relvals) on Run-2 data as well? If we change the code in cmssw then conditions for those past data should be updated too. Instead changing the HLT menu one would not care about the past and just have the current payloads used with the new code for the future HLT runs. |
@swagata87 , trying to clarify.
HLT tags can be consumed by HLT from the Offline GT in wfs where HLT and RECO run in the same job using the Offline GT. In fact (although it's technically no proof), the Offline GT contains the HLT-EGM regressions tags (looking for
That was my question, but Helena correctly pointed out that only the tag has changed, not the actual payload yet. Once the payload is changed, it's clear that the source code needs to be updated.
No preference from my side based on what I understood so far. |
@swagata87 so if these (new) regression tags will be also relevant for MC production, then it brings up again the point that the setting of the flag to true or false should be under some Run3 era modifier no? |
+alca
|
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
Thanks, Tamas. It's clearer now. Can you also clarify for me how the update of the payload will be sync'd with the update the source code? (is it "we make a new release with flag=true, we deploy it, and the new payload is picked from the first run that uses the new release"? or..)
Mh, not Run-2, but 2021 pilot-beam data (still older data, inevitably). For even-older data in RelVals, the HLT is 'Fake', so none of this applies.
I'm not sure I follow the distinction. In this case, changing a parameter in the HLT config is effectively the same as changing the parameter from fillDescriptions in the source code since that parameter affects only HLT. Aside from addOnTests/RelVals, we do routinely use the latest Run-3 HLT in the latest CMSSW release to estimate rates,etc on Run-2 (2018) data. In that case, we would currently pick up the Run-2 EGM tags and the flag in question would have to remain Maybe I'm missing something obvious to others. Apologies in advance. |
IIuc, the new flag only affects HLT (because another flag named |
Yes, what you wrote here sounds completely reasonable.
Right but that comment was about MC, like if we change the variable in the cpp code, then how will that affect Run-1 and Run-2 MC RelVals? Because IIUC changing the code to True we'd need to update the conditions, and w/o the era modifier, we'd need to update the MC conditions for Run-1/Run-2 while with the modifier, it's just the change of conditions for Run-3 (which we do have at hand -- thus the push for the modifier) |
Meaning wfs like (1) is important: it is the reason why I think that a modifier would not be needed in this specific case. When it comes to modifiers, RECO and HLT follow very different approaches; in short, HLT does not use them, because a given release only aims to support the 'latest' HLT menu (there is no attempt to maintain a Run2-like HLT centrally in 12_4_X, for example). Note that I'm not proposing nor opposing anything here. I'm just trying to figure out how this update unfolds and affects HLT. |
Thanks @missirol for the clarification. |
We are left with a question to EGM on how exactly TSG should deal with this update when using Run-2 data for measurements with the latest release and Run-3 HLT menu. The easiest/minimum would be for TSG to use a customisation that ensures the flag be set to Anyways, this can be clarified in the PR that will change the source code. Thanks for the clarifications. |
We would like to use Run3 HLT regression while re-running HLT on 2018 RAW data from now on. Another point is that the Run2 HLT regression was trained probably with 2015 or 2016 MC, as far as I know. So, while it was optimal for 2015/16 (may be even 2017) data-taking, it was not fully optimal for 2018 data-taking conditions. Run3 and 2018 conditions are more close to each other. Thus Run3 regression is rather appropriate for 2018. |
Then, I guess one solution for those measurements would be to overwrite the GT records similarly to how TSG is already testing the latest Run-3 HLT JECs on 2018 Data (example). There is a difference here with respect to the HLT-JEC case, though. For the HLT-JECs when running the latest Run-3 HLT on 2018 data, using the Run-3 or Run-2 ones is a bit of a choice, none of the two is wrong per se. In fact, that type of customisation is not in On the other hand, once these EGM updates are in place, if one simply follows the current TSG recipes (with |
+1
|
What about |
Hi Martin, |
Hmm, I thought that was ONLY for the L1 Trigger Menu Record?? |
Right, so L1T should be the only difference right? The other tags (including EGM ones) should be the same, as shown in the difference in #37557 (comment). Maybe am I missing something? |
123X_dataRun3_v4 and 123X_dataRun3_relval_v3 show more differences than just the L1 Menu - labelled IOV differences. Should those be discounted? |
You are right, there are some IOV differences due to the fact that |
This is a recipe for irreproducibility in relval workflows! |
I'm getting confused now. |
Indeed, this is what was asked in #37557 (comment).
By the way, What I don't know is whether we need to keep the HLT tags (in general, not specifically for EGM) in both |
Hi, there are a few points to be clarified and a proposal to follow, for your consideration. Concerning the Offline tags: We could use the same tags as the ones used in HLT GT. They would have the right IOV structure and only pick up the new payload for new data. In case @cms-sw/egamma-pog-l2 agrees, would you please ask your AlCaDb contact to queue these tags to the Queues
dataRun3_Prompt: @cms-sw/egamma-pog-l2 , @missirol , @Martin-Grunewald , please let us know how does this plan sounds for you. |
Thanks, sounds good to me. My understanding is that these HLT tags (i.e. the ones with labels To be exact, I do see one place in RECO where these labels are referenced: here. I think this is used in certain wfs that run only parts of the offline reconstruction, and that were mainly introduced for GPU-related development. I don't think this should prevent the update of the HLT tags in the Offline GTs, but I don't know these wfs well, so I thought it was worth mentioning this in case others see problems. |
Helena's proposal sounds good to me too. I had not noticed before that e/gamma HLT tags are also present already in offline data GTs. My apologies for the oversight. Since the HLT tags are already there in offline GTs, it makes sense to update them too, so that all Run3 GTs are consistent. @saumyaphor4252 , please take note of Helena's proposal above, and kindly queue egamma HLT supercluster regression tags to |
OK, sounds good, thanks! |
PR description:
This PR is to include the new 123X online GTs as well as updating 123X_dataRun3_HLT_relval in autoCond.py
The difference with respect to the previous version of the GTs is the inclusion of new PPS and EGM HLT supercluster regression tags (presented here: https://indico.cern.ch/event/1149147/#22-egm-hlt-supercluster-regres).
Note on the EGM tags: the current GTs do not yet have the new payloads. The new payloads will be appended once the FTV has validated them. And then, when doing the append, the corresponding flag should be changed as well.
The GT differences are listed below:
run3_hlt
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts//123X_dataRun3_HLT_v7/123X_dataRun3_HLT_v5
run3_hlt_relval
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts//123X_dataRun3_HLT_relval_v4/123X_dataRun3_HLT_relval_v3
run3_data_express
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts//123X_dataRun3_Express_v5/123X_dataRun3_Express_v4
run3_data_prompt
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts//123X_dataRun3_Prompt_v6/123X_dataRun3_Prompt_v5
PR validation:
The GTs were validated on a Tier0 replay (Express and Prompt) and a Fast Track Validation (HLT and Express).
if this PR is a backport please specify the original PR and why you need to backport that PR:
backport needed in 12_3_X