Skip to content
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

Force updateGrid in expectedFromGrid HybridNew.cc #941

Merged

Conversation

nucleosynthesis
Copy link
Contributor

if using --noUpdateGrid with --expectedFromGrid and the grid file was not produced using the same value for expectedFromGrid, then the limit will be identical to the result without --expectedFromGrid.

With this change, we now force an update if --expectedFromGrid option set. Usual workflow is to produce the grid (without this option) and then calculate expected results after, so this works with this workflow if user adds --noUpdateGrid

if using `--noUpdateGrid` with `--expectedFromGrid` and the grid file was not produced using the same value for expectedFromGrid, then the limit will be identical to the result without `--expectedFromGrid`. 

With this change, we now *force* an update if `--expectedFromGrid` option set. Usual workflow is to produce the grid (without this option) and then calculate expected results after, so this works with this workflow if user adds `--noUpdateGrid`
@nucleosynthesis
Copy link
Contributor Author

Added the needs work label since there could be a smarter way to do this, without having to resort to a full update at various points

@nucleosynthesis
Copy link
Contributor Author

Note, have also added a warning about this in the docs : 21ad9b0#diff-e1cf5d8257ea3082032f066028c25c4e5134426b497d6f3f40042e955177759dR585-R587

@nucleosynthesis
Copy link
Contributor Author

Slightly smarter now in that the update will only happen, even if user specifies --noUpdateGrid if the quantileExpected value from the grid file doesn't match the one being asked for (if expected, check against value in the file, if observed just compare with -1)

Not bullet proof but for normal workflows, should catch most issues

@nucleosynthesis
Copy link
Contributor Author

I think this one can be merged now, so if no objections, i'll do so soon

@nucleosynthesis nucleosynthesis merged commit c5f0dfa into main May 8, 2024
7 checks passed
@nucleosynthesis nucleosynthesis deleted the nucleosynthesis-fix-noUpdateGrid-with-expectedFromGrid branch June 7, 2024 17:11
kcormi pushed a commit to kcormi/HiggsAnalysis-CombinedLimit that referenced this pull request Jun 24, 2024
* Force updateGrid in expectedFromGrid HybridNew.cc

if using `--noUpdateGrid` with `--expectedFromGrid` and the grid file was not produced using the same value for expectedFromGrid, then the limit will be identical to the result without `--expectedFromGrid`. 

With this change, we now *force* an update if `--expectedFromGrid` option set. Usual workflow is to produce the grid (without this option) and then calculate expected results after, so this works with this workflow if user adds `--noUpdateGrid`

* Check if really necessary to update grid with expectedFromGrid based on quantileFrom gridfile

---------

Co-authored-by: Nick Wardle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant