-
Notifications
You must be signed in to change notification settings - Fork 389
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
Add instructions example for FC 2D #921
Conversation
|
||
which will produce a file `FeldmanCousins.root` that contains a `TGraph2D` which stores the calculated value of $p_{x}$ for each point in the grid. The script below can be used to draw these values and extract the contour corresponding to 68% CL ($\alpha=0.32$). | ||
|
||
/// details | **Show script** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if its worth adding this as directly as an executable script to the repo? Or is there a reason to prefer having them copy the code and run it that way themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think that for relatively simple scripts like this (i.e not official things like the combineTool scripts), its better not to have an actual script to maintain. This way, the code is instructive rather than something to just use and not think about (at least I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, I was considering that point of view too and saw both sides of it a little bit. We can leave this one as is for now.
docs/part3/commonstatsmethods.md
Outdated
@@ -1015,11 +1007,15 @@ Imposing physical boundaries (such as requiring $\mu>0$ for a signal strength) i | |||
|
|||
The boundary is imposed by **restricting the parameter range(s)** to those set by the user, in the fits. Note that this is a trick! The actual fitted value, as one of an ensemble of outcomes, can fall outside of the allowed region, while the boundary should be imposed on the physical parameter. The effect of restricting the parameter value in the fit is such that the test statistic is modified as follows ; | |||
|
|||
$q(x) = - 2 \ln \mathcal{L}(x,\hat{\hat{\theta}}(x))/\mathcal{L}(\hat{x},\hat{\nu})$, if $\hat{x}$ in contained in the bounded range | |||
$$q(x) = - 2 \ln \mathcal{L}(x,\hat{\hat{\theta}}(x))/\mathcal{L}(\hat{x},\hat{\nu}),$$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't notice this before, but I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, should be \nu instead of \theta and we can use \vec{\mu} as elsewhere instead of "x" - will fix this
docs/part3/commonstatsmethods.md
Outdated
|
||
There is a tool for extracting *2D contours* from the output of `HybridNew` located in `test/makeFCcontour.py`. This can be used provided the option `--saveHybridResult` was included when running `HybridNew`. It can be run with the usual <span style="font-variant:small-caps;">Combine</span> output files (or several of them) as input, | ||
For *two-dimensional* models, or if the parameter does not behave like a cross section, you will need to extract the contours from the output of `HybridNew` and plot them yourself. We will use the `toy-hgg-125.txt` datacard in the example below to demonstrate how this can be done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to explicitly include the path to toy-hgg-125.txt
datacard and to also provide the explicit text2workspace
command used here (since they have to also include the multiparameter options), just to save people the time of having to figure out these details when they are try to check this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, the instructions are already further up the page as its the same as for the multidimfit 2D example, but can repeat the instructions here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I think either repeating them, or linking to those instructions would be fine. Though maybe repeating is a bit easier in the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree - done (repeated instructions)
docs/part3/commonstatsmethods.md
Outdated
To extract 2D contours, the names of each parameter must be given `--xvar poi_x --yvar poi_y`. The output will be a ROOT file containing a 2D histogram of value of $p_{x,y}$ for each point $(x,y)$ which can be used to draw 2D contours. There will also be a histogram containing the number of toys found for each point. | ||
Once this is done, we extract the contours using | ||
```sh | ||
combineTool.py -M HybridNewGrid ./grid.json -d toy-hgg-125.root --task-name fc2d --job-mode 'interactive' --cycles 0 --output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about adding, here or above, a brief description of what --cycles 1
and --cycles 0
are doing? It was a little cryptic to me at first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me too :) @ajgilbert might be able to suggest something, but once the combineTool moves into combine I hope it comes with more documentation (and cycles would be included there I assume)
``` | ||
|
||
To extract 2D contours, the names of each parameter must be given `--xvar poi_x --yvar poi_y`. The output will be a ROOT file containing a 2D histogram of value of $p_{x,y}$ for each point $(x,y)$ which can be used to draw 2D contours. There will also be a histogram containing the number of toys found for each point. | ||
Once this is done, we extract the contours using | ||
```sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting an error when I try to run this about 'plot_settings' not being a key in config
and then, if I fix that manually, another error about indexing opts
. Both are coming from PlotTestStat
in
/work/kcormier/CAT/combine_tutorial/run_preexercices/CMSSW_11_3_4/python/CombineHarvester/CombineTools/combine/LimitGrids.py
Is it me, did I make a mistake in the setup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran this again and seems to work fine (we don't need the line about making plots in the JSON so i removed it but I don't think it will make a difference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, found it - needed to specifically include the line "make_plots": false,
in the json. Just checked out a clean area and ran the commands to verify everything works now.
@kcormi - are we ok to merge this now? There's no change to the combine code. These instructions work with v9.2.0 but are improved wrt the original script as they make use of combineTool |
Yes, thanks Nick! Sorry for being a little slow on the merge, this is a nice improvement. |
As in the title - added a nice example for running and plotting a FC calculation (here is the 2D Hgg toy card) in the docs using combineTool
Also removed output for 2D NLL scan (not sure its very helpful).