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

Integration of correction-lib within CorrectedJets/METFactory #1134

Open
wants to merge 4 commits into
base: backports-v0.7.x
Choose a base branch
from

Conversation

anpicci
Copy link

@anpicci anpicci commented Jul 19, 2024

This PR is intended to track the progress on integrating correction-lib within the coffea JME tools.

The very first contribution is the calculation of the JECs using inputs from the correction-lib tool.

FYI @lgray @nsmith-

class CorrectedJetsFactory(object):
def __init__(self, name_map, jec_stack):
def __init__(self, name_map, jec_stack, jec_names, jec_year):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not alter the signature of this function (especially with new non-defaulted arguments). We want to maintain backwards compatibility for the time being.

Could you try implementing this such that the code detects correction-lib inputs coming in from the JEC stack, and then follow a different code path in the case of correctionlib inputs?

Copy link
Author

@anpicci anpicci Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgray thank you for pointing out this. Would it work if jec_stack is a list or a dict if one wants to use correction-lib? IMU, the JECStack class (as well as the auxiliary ones) is not needed when using correction-lib, as the latter already has functionalities to get the right numbers from the formulas that are part of the JME corrections/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JEC stack also defines the mapping from names in your data to the arguments of the functions to evaluate. Sure, you can have the jec_stack be something much simpler (like a list) if correctionlib does a bunch of the bookkeeping for you.

Though I think you'll need to do some arranging yourself for the JES uncertainties.

@@ -171,6 +191,29 @@ def build(self, jets, lazy_cache):
out_dict = dict(in_dict)

# take care of nominal JEC (no JER if available)
## General setup to use correction-lib
clib_json = self.jec_year
json_path = f"/cvmfs/cms.cern.ch/rsync/cms-nanoAOD/jsonpog-integration/POG/JME/{clib_json}/jet_jerc.json.gz"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not put hard dependencies on cvmfs paths in coffea.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be configurable.

I'd also be careful of building new objects every time you call uncertainties, this can become very slow.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the json file name would change depending on if it's AK8 or AK4. So, I would also recommend against hard-coding this and instead keeping this in the jec_stack.

self.jec_names = jec_names
self.jec_year = jec_year
## the last element of jec_names has to be a boolean, specifying if the use wants to save the different correction levels
self.separated = self.jec_names.pop()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a useful debug feature, but from the JME point of view, providing just the name of the JEC tag for the required step should suffice. I don't foresee a need for analyses to store the result of each step, but I'm open to hearing about any exceptions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garvitaa I would suggest keeping it to provide users more flexibility, without the need for them to put the hands on the coffee scripts

@anpicci
Copy link
Author

anpicci commented Aug 22, 2024

@lgray @nsmith- @garvitaa I have pushed a commit to address your comments, as well as to extend the correction-lib implementation to JER and JEC uncertainties

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

Successfully merging this pull request may close these issues.

3 participants