-
Notifications
You must be signed in to change notification settings - Fork 8
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
Change tau trigger bits for nanoAOD #164
Change tau trigger bits for nanoAOD #164
Conversation
@lwezenbe, @kandrosov I have just went through the changes and they seem sensible for me. However, my knowledge on current tau triggers does not allow to check if there are not missing patters of trigger filter names or so. |
@mbluj I have checked this within my capabilities. For 2016, 2017 and 2018 I have run it on miniAOD before with the version of the trigger menu used for that specific sample and it worked if I used the appropriate CMSSW version and ported the changes into it. For Run III samples I have also tried to run it on the already existing MiniAOD that has already been produced but I wasn't sure what menu they had used exactly, I was trying to figure that out earlier. I wanted to recheck if everything is fine after the changes and just do another sanity check today but I had a lot of interference with from other projects/teaching this week. |
@lwezenbe thank you. I think these tests are good enough. I will merge this development and prepare a PR to official CMSSW. I suppose that this PR will be processed some time (~1 week or so) so additional last minute fixes will be still possible. @kandrosov, is backport of this development to 10_4_X foreseen? |
+1 |
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.
I went through this development, I found proposed changes sensible and I approve them.
For now, no backports are planned. We going to follow XPOG guidelines once they are defined. Currently, PPD does not plan to reprocess Run2 in nano v10 format (they want to accumulate more changes before), so this change will only go for Run 3 + private nano productions for Run 2. @lwezenbe we can point to this code in the tau trigger TWiki for the analysers who plans to have private UL nanoAODs with correct trigger bits. |
OK, fine. But as far as I know with current production setup nanoAOD is produced together with RECO/AOD/MiniAOD, so the first, prompt nanoAOD will still contain tau trigger bits with old (partially buggy) setup. On the other hand, backport to 12_4_X will require special switch to ensure backward compatibility of the nanoAOD content so most likely prompt nanoAOD will be also produced with the old setup... |
@mbluj good point. Perhaps we will need to clarify with XPOG what would be the best way to proceed. But I think it is better to accumulate all modifications that we want to have for Run 3 nanos (e.g. DeepTau v2p5) before. I expect that we'll open a PR for v2p5 for nano (into the cms-tau-pog fork) the next week at the latest. Could you, please, prepare a branch for it? |
It sounds as a good plan. I will prepare a new branch for the WPs; whom I should inform that it is ready? |
@lwezenbe, PR to the official CMSSW is being reviewed. In particluar they ask if observed changes here: https://gitlab.cern.ch/cms-nanoAOD/nanoAOD-integration/-/issues/155 are as expected. I am going to check this, but a cross check from your side is welcome. Thanks! |
me and Oleg. Thank you! |
Description
The tau trigger bits for NanoAOD never provided a match in 2018 trigger because the 'Hps' part in the name was forgotten. This PR is meant as both a fix for that and as a preparation for Run III. We have decided to go with a unified strategy that will work for both Run II and Run III triggers, hence the addition of extra bits containing DeepTau. It now also contains trigger bits for taus in the PAG VBF and displaced tau triggers. A full overview of all trigger filters in Run II and Run III was made and is contained in the following google sheets:
https://docs.google.com/spreadsheets/d/1L8P4HeiUSaisewuSFBEJOM5t5xXcDjNbfBtD9hEE5F8/edit#gid=0
This might need a review from a few more people to make sure that we have everything we need. Also bits 65536 and beyond were added for completeness but I am not fully sure if we need them
Changes
Tests