Move combineTool.py and routines needed to create impact plots to combine

[pkausw]

This PR moves combineTool.py and the scripts needed for different helpful scripts in the CombineHarvester into combine. In particular, this will currently enable producing impact plots with combine alone.

Tested features:

• Build workspace with combineTool.py -M T2W both locally and via htcondor (tested on T2 DESY NAF)
• Calculate impacts: initial fit was run locally, fits for nuisances was submitted to htcondor system. Tested on:
  • T2 DESY NAF
  •  CERN LXPLUS (lxplus8 machine, CMSSW setup with Singularity, used outside via scram-venv mechanism)

[kcormi]

Thanks for this!

I am running some tests, and so far everything seems to be working as expected.

While we are at it, maybe it would be good to also merge and add the various plot1Dscan.py scripts?

I think there are three existing ones:

• https://github.com/cms-analysis/CombineHarvester/blob/master/CombineTools/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/tutorial_unfolding_2023/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/longexercise/plot1DScan.py

Ideally we would have one script to replace all of these and which is added into the list of combine executables.

We may also want to check for other similar scripts which are sitting somewhere in here which import from CombineHarvester python packages, but can now be switched directly into this one.

Then updating the documentation in the tutorials etc would also be important, but as we discussed should be part of a separate Pull Request, so that the documentation matches with the recommended release (and not with the main branch for example).

[adewit]

While we are at it, maybe it would be good to also merge and add the various plot1Dscan.py scripts?

I think there are three existing ones:

• https://github.com/cms-analysis/CombineHarvester/blob/master/CombineTools/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/tutorial_unfolding_2023/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/longexercise/plot1DScan.py

Ideally we would have one script to replace all of these and which is added into the list of combine executables.

I would maybe decouple this from this PR - the plot1DScan scripts are a little bit of a mess, it could take time to get into reasonable shape (OTOH if Philip has the time and wants to consolidate them now I guess I will not object :') )

[kcormi]

While we are at it, maybe it would be good to also merge and add the various plot1Dscan.py scripts?
I think there are three existing ones:

• https://github.com/cms-analysis/CombineHarvester/blob/master/CombineTools/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/tutorial_unfolding_2023/scripts/plot1DScan.py
• https://github.com/cms-analysis/HiggsAnalysis-CombinedLimit/blob/main/data/tutorials/longexercise/plot1DScan.py

Ideally we would have one script to replace all of these and which is added into the list of combine executables.

I would maybe decouple this from this PR - the plot1DScan scripts are a little bit of a mess, it could take time to get into reasonable shape (OTOH if Philip has the time and wants to consolidate them now I guess I will not object :') )

Okay, good thinking, I hadn't looked into them to see what level of mess they were, and was hoping it was maybe a small one. :)

We can have that as a separate PR then, which Philip may or may not take care of.

Still, maybe checking through and changing the relevant import statements so that they can run without CombineHarvester would be good for this PR, I think.

[ajgilbert]

Hi everyone, just checking if there's any more work to do on this PR? It could be good to merge sooner than later (and have a corresponding PR in CH to remove the same scripts). Then a new tag of both to help avoid confusion - I don't know how CMSSW will handle it if there are two scripts with the same name in different packages.

[anigamova]

Hi @ajgilbert, @pkausw,
I think the only thing left is to update the documentation, and indeed remove the combineTool.py scripts from the CH.
Then we should decide how to handle the tutorial docs:

Option 1 is to just leave it as it is because they anyway refer to the sparse checkout scripts:

bash <(curl -s https://raw.githubusercontent.com/cms-analysis/CombineHarvester/main/CombineTools/scripts/sparse-checkout-https.sh)

and these scripts point to the tags, so it should work.

Option 2 is to update all of the tutorial docs and always point to the main installation instruction as done in the unfolding tutorial for example.

I think both options are fine, and the first one requires a bit less work.

[ajgilbert]

Hi @anigamova: i think actually we don't even need this sparse checkout scripts anymore. They were useful at the time we had a lot of analysis-specific packages in CH too, but now there's not much difference at all between the sparse checkout and just cloning the full repo. That said, we can leave them for now, to avoid having to change the docs too much.

If I understand correctly, @pkausw went through and updated most of the tutorials already to point out that CombineHarvester is no longer needed?

[anigamova]

I think everything looks great, I tested the setup in a clean environment with CMSSW_14_0_0_pre1 and CombineHarvester with changes from cms-analysis/CombineHarvester#318

[pkausw]

Hi @ajgilbert , I went through the docs with @kcormi last week. I mainly updated the parts of the (tutorial) docs that stated that the combineTool.py is part of the CombineHarvester repo. However, I didn't remove the references to the Harvester completely from the docs because we didn't move all of the plotting scripts initially and I wanted to ensure that the instructions still work. Now that all plotting routines are moved from the Harvester, we might want to think about removing the instructions to clone the CombineHarvester repo from the tutorials altogether, but I think that's nominally not part of this PR but rather PR #908 , right?

[ajgilbert]

Yes, I saw #908. Ok, if all the plotting is moved then I agree, remove the instructions for CH. Thanks a lot for the effort on this!

[anigamova]

I think we can merge this if no one objects, but it would be good to update the #908 and remove the CombineHarvester setup instructions from the tutorials for the upcoming release.

[ajgilbert]

Please note a small update was made to the LimitGrid.py module in CH: cms-analysis/CombineHarvester#319 it should be ported here