-
Notifications
You must be signed in to change notification settings - Fork 126
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
R22 Jet Pile-up tagging and MET pre-recommendations #1580
Conversation
Hi @miholzbo , Thanks for implementing this. Up to now, we were recalculating JVT and fJVT in JetCalibrator. I'm in favour of making this change but we should remove that option in JetCalibrator then (m_redoJVT and m_calculatefJVT). Also, it looks like you are recalculating JVT but not fJVT, don't we need to recalculate fJVT as well? Also, please note that by default we were never recalculating JVT by default. We could change that behaviour but we need at least to let users to be able not to recalculate (f)JVT (this was needed in some cases). |
Hi @jbossios, thanks a lot for pointing this out; I didn't think of the "(f)Jvt handling" that is already done in the JetCalibrator. Actually, it would make a lot of sense to do the re-calculation of NNJvt there (as an "after-burner" of the jet calibration) but then we would need to set up the same tool twice: in the JetCalibrator to calculate and then in the JetSelector to apply the WP. But maybe that's the best way ... let me know what your thoughts are about this! Ideally, we would recalculate fJvt as well but that's not really possible as for EMPFlow jets it requires to calculate a EMPFlow jet collection for all PU vertices. Hence, we rely on the fJvt decoration calculated at Derivation level (which actually causes some problems still to be solved ...). Out of curiosity: why was Jvt not re-calculated as default? It should be the correct thing to day as Jvt also depends on the jet pT and can change after the calibration was applied at analysis level. |
Both options have cons and pros but I'm more inclined to move the recalculation to JetSelector and this is a good time to make a change and reduce complexity/duplication. Anyone needing (NN)Jvt should be relaying on the tool to decide if a jet passes the selection or not, hence they should be using the JetSelector, instead of simply dumping the (NN)Jvt score, and that's where we can make the recalculation.
I see, thanks for the clarification.
I would make the default to always recalculate JVT but we should allow people to use JetSelector for jets for which we don't have a JVT recommendation (like for online EMTopo jets). |
Thanks for removing the recalculation of (f)JVT in JetCalibrator. I think the only thing left to do is to add a flag to be able to disable the recalculation of (NN)JVT in JetSelector.cxx#637 (we also probably don't want to init the tool in that case as well). |
Sounds reasonable to me! The latest makes this re-calculation configurable (default: true), as well as the tagging algorithm (with the default being NNJvt). The later should make sense because otherwise it wouldn't work if someones specifies EMTopo jets (which don't carry NNJvt), in that case Jvt can be at least tested (even if there likely won't be any calibrations.) |
Looks perfect. I will merge if CI tests pass. |
This MR implements recommendations that have become available with AnalysisBase 22.2.97 (releases before that are not compatible). In particular NNJvt is now used as successor of Jvt.
This mostly affects the setup of the tools which apply the NNJvt and fJvt requirements and retrieve the SFs. The MR also adds back the fJvt which has been commented out so far in R22.
In addition, some additional features/changes have been implemented: