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

[RF] RooTruthModel analytical integrals for general integration ranges #14021

Merged
merged 1 commit into from Nov 6, 2023

Conversation

guitargeek
Copy link
Contributor

The analytical integral code of the RooTruthModel was making the wrong assumption that if one uses the single-sided bases, the minimum x value is always at zero (or the maximum value at zero, for the case of the flipped bases).

This resulted in wrong integral values when integrating over a subrange, as reported here on the forum:

https://root-forum.cern.ch/t/possible-bug-in-integration-of-roobdecay-and-rooabsanaconvpdf/56968

This commit rewrites the RooTruthModel analytical integral code to also consider these cases. To avoid that with the additional code branches the code becomes too verbose, the code was refactored to use a helper function for evaluating indefinite integrals of symmetric or asymmetric basis functions.

The refactored code is tested by the integration tests in stressRooFit, and the problem that was reported on the forum is covered by a new unit test.

The analytical integral code of the `RooTruthModel` was making the wrong
assumption that if one uses the single-sided bases, the minimum x value
is always at zero (or the maximum value at zero, for the case of the
flipped bases).

This resulted in wrong integral values when integrating over a subrange,
as reported here on the forum:

https://root-forum.cern.ch/t/possible-bug-in-integration-of-roobdecay-and-rooabsanaconvpdf/56968

This commit rewrites the RooTruthModel analytical integral code to also
consider these cases. To avoid that with the additional code branches
the code becomes too verbose, the code was refactored to use a helper
function for evaluating indefinite integrals of symmetric or asymmetric
basis functions.

The refactored code is tested by the integration tests in `stressRooFit`,
and the problem that was reported on the forum is covered by a new unit
test.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac12arm/cxx20, windows10/default
How to customize builds

Copy link

github-actions bot commented Nov 6, 2023

Test Results

       11 files         11 suites   1d 22h 18m 58s ⏱️
  2 481 tests   2 335 ✔️ 0 💤 146
26 220 runs  26 073 ✔️ 0 💤 147

For more details on these failures, see this check.

Results for commit d12d1e0.

Copy link
Member

@lmoneta lmoneta left a comment

Choose a reason for hiding this comment

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

LGTM!

@guitargeek guitargeek merged commit 93ae731 into root-project:master Nov 6, 2023
10 of 16 checks passed
@guitargeek guitargeek deleted the rootruthmodel_1 branch November 6, 2023 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants