search

cms-analysis / CombineHarvester

CMSSW package for the creation, editing and analysis of combine datacards and workspaces
cms-analysis.github.io/CombineHarvester/
15 stars 182 forks source link

Plots for18016 aug14 #172

Closed capalmer85 closed 6 years ago

capalmer85 commented 6 years ago

@adewit, maybe let's look over this a bit before merging. postFitPlot.py does a lot of things now.

adewit commented 6 years ago

Hmm, what was wrong with keeping the scripts for the mjj plot separate from the original postFitPlot.py? I know it duplicates some code then but I don't see a requirement for a plotting script to have to be able to make all of the plots that use slightly similar code. Not saying the script was ever very clean, but having to keep in mind N different plots you might want to make when making quick changes is not ideal. However, given that we're probably not going to reuse this script as it is, as long as the script actually reproduces the plots as expected I don't think it is a problem. Have you checked this already?

Also, I'm not sure what m2.json is but doesn't look like anything we have used for any of the plots. Are you also still planning on adding the root files with histograms that are inputs to some of these scripts or has the plan changed?

capalmer85 commented 6 years ago

Mostly it just seemed natural to put them all together. But actually it seamlessly solved formatting problems with the other post fit plots (DNNs).

The big thing that I added was the flag for the bkg subtraction.

On Wed, Aug 15, 2018 at 6:03 AM adewit notifications@github.com wrote:

Hmm, what was wrong with keeping the scripts for the mjj plot separate from the original postFitPlot.py? I know it duplicates some code then but I don't see a requirement for a plotting script to have to be able to make all of the plots that use slightly similar code. Not saying the script was ever very clean, but having to keep in mind N different plots you might want to make when making quick changes is not ideal. However, given that we're probably not going to reuse this script as it is, as long as the script actually reproduces the plots as expected I don't think it is a problem. Have you checked this already?

Also, I'm not sure what m2.json is but doesn't look like anything we have used for any of the plots. Are you also still planning on adding the root files with histograms that are inputs to some of these scripts or has the plan changed?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cms-analysis/CombineHarvester/pull/172#issuecomment-413151642, or mute the thread https://github.com/notifications/unsubscribe-auth/AFKqh290BSe4W1A4jadwdFDK6O0k5VYsks5uQ_GhgaJpZM4V9d3G .

adewit commented 6 years ago

@capalmer85 you are not planning any further updates imminently, right?

capalmer85 commented 6 years ago

I just added the stuff for the log S/B VZbb plot. I don't envision anything further. :)