search

cms-jet / JetToolbox

Python framework for configuration of jet tools via the jet toolbox.
https://twiki.cern.ch/twiki/bin/viewauth/CMS/JetToolbox
7 stars 36 forks source link

Fixing addQJets #13

Closed mhwalker closed 9 years ago

mhwalker commented 9 years ago

The current code adds an incorrectly named user variable, which throws a ProductNotFound exception. I changed it to match the correct name.

rappoccio commented 9 years ago

What release/releases is this compatible with?

alefisico commented 9 years ago

I think actually it must be something like in the case of PUJetID: https://github.com/mhwalker/JetToolbox/blob/0873adde5a1c0490912f599b9babb2b452fe8053/python/jetToolbox_cff.py#L714-L732 because otherwise there will not be difference if you want to run Puppi or CHS or other PUMethod.

rappoccio commented 9 years ago

OK, what is the recipe to be consistent? Is this PR synced to the JetToolbox?

ahinzmann commented 9 years ago

It looks like the q/g-discrimination code is under "if addQJets". QJets (for W-tagging) and QG-likelihood are completely different discriminators. Looking at the code with github indentations. Maybe this is simply a display problem of tabs and spaces.

mhwalker commented 9 years ago

The change I made works in CMSSW_7_4_2. Based on the surrounding comments in the code, I guess it won't work in 73X, though I didn't test it.

alefisico commented 9 years ago

I think we will have the issue as with PUJetID with the names for Puppi or CHS. Let me fix this (changing it to QGJets too) and I will make a new pull request

rappoccio commented 9 years ago

Thanks @alefisico. Can you do the jetToolbox and this together?

We're going to need to make a specific tag for this in both of the cases so that we can ensure people are using the right thing, since they can now get out of sync.

alefisico commented 9 years ago

@rappoccio sorry, what do you mean by do the jetToolbox and this together?

rappoccio commented 9 years ago

Wait, never mind. I misread this and thought it was part of the JMEValidator.

OK, sorry for the confusion! :)

alefisico commented 9 years ago

I just fix the QGtagger part in another pull: https://github.com/cms-jet/JetToolbox/pull/15 Therefore I will close this pull.