Merge UCSD efforts into main - #121
aashayarora wants to merge 12 commits into
Conversation
|
Thanks for preparing this PR @aashayarora! Before getting into the details, I wanted to understand why the configs in the old configs directory are needed, is there something in particular these channels need that are not available on the main configs on the master branch? |
mmfsz
left a comment
There was a problem hiding this comment.
Thank you Aashay! I have two comments:
-
On this branch the 2025 era is keyed as 2025Prompt in the C++ (corrections.cpp, weights.h, utils.cpp), but etc/dataset_names_ref.py and LUMI_DICT in etc/make_sample_jsons.py still tag the 2025 data as 2025. So anyone generating input jsons with make_sample_jsons.py on this branch gets year = "2025" in the metadata, which matches none of the 2025Prompt table entries. For JEC/JER that throws at runtime; for pileup, golden JSON, b-tagging, and the is2025 trigger define it silently falls through. Could you either switch the C++ here to 2025, or update dataset_names_ref.py, LUMI_DICT, and etc/build_trigger_logical_or.py to 2025Prompt?
-
Can you clarify for which MC samples and with which preselection the resampling was derived for? I left two inline comments to suggest changing the .root file names to something more descriptive. These could include also something indicating at which point the resampling is done, i.e. can one use these also in other channels, or are they specific to one channel?
Regarding this, just for reference: As discussed on the Mattermost, we modified the way the paths and prefixes are handed (#122) so that this PR can now hopefully move to using the central common jsons. |
| const std::string trigger_logic_string_0lep1FJ = "(is2016 && (( ((shortname==\"JetHT\") || !isData) && ((HLT_AK8DiPFJet280_200_TrimMass30_BTagCSV_p20 == true) || (HLT_AK8PFHT700_TrimR0p1PT0p03Mass50 == true) || (HLT_AK8PFJet360_TrimMass30 == true) || (HLT_PFHT650_WideJetMJJ900DEtaJJ1p5 == true) || (HLT_PFHT650_WideJetMJJ950DEtaJJ1p5 == true) || (HLT_PFHT800 == true) || (HLT_PFHT900 == true) || (HLT_PFJet450 == true)) ))) || (is2017 && (( ((shortname==\"JetHT\") || !isData) && ((HLT_AK8PFHT800_TrimMass50 == true) || (HLT_AK8PFJet330_BTagCSV_p17 == true) || (HLT_AK8PFJet400_TrimMass30 == true) || (HLT_AK8PFJet420_TrimMass30 == true) || (HLT_AK8PFJet500 == true) || (HLT_PFHT1050 == true) || (HLT_PFJet500 == true)) ))) || (is2018 && (( ((shortname==\"JetHT\") || !isData) && ((HLT_AK8PFHT800_TrimMass50 == true) || (HLT_AK8PFJet330_TrimMass30_PFAK8BoostedDoubleB_np2 == true) || (HLT_AK8PFJet400_TrimMass30 == true) || (HLT_AK8PFJet420_TrimMass30 == true) || (HLT_AK8PFJet500 == true) || (HLT_PFHT1050 == true) || (HLT_PFJet500 == true)) ))) || (is2022 && (( ((shortname==\"JetMET\") || !isData) && ((HLT_AK8PFJet250_SoftDropMass40_PFAK8ParticleNetBB0p35 == true) || (HLT_AK8PFJet425_SoftDropMass40 == true)) ))) || (is2023 && (( ((shortname==\"JetMET\") || !isData) && ((HLT_AK8PFJet250_SoftDropMass40_PNetBB0p06 == true) || (HLT_AK8PFJet425_SoftDropMass40 == true)) ))) || (is2024 && (( ((shortname==\"JetMET\") || !isData) && ((HLT_AK8PFJet250_SoftDropMass40_PNetBB0p06 == true) || (HLT_AK8PFJet425_SoftDropMass30 == true)) ))) || (is2025 && (( ((shortname==\"JetMET\") || !isData) && ((HLT_AK8PFJet250_SoftDropMass40_PNetBB0p06 == true) || (HLT_AK8PFJet425_SoftDropMass30 == true)) )))"; | ||
|
|
||
| const std::string trigger_logic_string_singlelep = "(is2016 && ((((isData && (shortname == \"SingleMuon\")) || !isData) && (HLT_IsoMu24 == true || HLT_IsoTkMu24 == true)) || (((isData && (shortname == \"SingleElectron\")) || !isData) && (HLT_Ele27_eta2p1_WPTight_Gsf == true) && !(HLT_IsoMu24 == true || HLT_IsoTkMu24 == true)))) ||(is2017 && ((((isData && (shortname == \"SingleMuon\")) || !isData) && (HLT_IsoMu27 == true)) || (((isData && (shortname == \"SingleElectron\")) || !isData) && (HLT_Ele32_WPTight_Gsf_L1DoubleEG == true) && !(HLT_IsoMu27 == true)))) ||(is2018 && ((((isData && (shortname == \"SingleMuon\")) || !isData) && (HLT_IsoMu24 == true)) || (((isData && (shortname == \"EGamma\")) || !isData) && (HLT_Ele32_WPTight_Gsf == true) && !(HLT_IsoMu24 == true)))) ||((is2022 || is2023 || is2024 || is2025) && ((((isData && (shortname == \"Muon\" || shortname == \"Muon0\" || shortname == \"Muon1\")) || !isData) && (HLT_IsoMu24 == true)) || (((isData && (shortname == \"EGamma\" || shortname == \"EGamma0\" || shortname == \"EGamma1\" || shortname == \"EGamma2\" || shortname == \"EGamma3\")) || !isData) && (HLT_Ele30_WPTight_Gsf == true) && !(HLT_IsoMu24 == true))))"; | ||
| const std::string trigger_logic_string_ht = "(is2016 && (HLT_PFHT800 || HLT_PFHT900)) || (!is2016 && HLT_PFHT1050)"; |
There was a problem hiding this comment.
I am not sure if this trigger_logic_string_ht is exactly what we want. I think the trigger_logic_string_ht is used by 0lep_3FJ and 0lep_2FJ, and I think these channels include JetHT, MET, and SingleMuon datasets (for Run 2). I'm not sure if this trigger string actually removes the overlap among these datasets.
Could you confirm how the overlap is being handled for these channels?
There was a problem hiding this comment.
for the 0lep3FJ channel, in our custom configs we were only using JetHT and JetMET. Why do we need the MET and SingleMuon datasets?
There was a problem hiding this comment.
I agree those don't seem to be needed. But I think they are skimmed for that channel and the jsons exist (maybe it was for some study?) so unless there is some guard against it that I am missing, I think in the current workflow they would be run (and overlap among them would not be handled).
| // calls HvsQCD / VvsQCD -- identical formulas, Xbb/(Xbb+QCD) and | ||
| // (Xqq/3+Xcs)/(Xqq/3+Xcs+QCD), so the templates apply unchanged. See | ||
| // AK8JetsSelection in selections.cpp. | ||
| df = df.Redefine("fatjet_Hbb", [](const RVec<RVec<float>>& hv) { return hv[0]; }, {"_qcd_resampled_hv"}); |
There was a problem hiding this comment.
Are the fatjet_Hbb and the fatjet_Wqq the objects we want to use here? Just not sure if here it should be the capital or lower case ones in this context and how that interfaces with how the corrections are applied (maybe @mmfsz could comment)
There was a problem hiding this comment.
I believe only FatJet_Hbb and FatJet_Wqq are defined in selections.cpp. I think this should fail at runtime?
There was a problem hiding this comment.
df = applyObjectMaskNewAffix(df, "_good_ak8jets", "FatJet", "fatjet");
This redefines the collection with the new mask, but regardless it shouldn't matter.
There was a problem hiding this comment.
Ah sorry, that is true, so it should not fail at runtime. The only issue is that it's not ideal that the fatjet collection (which is nominal JECs) ends up resampled, while the FatJet[nominal_JEC_mask] collection remains not-resampled. At the same time, you probably don't want to resample FatJet, so I think it's fine to leave it as is.
|
|
||
| def spec_path(run): | ||
| """The production spec JSON the preselection itself is run on for this CR's skim.""" | ||
| return os.path.join(_PRESEL, "etc", "old_config", "0Lep3FJ", f"0Lep3FJ_{run}-data.json") |
There was a problem hiding this comment.
I think this "old_config" area is outdated now right?
There was a problem hiding this comment.
This script never needs to be run again in principle, but it should be easy to configure it in case someone does need to. But yes, we have moved to using the central configs.
There was a problem hiding this comment.
Ahh, ok. Could you add some comment to the top (unless it's already in some readme or comment?) that describes this, and mentions it would need to be re-configured if it needs to be run again?
| _HERE = os.path.dirname(os.path.abspath(__file__)) # preselection/data | ||
| _PRESEL = os.path.dirname(_HERE) # preselection | ||
| _SUFFIX = "" if RUN == "run3" else f"_{RUN}" | ||
| OUT_ROOT = os.path.join(_HERE, f"resampling_pdfs{_SUFFIX}.root") |
There was a problem hiding this comment.
Is the applyQCDScoreResampling in utils expecting something like "resampling_pdfs_QCD_run2.root"? If so maybe the QCD needs to get added here (or removed there), unless the string is getting modified somewhere else I guess
|
Thanks for the PR @aashayarora! In general I think it looks pretty good. I've just left a few comments above. |
|
Hi Aashay, I believe my previous comments have all been addressed. There are a few outstanding comments from Kelci and then I just wanted to ask if you could add two README.md files under the |
Uh oh!
There was an error while loading. Please reload this page.