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

Sum of weights for non-EFT process #27

Closed
bryates opened this issue Apr 20, 2024 · 5 comments · Fixed by TopEFT/topeft#411
Closed

Sum of weights for non-EFT process #27

bryates opened this issue Apr 20, 2024 · 5 comments · Fixed by TopEFT/topeft#411
Assignees
Labels
documentation Improvements or additions to documentation wontfix This will not be worked on

Comments

@bryates
Copy link
Contributor

bryates commented Apr 20, 2024

Forcing the storage type to Double

kwargs.setdefault("storage", "Double")

means we lose the sum of weights, even in the non-EFT samples. The data is simply sqrt(N), but the nonprompt leptons have an MC component subtracted off
https://github.com/TopEFT/topeft/blob/3ae0f05890a129aaa226efeaec949d13e1778468/topeft/modules/dataDrivenEstimation.py#L115-L129.
This means the nonprompt templates have no uncertainties, but we still ask combine to compute the MC stat errors. This is likely causing the remaining discrepancy in the limits when compared to the master branch on the TopEFT repo.

@bryates bryates added the bug Something isn't working label Apr 20, 2024
@bryates bryates self-assigned this Apr 20, 2024
@bryates bryates moved this to In progress in Calendar Coffea roadmap Apr 20, 2024
@bryates
Copy link
Contributor Author

bryates commented Apr 23, 2024

@btovar do you have any thoughts on this? Just changing the default type to Weight (and disabling the Double check) isn't enough. I think the way _ak_rec_op builds things is incompatible with Weight. As an alternatives, we could just store the sum of square of weights (at the SM) in a dict or another dense axis.

@bryates bryates pinned this issue Apr 23, 2024
@btovar
Copy link
Contributor

btovar commented Apr 24, 2024

@bryates I think that was the idea, to record it separately from that double axis. As I understood it, Weight is going to do the wrong thing for the EFT hists.

@bryates
Copy link
Contributor Author

bryates commented Apr 24, 2024

@bryates I think that was the idea, to record it separately from that double axis. As I understood it, Weight is going to do the wrong thing for the EFT hists.

Right, we don't really need it for the actual EFT histograms (sine it would be a quartic function), but we'll need it for the SM terms that are subtracted off later one. Do you know of a coinvent way to add another axis, or would it be easier (but maybe less efficient) to just save another histogram with the weights^2?

@btovar
Copy link
Contributor

btovar commented Apr 24, 2024

I think it would be much easier to have another histogram with the weights. The way the new histograms work make it hard to have special treatment of some axis.

@bryates
Copy link
Contributor Author

bryates commented Apr 24, 2024

I think it would be much easier to have another histogram with the weights. The way the new histograms work make it hard to have special treatment of some axis.

Ok thanks. I was starting to suspect that after trying to hack something in.

@bryates bryates added documentation Improvements or additions to documentation wontfix This will not be worked on and removed bug Something isn't working labels Apr 25, 2024
@bryates bryates linked a pull request May 22, 2024 that will close this issue
@bryates bryates closed this as completed May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation wontfix This will not be worked on
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants