[RF] Use std::abs all the way in RooFit
#11986
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As noted in d008860, using `abs()` without `std::` can be very dangerous because there might be no overload for floating point numbers. To make sure that no bugs associated to this can happen, I suggest to avoid using `abs()` without `std::` all the way in RooFit. Also, to go with modern C++ all the way, the occurences of `fabs()` and `std::fabs()` are also replaced with `std::abs()`.
|
Starting build on |
52 tasks
|
Build failed on ROOT-ubuntu18.04/nortcxxmod. Warnings:
And 60 more Failing tests:
|
|
Build failed on mac12/noimt. Warnings:
And 53 more Failing tests:
|
|
Build failed on ROOT-performance-centos8-multicore/cxx17. Warnings:
And 130 more Failing tests:
|
|
Build failed on mac11/cxx14. Warnings:
And 54 more Failing tests:
|
lmoneta
approved these changes
Jan 9, 2023
Member
lmoneta
left a comment
lmoneta
left a comment
There was a problem hiding this comment.
LGTM!
We should probably make sure that the header <cmath> is included where std::abs is used.
Contributor
Author
|
Yes you're right. It's probably alright because the CI is passing, but I'll verify that later |
|
Build failed on ROOT-ubuntu2004/python3. Errors:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As noted in d008860, using
abs()withoutstd::can be very dangerous because there might be no overload for floating point numbers.To make sure that no bugs associated to this can happen, I suggest to avoid using
abs()withoutstd::all the way in RooFit.Also, to go with modern C++ all the way, the occurences of
fabs()andstd::fabs()are also replaced withstd::abs().