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

Fix test failure on ppc64le and aarch64 with gcc 12 #9601

Closed
wants to merge 1 commit into from

Conversation

ellert
Copy link
Contributor

@ellert ellert commented Jan 17, 2022

This Pull request:

Changes or fixes:

.../hist/hist/test/test_tprofile2poly.cxx:61: Failure
The difference between cont1 and cont2 is 1.4551915228366852e-11, which exceeds delta, where
cont1 evaluates to 54886.064319363642,
cont2 evaluates to 54886.064319363628, and
delta evaluates to 9.999999960041972e-12.
.../hist/hist/test/test_tprofile2poly.cxx:61: Failure
The difference between cont1 and cont2 is 1.4551915228366852e-11, where
cont1 evaluates to 109868.61342004745,
cont2 evaluates to 109868.61342004743.
The abs_error parameter delta evaluates to 9.999999960041972e-12 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 1.4551915228366852e-11, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

.../hist/hist/test/test_tprofile2poly.cxx:61: Failure
The difference between cont1 and cont2 is 1.4551915228366852e-11, which exceeds delta, where
cont1 evaluates to 54886.064319363642,
cont2 evaluates to 54886.064319363628, and
delta evaluates to 9.999999960041972e-12.
.../hist/hist/test/test_tprofile2poly.cxx:61: Failure
The difference between cont1 and cont2 is 1.4551915228366852e-11, where
cont1 evaluates to 109868.61342004745,
cont2 evaluates to 109868.61342004743.
The abs_error parameter delta evaluates to 9.999999960041972e-12 which is smaller than the minimum distance between doubles for numbers of this magnitude which is 1.4551915228366852e-11, thus making this EXPECT_NEAR check equivalent to EXPECT_EQUAL. Consider using EXPECT_DOUBLE_EQ instead.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

@ellert
Copy link
Contributor Author

ellert commented Dec 13, 2022

Now passes all tests.

@ellert
Copy link
Contributor Author

ellert commented Feb 28, 2023

Still happens in 6.28.00.

@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from eguiraud Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
@root-project root-project deleted a comment from phsft-bot Nov 14, 2023
guitargeek added a commit to guitargeek/root that referenced this pull request Nov 14, 2023
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.
@guitargeek
Copy link
Contributor

Hi! I'm not so happy with the uncommented magic factor of two :( Can we maybe solve the problem like this?

guitargeek added a commit that referenced this pull request Nov 23, 2023
As seen in #9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.
@guitargeek
Copy link
Contributor

Hi @ellert, since there was no reply I just went ahead with merging the other PR. But if there are still issues, feel free to re-open this PR, create a new PR or open a GitHub issue about it!

Thanks,
Jonas

@guitargeek guitargeek closed this Nov 23, 2023
guitargeek added a commit to guitargeek/root that referenced this pull request Nov 24, 2023
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.
guitargeek added a commit to guitargeek/root that referenced this pull request Nov 24, 2023
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.

In some cases, `EXPECT_NEAR` is still used to make the test pass on all
platforms, but with a relative tolerance.
guitargeek added a commit to guitargeek/root that referenced this pull request Nov 24, 2023
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.

In some cases, `EXPECT_NEAR` is still used to make the test pass on all
platforms, but with a relative tolerance.
guitargeek added a commit to guitargeek/root that referenced this pull request Nov 27, 2023
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.

In some cases, `EXPECT_NEAR` is still used to make the test pass on all
platforms, but with a relative tolerance.
guitargeek added a commit that referenced this pull request Nov 27, 2023
As seen in #9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.

In some cases, `EXPECT_NEAR` is still used to make the test pass on all
platforms, but with a relative tolerance.
@andriish
Copy link
Contributor

andriish commented Dec 11, 2023

Hi @guitargeek , the factor is not more magic than the delta itself and different architectures have different precision.
Could we have this MR in?

maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.
maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024
As seen in root-project#9601, using `EXCEPT_NEAR` with an absolute error is not so
good, because if the numbers are large the hardcoded absolute error can
be smaller than the double precision. In these cases, `EXPECT_DOUBLE_EQ`
is recommended instead.

In some cases, `EXPECT_NEAR` is still used to make the test pass on all
platforms, but with a relative tolerance.
@ellert ellert deleted the adjust-delta branch April 11, 2024 14:58
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

5 participants