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

fix:Incorrect MET correction method CorrectedMETFactory.py #1066

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

9GaoHong
Copy link

No description provided.

@9GaoHong 9GaoHong changed the title Update CorrectedMETFactory.py fix:Incorrect MET correction method CorrectedMETFactory.py Mar 21, 2024
@lgray
Copy link
Collaborator

lgray commented Mar 21, 2024

Please fix the tests your change has cause to fail.

https://github.com/CoffeaTeam/coffea/actions/runs/8376520077/job/22936434720?pr=1066#step:10:477

):
sj, cj = numpy.sin(jet_phi), numpy.cos(jet_phi)
x = met_pt * numpy.cos(met_phi) + awkward.sum((jet_pt - jet_pt_orig) * cj, axis=1)
y = met_pt * numpy.sin(met_phi) + awkward.sum((jet_pt - jet_pt_orig) * sj, axis=1)
x = met_pt * numpy.cos(met_phi) + awkward.sum(jet_pt * cj, axis=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be calculating a difference in the corrected momentum with respect to each jet? That's what it says in the equation you quote, that's what the code was doing before. Now it does not. Please justify this change.

That or create a test that compares to outputs from some other tool.

Copy link
Author

@9GaoHong 9GaoHong Mar 22, 2024

Choose a reason for hiding this comment

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

Sorry, the + here should be the -, I have updated it.
I have attached the example code in the issue.
The Type-I correction calculation formula has been implemented in corrected_jets

def corrected_jets(self, jets, event_rho, lazy_cache):
        jets_L123 = self.jec_factory.build(
            add_jme_variables(jets, event_rho),
            lazy_cache #events.caches[0]
        )
        jets_L1 = self.jec_factory_L1.build(
            add_jme_variables(jets, event_rho),
            lazy_cache #events.caches[0]
        )
        emFraction = jets_L123.chEmEF + jets_L123.neEmEF
        mask_jec = (jets_L123.pt > 15) & (emFraction <= 0.9)
        selected_jets_L123 = ak.mask(jets_L123, mask_jec)
        jets_jec = selected_jets_L123
        jets_jec['pt'] = selected_jets_L123.pt - jets_L1.pt
        return jets_jec

Then propagate the difference value calculation of jet_pt corrected by L123 and L1 to MET through the following code:

jets_col = self._jmeu.corrected_jets(jets_col, rho, cache)
met  = self._jmeu.corrected_met(event.RawMET,event.MET, jets_col, event.fixedGridRhoFastjetAll, event.caches[0])

Does this explain that my updated code is correct?

Copy link
Collaborator

@lgray lgray Mar 22, 2024

Choose a reason for hiding this comment

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

Ah this is a very clear to implement this since it now becomes unclear what the user is getting as a physics object. They expect to get the jets corrected by the stack that they put in, not something else that is suitable for MET correction.

Instead, it would be best to create two CorrectedJetsFactories:

jets_l123 = CorrectedJetFactory(config_L123).build(events.Jet) #add rho and whatnot to events.Jet
jets_l1 = CorrectedJetFactory(config_L1).build(events.Jet)

#then set ptRaw in the JECStack to jets_l1.pt
corr_met = CorrectedMetFactory(config_l123_l1).build(type1_met, jets_l123)

This would require no code changes to my understanding.

Copy link
Author

Choose a reason for hiding this comment

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

I agree that set ptRaw to jets_L1.pt,it's a good alternative.
But the plus signs + here should be changed to -. Just like:
x = met_pt * numpy.cos(met_phi) - awkward.sum((jet_pt - jet_pt_orig) * cj, axis=1)

@@ -36,7 +36,7 @@ def __init__(self, name_map):

self.name_map = name_map

def build(self, in_MET, in_corrected_jets):
def build(self, in_MET, type1_MET, in_corrected_jets):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couldn't you just set the Type1 MET as the MET that is input to the build function? This seems odd.

Copy link
Author

Choose a reason for hiding this comment

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

  • In order to obtain the corresponding uncertainty values of MET after correction for JEC and JER, we must use RawMET as input for correction.RawMET means uncorrected PF MET, while MET refers to Type-1 corrected PF MET. If using MET as input, it means that it has been corrected twice.

  • But if we want to obtain the UnclusterdEnergy uncertainty value of MET, you must use MET as input, as in NanoAOD, only MET has the branch for MetUnclusterEnUpDeltaX/Y, while RawMET does not. Therefore, it is necessary to use MET to obtain the UnclusteredEnergy uncertainty value.

So I have to input both in_MET and type1_MET meanwhile, they represent RawMET and MET respectively, for example:
met = self._jmeu.corrected_met(event.RawMET,event.MET, jets_col, event.fixedGridRhoFastjetAll, event.caches[0])

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps a better way to put this is I think this can be solved by changing the flexibility in the configuration as opposed to changing the meaning of the physics objects that people get out from correctors.

Copy link
Author

Choose a reason for hiding this comment

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

But it is obvious that the CorrectedMETFactory function here cannot implement the correction of RawMET as input. Because RawMET does not have the MetUnclusterEnUpDeltaX/Y branch, a bug will occur. How can I modify the configuration file to solve this problem? At the same time, I also need to obtain the uncertainty of MET_UnclusterdEnergy_up/down

Copy link
Collaborator

Choose a reason for hiding this comment

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

This would be better solved by making a new met object with the appropriate substitutions.

raw_met = events.RawMET
met_to_correct = events.MET
met_to_correct["pt"] = raw_met.pt
met_to_correct["phi"] = raw_met.phi

# then go on to correct met_to_correct, adjusting configuration as appropriate

Copy link
Collaborator

Choose a reason for hiding this comment

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

Though I'm not sure if this spoils the validity of the systematic variations.

Copy link
Author

Choose a reason for hiding this comment

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

I know what you mean! Great!
This will not spoils the validity of the systemic variations, the uncertainty of MET_UnclusteredEnergy is calculated using the corrected MET_pt value through corrected_polar_met, rather than RawMET.pt.
You are incredibly smart, thank you so much for your patient and thoughtful answers.

@9GaoHong 9GaoHong closed this Mar 22, 2024
@9GaoHong 9GaoHong reopened this Mar 22, 2024
9GaoHong added a commit to 9GaoHong/coffea that referenced this pull request Mar 22, 2024
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.

2 participants