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

comparison key accept callables in addition to named strings #174

Open
kratsg opened this issue Aug 28, 2024 · 3 comments
Open

comparison key accept callables in addition to named strings #174

kratsg opened this issue Aug 28, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@kratsg
Copy link

kratsg commented Aug 28, 2024

There are many many different calculations one could put into the ratio plot, and while it's great to have the flexibility and configurability of named strings such as comparison="pull" , it would additional be great to have something like comparison=lambda h1,h2: h1/h2 supported at the minimum, and one could also extend it to work for N histograms along the lines of comparison=lambda h1,h2,h3: (h1/h2, h2/h3) where the return value is a histogram or an iterable of histograms to plot.

@0ctagon 0ctagon added the enhancement New feature or request label Aug 28, 2024
@cyrraz
Copy link
Owner

cyrraz commented Aug 29, 2024

Interesting suggestion, thank you!

One difficulty that I see is the error propagation. Currently, we manually compute the uncertainty in the code for the comparisons we support. As a first approach, we could ask the user to provide the uncertainty in case they do not use a named string.

Regarding comparison=lambda h1,h2,h3: (h1/h2, h2/h3), I do not think we need it as we already have plot_comparison() that only plots the result of the comparison between two histograms. So in the example you give, the user would simply need to call plot_comparison() as many times as the number of comparisons they want. See this example.

@kratsg
Copy link
Author

kratsg commented Aug 29, 2024

One difficulty that I see is the error propagation. Currently, we manually compute the uncertainty in the code for the comparisons we support. As a first approach, we could ask the user to provide the uncertainty in case they do not use a named string.

you can rely on autodiffability to compute a forward or backward diff as long as the histograms are nodes in a computational graph. This part I would consider trivial and likely a timesaver as you can then do arbitrary calculations and propagate through.

Regarding comparison=lambda h1,h2,h3: (h1/h2, h2/h3), I do not think we need it as we already have plot_comparison() that only plots the result of the comparison between two histograms. So in the example you give, the user would simply need to call plot_comparison() as many times as the number of comparisons they want. See this example.

Perhaps, but it would also be important to both allow multiple ratio plots as well as overlaying them on the same ratio plot.

@cyrraz
Copy link
Owner

cyrraz commented Aug 30, 2024

you can rely on autodiffability to compute a forward or backward diff as long as the histograms are nodes in a computational graph. This part I would consider trivial and likely a timesaver as you can then do arbitrary calculations and propagate through.

You are right, automatic differentiation would definitely be the way to go. I just need to think more about the case of asymmetrical uncertainties. Another thing that we will need to carefully document is that the histograms will be assumed uncorrelated. We have the named strings "ratio" and "efficiency", that both correspond to h1/h2, but different kinds of correlations.

Perhaps, but it would also be important to both allow multiple ratio plots as well as overlaying them on the same ratio plot.

Actually, plot_comparison() takes an ax as an argument, so the user can overlay as many comparisons as they want on a single ratio plot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants