cms-tau-pog / TauFW

Analysis framework for tau analysis at CMS using NanoAOD
9 stars 40 forks source link

Rewriting using config file #15

Closed smonig closed 2 years ago

smonig commented 2 years ago

These changes aim at removing hard-coded configuration for samples, systematics, observables, regions, etc. that need to be repeated several times and thus suffer from copy paste errors and inconsistencies. All this information should be centralised in a config file in yaml format and be read by different routines.

One config file per channel is needed, so also for signal and control regions if they use different channels. Combination is done at datacard level in the fit through defining common parameters in the workspaces.

IzaakWN commented 2 years ago

Hi @smonig, overall it looks good and it's a nice idea to make it easier to swap config files and avoid errors.

One important thing: Plotter/plot.py script for general use. Right now you have made it very TauES-specific. Can you either undo the changes you made to this script, or create the needed config files in TauFW/Plotter/config for the previous default setup for each channel, please?

  # SELECTIONS
  if 'mumu' in channel:
    baseline = "q_1*q_2<0 && pt_2>15 && iso_1<0.15 && iso_2<0.15 && idMedium_1 && idMedium_2 && !extraelec_veto && !extramuon_veto && metfilter && m_ll>20"
  elif 'mutau' in channel:
    baseline = "q_1*q_2<0 && iso_1<0.15 && idDecayModeNewDMs_2 && idDeepTau2017v2p1VSjet_2>=16 && idDeepTau2017v2p1VSe_2>=2 && idDeepTau2017v2p1VSmu_2>=8 && !lepton_vetoes_notau && metfilter"
  elif 'etau' in channel:
    baseline = "q_1*q_2<0 && iso_1<0.10 && mvaFall17noIso_WP90_1 && idDecayModeNewDMs_2 && idDeepTau2017v2p1VSjet_2>=16 && idDeepTau2017v2p1VSe_2>=8 && idDeepTau2017v2p1VSmu_2>=1 && !lepton_vetoes_notau && metfilter"
  elif 'tautau' in channel:
    baseline = "q_1*q_2<0 && iso_1<0.15 && iso_2<0.15 && idMedium_1 && idMedium_2 && !extraelec_veto && !extramuon_veto && m_ll>20 && metfilter"
  else:
    raise IOError("No baseline selection for channel %r defined!"%(channel))
smonig commented 2 years ago

Hi @IzaakWN ,

I have added default config files for all these channels in Plotter/ChannelConfigs. These are just very basic config files containing all info to use plot.py. One can also use the more complete ones with extra info, used for the TauES fit of course.

smonig commented 2 years ago

Hi @IzaakWN ,

with a bit of distance, I think it should now be (hopefully) complete. If you think so as well, maybe you could think of merging soon so we do not diverge?

IzaakWN commented 2 years ago

Hi @IzaakWN ,

with a bit of distance, I think it should now be (hopefully) complete. If you think so as well, maybe you could think of merging soon so we do not diverge?

Hey @smonig, thanks for your contributions (and the reminder)! I think the yml config file will help make it more customizable for different users. It seems like the only comments I have is renaming/moving a few things. Once you do that, I think it's ready for merging.

smonig commented 2 years ago

Hi @IzaakWN , with a bit of distance, I think it should now be (hopefully) complete. If you think so as well, maybe you could think of merging soon so we do not diverge?

Hey @smonig, thanks for your contributions (and the reminder)! I think the yml config file will help make it more customizable for different users. It seems like the only comments I have is renaming/moving a few things. Once you do that, I think it's ready for merging.

Sure, I'll rename / move things as you write them (or did I miss some previous comment from you?)

IzaakWN commented 2 years ago

Hi @IzaakWN , with a bit of distance, I think it should now be (hopefully) complete. If you think so as well, maybe you could think of merging soon so we do not diverge?

Hey @smonig, thanks for your contributions (and the reminder)! I think the yml config file will help make it more customizable for different users. It seems like the only comments I have is renaming/moving a few things. Once you do that, I think it's ready for merging.

Sure, I'll rename / move things as you write them (or did I miss some previous comment from you?)

Sorry, had to submit the review comments. ;)

smonig commented 2 years ago

Hi @IzaakWN , with a bit of distance, I think it should now be (hopefully) complete. If you think so as well, maybe you could think of merging soon so we do not diverge?

Hey @smonig, thanks for your contributions (and the reminder)! I think the yml config file will help make it more customizable for different users. It seems like the only comments I have is renaming/moving a few things. Once you do that, I think it's ready for merging.

Sure, I'll rename / move things as you write them (or did I miss some previous comment from you?)

Sorry, had to submit the review comments. ;)

Thanks! Changes are submitted; I'll let you resolve the conversations.

IzaakWN commented 2 years ago

Thanks again for the changes! Unless @cardinia has anything else, we'll merge this week.

cardinia commented 2 years ago

Hi @smonig @IzaakWN , no comments from my side, the PR seems complete and the changes are very useful also for other measurements. Please go ahead!

IzaakWN commented 2 years ago

I missed this: The PicoProducer/config/config.json was still in the PR... :/ I'll reset it myself, but this will screw up other people's config if they don't back it up properly.

smonig commented 2 years ago

I missed this: The PicoProducer/config/config.json was still in the PR... :/ I'll reset it myself, but this will screw up other people's config if they don't back it up properly.

Arghh, why? I am sure I removed it after your comment :-( Maybe is it possible to "forbit" edditing specific files unless explicitely telling to do so?

IzaakWN commented 2 years ago

Not sure I can do it globally. Locally, you can do:

git update-index --assume-unchanged PicoProducer/config/config.json

The problem is the file is tracked, so .gitignore does not seem to work. I don't want to remove it from the repo, because it's an example file.

cardinia commented 2 years ago

@IzaakWN an alternative could be changing the FW so that the config.json file is initialized on first use starting from a template-config.json which is instead taken from GitHub.