Skip to content
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

Minor bug in DeepTauId.cc #23

Open
veelken opened this issue Aug 26, 2020 · 7 comments
Open

Minor bug in DeepTauId.cc #23

veelken opened this issue Aug 26, 2020 · 7 comments

Comments

@veelken
Copy link

veelken commented Aug 26, 2020

Hi,

I think I have found a minor bug in the DeepTauId.cc code: In lines 1190 and 1198 of
https://cmssdt.cern.ch/lxr/source/RecoTauTag/RecoTau/plugins/DeepTauId.cc
the variables "footprintCorrectiondR03" and "photonPtSumOutsideSignalConedR03" need to be replaced by "footprintCorrection" and "photonPtSumOutsideSignalCone", respectively, for consistency with lines 40 and 42 of
https://github.com/cms-tau-pog/TauMLTools/blob/master/Training/python/common.py

I consider this to be a minor bug only, because the footprintCorrection and photonPtSumOutsideSignalCone are very similar for dR=0.5 and dR=0.3. In some events that I am debugging right now, the effect of removing the "dR03" from lines 1190 and 1198 of the DeepTauId.cc code changes the VSjetraw output by 0.01 to 0.02.

@mbluj
Copy link
Contributor

mbluj commented Aug 26, 2020

Just a comment: as far as I remember "photonPtSumOutsideSignalCone" and "photonPtSumOutsideSignalConedR03" are in fact identical; the version with "R03" suffix was setup to avoid confusions, but does not change content. On the other hand footprintCorrection is different for R03 as it is computed withing isolation cone of 0.3 while the standard one within 0.5, so here the suffix is meaningful.

@swozniewski
Copy link

It looks like before march 2019, both versions were in the list in common.py and this entry in DeepTauId.cc was added in April 2019. Are we sure that the current training (which I guess was started around that time) is based on the 0p5 version? Maybe @kandrosov knows?

@kandrosov
Copy link
Collaborator

I confirm that the DeepTau v2 training is based on 0p5 version of footprintCorrection and photonPtSumOutsideSignalCone. Unfortunately, this inconsistency in DeepTauId.cc was not spotted during integrations checks. Probably, because it is not very frequent that footprintCorrection != footprintCorrectiondR03, so it did not occur on the set validation of events (the number of events was few thousands if I recall correctly).

@kandrosov
Copy link
Collaborator

kandrosov commented Aug 26, 2020

On the other hand, I don't think that we should fix the bug in CMSSW now, considering that the effect should be minor. In my opinion, it would be better to keep bug-to-bug compatibility with DeepTau v2p1 to have a consistent UL production. Then for Run 3, many things will change, and this issue can be fixed in DeepTau v3 implementation.
@mbluj @swozniewski do you agree?

@mbluj
Copy link
Contributor

mbluj commented Aug 26, 2020

On the other hand, I don't think that we should fix the bug in CMSSW now, considering that the effect should be minor. In my opinion, it would be better to keep bug-to-bug compatibility with DeepTau v2p1 to have a consistent UL production. Then for Run 3, may things will change, and this issue can be fixed in DeepTau v3 implementation.
@mbluj @swozniewski do you agree?

In tend to agree.

@swozniewski
Copy link

yes, me too

@veelken
Copy link
Author

veelken commented Aug 26, 2020

Hi,

I agree with Konstantin that it is better to keep the bug in the CMSSW release that is used for the UL processing (to reduce e.g. changes to data/MC scale factors). In my opinion it will be useful to fix the bug in the first open CMSSW release after the UL processing, in order to bring the DeepTau training code in sync with the DeepTauId.cc code in CMSSW.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants