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

Correctly use Systematic Groups in BinnedNLLH::AddPDFs #58

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

willp240
Copy link

@willp240 willp240 commented Dec 6, 2024

Oof here's a bug and a half. In our BinnedNLLH teststat we have a few overloaded AddPDF and AddPDFs methods. When using AddPDFs, if you pass the systematic groups as an argument, it currently doesn't use the corresponding AddPDF with the syst groups as an argument.

Looks like I introduced it here and it made it through the great merge of 2022!

Copy link

@dcookman dcookman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot!
Compiles & passes unit tests (tbf we don't currently have unit tests for this combination of circumstances, so not surprising).
Mercifully this bug will only have impacted those who were trying to use BinnedNLLH by adding multiple PDFs at once, with systematics AND Beeston-Barlow.

@dcookman dcookman added the merge soon to be merged soon if no objections label Dec 20, 2024
@dcookman dcookman merged commit 999ae11 into snoplus:master Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge soon to be merged soon if no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants