-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add Cruijff
PDF
#64
Conversation
@jonas-eschle FYI this is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, minor things, but other than that it's fine. A pity that no analytic integral is available, but fine, the numeric integrator catches it well. I'll approve, b
I think there is just no anti-derivative. I tried with some integral calculators and got nowhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the suggestion to refactor. I think it makes sense? If so, just apply the suggestion and the PR is approved
@jonas-eschle there was incorrect indentation in your suggestion but I fixed it. All good now, tests pass. Will merge as soon as ci completes. |
yep, just noticed, thanks for the fix |
Adds
Cruijff
PDF as unnormalized densityRelated to zfit/zfit#512
Proposed Changes
Cruijff
PDF as unnormalized densityTests added
tests/test_pdf_cruijff.py
Checklist