Nckw add example teststat plots #845
Conversation
|
|
||
| !!! info | ||
| If you used the TEV or LEP style test statistic (using the commands as described above), then you should include the option `--doublesided`, which will also take care of defining the correct integrals for $p_{\mu}$ and $p_{b}$. | ||
| If you used the TEV or LEP style test statistic (using the commands as described above), then you should include the option `--doublesided`, which will also take care of defining the correct integrals for $p_{\mu}$ and $p_{b}$. Click on the examples below to see what a typical output of this plotting tool will look like when using the LHC test statistic, or TEV test statistic. |
There was a problem hiding this comment.
Not necessarily for this PR, but I wonder if it is worth making this behaviour default, based on the test stat that is being used, rather than requiring the user to do it manually?
There was a problem hiding this comment.
The reason I would leave this option open is that we did have a workflow that was specifically for spin Hypotheses and iirc it was not quite the same as using the TeV test stat but was double sided. Honestly, if you don't use the option when you should, you'll see pretty quickly from the resulting figure that you should have used it. In any case, this script doesn't know about the test stat but I think what you're saying is you we could pass the test-stat in as an option and have the plot figure it out?
test/plotTestStatCLs.py
Outdated
| @@ -42,6 +43,13 @@ | |||
| type="str", | |||
| help="poi values, comma separated (type all to make a plot for every value found in the file)", | |||
| ) | |||
| parser.add_option( | |||
| "", | |||
| "--sub_label", | |||
There was a problem hiding this comment.
I'm not sure if there is an existing convention for some of this combine scripts already. But (at least with recent versions) of option/argument parser, using --some-command-line-option name will automatically produce the variable using underscores rather than -, i.e. some_command_line_option in this case. Which I think is just a little more user friendly, as it is a more often used convention.
(Same comment below)
There was a problem hiding this comment.
No strong feelings here so can go with hyphens instead
Modify options to use hyphens instead of underscore in `plotTestStatCLs.py` Added a warning to user if it looks like `--doublesided` option should be used and hasn't been used.
|
@kcormi - latest commit is an attempt to address your comments. Let me know what you think |
|
Looks like I spoke too soon, the figures seem to appear inside the show/hide box in the preview on GitHub but don't seem to work on the docs page - any ideas? |
|
Did you try a relative path (and maybe even the include syntax used elsewhere?), e.g.
I have a vague recollection that relative/absolute paths caused some problems at least with page links, so maybe worth a try? |
|
It could be an issue with using the absolute path, I had issues with the paths being treated slightly differently in a local test and in a deployment previously. When I inspect other images on that page in the browser, they are given by relative paths "../images/<img_name>.png", whereas, these ones are using the absolute path with "/docs" as the root. |
|
Yeah for a standard image, the relative path is correct but this doesn't seem to work inside a |
|
I set up a similar github.io page on my own github account for testing some of the other documentation updates. I can try using that one to make some tests and see what works. |
|
Also, if its related to the <details> block, I wonder if it is handled properly with some of the updates that will come in as part of #839. There I added the block style. I wonder if markdown then handles the image stuff properly. I can also check when I run a few tests shortly. |
|
Right, if you can check whether using the pymdownx extension works, i'd
rather swap to using that here too. There's probably a few other places
where we might want to hide figures for ease of reading but if we get this
change in, that will make it easier to do. Thanks!
…On Thu, Jul 6, 2023 at 10:37 AM kcormi ***@***.***> wrote:
Also, if its related to the
block, I wonder if it is handled properly with some of the updates that
will come in as part of #839
<#839>.
There I added the pymdownx.blocks.details markdown extension, to make use
of the
/// details | header name
contents
///
block style. I wonder if markdown then handles the image stuff properly. I
can also check when I run a few tests shortly.
—
Reply to this email directly, view it on GitHub
<#845 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMEVW5ETZVYG7MW3I2TTITXO2BL5ANCNFSM6AAAAAAZZ3SYDA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***
com>
|
|
It seems to work using the details block extension: https://kcormi.github.io/HiggsAnalysis-CombinedLimit/part3/commonstatsmethods/#complex-models code is here: It seems also possible to do it without the extension, but I also had to set the width explicitly in the img html block, otherwise it displayed very poorly. The code: Also, for testing purposes, the behaviour I got using a local mkdocs build and just checking it in my browser seemed to reproduce the same results as the actual deployed page. I guess this makes sense that it is slightly different behaviour than what the github preview gives, both using markdown, but I guess with slightly different details. Instructions on setting up a local environment for testing the documentation is in the contributing guide: |
|
Actually, the local mkdocs build also works with absolute paths, even though it doesn't on deployment. So that is one issue the local test won't catch. Hopefully its the only one, I'll add a note about that in the contributing guide. |
|
Thanks Kyle - can we just add the extension to the main branch and use your
version of the code?
…On Thu, Jul 6, 2023 at 11:20 AM kcormi ***@***.***> wrote:
Actually, the local mkdocs build also works with absolute paths, even
though it doesn't on deployment. So that is one issue the local test won't
catch. Hopefully its the only one, I'll add a note about that in the
contributing guide.
—
Reply to this email directly, view it on GitHub
<#845 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMEVW3FQRBWO62HHXLWUGTXO2GP7ANCNFSM6AAAAAAZZ3SYDA>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***
com>
|
Add example plots of test-stat distributions in part explaining limit calculation