Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add optional range for channels in ChannelCompatibilityCheck #760

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

IzaakWN
Copy link
Contributor

@IzaakWN IzaakWN commented May 23, 2022

With these changes, the user can optionally set the range of channel POIs as

combine -M ChannelCompatibilityCheck workspace.root -g chan1 -g chan2=-10,20 ...

This is useful in the case the POI hit the boundary in some channels, e.g. due to low sensitivity, large excess, ...

Alternatively, I also tried adding the channel POIs to autoMaxPOIs_ or autoBoundsPOIs_ by adding this snippet to ChannelCompatibilityCheck::runSpecific. This helped with boundary issues, but it's a bit ugly, and you get an error for e.g.

combine -M ChannelCompatibilityCheck workspace.root -g chan1 -g chan2 --autoMaxPOIs r,_ChannelCompatibilityCheck_r_chan2 ...
...
[#0] ERROR:InputArguments --  RooWorkspace::argSet(w) no RooAbsArg named "_ChannelCompatibilityCheck_r_chan2" in workspace

because this parameter does not exist before ChannelCompatibilityCheck::runSpecific.

@amarini amarini mentioned this pull request Oct 18, 2022
@nucleosynthesis
Copy link
Contributor

can we point this PR to main instead of 102x? I don't see why this can't be merged once thats done.

@IzaakWN IzaakWN changed the base branch from 102x to main October 10, 2023 09:45
@IzaakWN
Copy link
Contributor Author

IzaakWN commented Oct 10, 2023

Hi @nucleosynthesis, yes of course, done. However, I haven't validated it in this new branch.

@IzaakWN
Copy link
Contributor Author

IzaakWN commented Oct 10, 2023

Apologies for the noise, did you want to do a separate PR to main in addition to 102X? I just changed the target branch of this PR to main.

@nucleosynthesis
Copy link
Contributor

No, I meant just change the target, as you have done. Thanks!

@nucleosynthesis
Copy link
Contributor

Just tested and this works in main. The user could already change the ranges for all parameters using the --rMin, --rMax options but this indeed adds extra flexibility to change only some parameters in the split channel model.

@nucleosynthesis nucleosynthesis merged commit 965fc6a into cms-analysis:main Oct 10, 2023
@IzaakWN
Copy link
Contributor Author

IzaakWN commented Oct 10, 2023

Just tested and this works in main. The user could already change the ranges for all parameters using the --rMin, --rMax options but this indeed adds extra flexibility to change only some parameters in the split channel model.

Yeah, agreed. This was in context of EXO-19-016 where there the signal sensitivity varied greatly between different signal regions, depending on the model parameters, which caused some fit instability / problems with convergence.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants