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

If give limits_func in FFTConvPDFV1 allow to not include leakge #453

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

tinnuche
Copy link

@tinnuche tinnuche commented Mar 9, 2023

When a user specifies in FFTConvPDFV1 the arg limits_func , leakage is still added. This commit will remove this.

leakage here refers to the following: if I have a pdf with a range ( 10 , 20 ) and a kernel with a range ( -5 , 5 ), FFTConvPDFV1 will form a set of values from which to interpolate by convovling the kernel with the func but convolving the func over the range: ( 10 - 5 , 20 + 5 ). This may or may not be wanted, and ideally would be optional.

User may want functionality to specify limits_func AND include leakage. Two options: either we make it clear to do this you have to define in the obs of your func pdf. Or add an extra arg (i.e., include_leakage with default value True) which would work as follows:

limits_func = None & include_leakage = False : take obs limits of func and do not include leakage
limits_func = None & include_leakage = True : take obs limits of func and account for leakge
limits_func = ( lo , hi ) & include_leakage = False : take limits_func as limits and do not include leakage
limits_func = ( lo , hi ) & include_leakage = True : take limits_func as limits and account for leakge

There are 3 other changes I would like to make to FFTConvPDFV1 but will make those seperate pull requests:

  • smaller func range than kernel functionality ( either manual swap of two (make sure that works) or automatic in FFTConvPDFV1)
  • obs of func and kernel different to output FFTConvPDFV1 pdf
  • add documentation for FFTConvPDFV1

@tinnuche tinnuche changed the title limits_func doesn't add leakage If give limits_func in FFTConvPDFV1 can not include leakge Mar 9, 2023
@tinnuche tinnuche changed the title If give limits_func in FFTConvPDFV1 can not include leakge If give limits_func in FFTConvPDFV1 allow to not include leakge Mar 9, 2023
@jonas-eschle
Copy link
Contributor

When a user specifies in FFTConvPDFV1 the arg limits_func , leakage is still added. This commit will remove this.

leakage here refers to the following: if I have a pdf with a range ( 10 , 20 ) and a kernel with a range ( -5 , 5 ), FFTConvPDFV1 will form a set of values from which to interpolate by convovling the kernel with the func but convolving the func over the range: ( 10 - 5 , 20 + 5 ). This may or may not be wanted, and ideally would be optional.

Thanks a lot for this and the thoughts your putting into it!
This is an interesting one: I do agree that, given that we allow the limits as an explicit argument, this maybe needs to be respected. That's true.

But then, I am not sure when this would not be wanted? I don't think actually this is optional and rather an implementation detail.
To elaborate: he convolution is defined from -inf to inf. Due to numerics and limitations, we cannot go from -inf to inf. So we want to go from the closest, reasonable space, which is currently the funclower + kernellower.

However, it would be quite an advanced option, as I think in every case you will have something like funclower+kernellower. Or is there a use-case where you can think that one maybe really needs to limit this? I think usually it's fine because technically the conv goes -inf to inf, right?

Or is there maybe something else hidden, i.e. another problem that we maybe can solve more directly?

@tinnuche
Copy link
Author

@jonas-eschle This functionality will not be used used often I suspect.

The example where it might be used is for when one wants to approximate a kernel that varies continously in space ( say the shape of the q2 resolution func depends on the true q2 ) by splitting up the func into regions, convolving each with a different kernel ( one kernel for each func region for example) and then add the convolved pdf together afterwards. If you did that but had the 'leakage' im describing you would end up with some double counting. to do this addition safely also requires a bit of work but that can come later.

This is a niche case, but it does come up. As you say a convolution goes from -inf to inf. In the use-case I describe what one wants to do is convolve a theory func with the kernel and have the theory func be exactly 0 outside of some range, which hopefully gives the same result as this pull request.

Adding this functionality allows the easy implementation of more complex models. For my own work, I want to have a zfit implementation of the B -> K e e q2 model including efficency, resolution, and backgrounds. To do in zfit in a neat way requires some added functionality, including what I am trying to add with this pull request.

My aim was to add this functionality in such a way that it means most users never need to think about it, unless they specifically want to use it, so that for most users it's like the functionality isn't there.

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.

None yet

2 participants