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
Fix for jet flavour clustering #3994
Fix for jet flavour clustering #3994
Conversation
A new Pull Request was created by @ferencek (Dinko Ferencek) for CMSSW_7_1_X. Fix for jet flavour clustering It involves the following packages: PhysicsTools/JetExamples @nclopezo, @vadler, @cmsbuild, @Degano, @monttj can you please review it and eventually sign? Thanks. |
if( matchedIdx>=0 ) | ||
{ | ||
matchedLocks.at(matchedIdx) = true; | ||
if ( matchedDR2 > rParam_*rParam_ ) |
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.
matchedLocks
remains true
in this case?
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.
@vadler With this condition met, something was not configured properly and the final results should not be used until the problem is fixed. So in that sense, one could argue that it doesn't matter much to what value matchedLocks
was set since the results probably don't make much sense anyways. However, I do agree that it is probably better not to set matchedLocks
to true
if this condition is met. The same is true for https://github.com/cms-sw/cmssw/pull/3994/files#r13046815 I will commit a fix soon. Thanks for spotting this.
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.
Thanx!
Please also propagate the fix to the other release cycles, so that I can sign all four of them right away ;-)
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.
Will do. Working on it right now.
+1 |
-1 |
+1 |
@vadler Correct, the current behavior is the intended one. |
Btw, will this PR be automatically propagated to the 7_2_X branch? |
…m-CMSSW_7_1_0_pre8 Fix for jet flavour clustering
yes |
Fix for correct handling of pathological cases of Pt=0 groomed jets in the jet flavour clustering.