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

UL case not covered in fatJetUncertaintiesProducer #285

Open
nowaythatsok opened this issue Jul 12, 2021 · 3 comments
Open

UL case not covered in fatJetUncertaintiesProducer #285

nowaythatsok opened this issue Jul 12, 2021 · 3 comments

Comments

@nowaythatsok
Copy link

nowaythatsok commented Jul 12, 2021

The first dict in jetmetHelperRun2.py seems to suggest that for UL datasets the correct dataYear parameter starts with UL. Subsequently, in fatJetUncertaintiesProducer this leads to a crash in this line ( UnboundLocalError: local variable 'jmstau21DDTNomVal' referenced before assignment ) as era strings starting with UL are not covered in the previous if statement.
I am tagging my colleague @bartokm so we can both follow this issue.

@meisonlikesicecream
Copy link
Contributor

meisonlikesicecream commented Mar 12, 2022

Also stumbled upon this issue, and wondering if you ever solved it? I can see the PR but it's still open after almost a year...

Currently I'm just doing it the ugly way and removing the UL substring in the following line: https://github.com/cms-nanoAOD/nanoAOD-tools/blob/master/python/postprocessing/modules/jme/fatJetUncertainties.py#L34

In other words: self.era = era.replace("UL", "")

@bartokm
Copy link
Contributor

bartokm commented Mar 12, 2022

Hi,
That is one way to do it, but I think it's not optimal for future changes. What we did is the same as in this PR (which was never merged...):
https://github.com/cms-nanoAOD/nanoAOD-tools/pull/286/files

@meisonlikesicecream
Copy link
Contributor

Hi, That is one way to do it, but I think it's not optimal for future changes. What we did is the same as in this PR (which was never merged...): https://github.com/cms-nanoAOD/nanoAOD-tools/pull/286/files

Definitely agree that's the better way to do it. Shame that the PR hasn't been merged :-(

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

No branches or pull requests

3 participants