8000 TST: Some fixes & refactoring around glibc-dependent skips in test_umath.py by h-vetinari · Pull Request #20274 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

TST: Some fixes & refactoring around glibc-dependent skips in test_umath.py #20274

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 16, 2021
Merged

TST: Some fixes & refactoring around glibc-dependent skips in test_umath.py #20274

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
01443e8
TST: turn glibc_older_than_2.17 into boolean instead of xfail
h-vetinari Nov 2, 2021
56268d5
TST: use existence of glibc-version to clean up a test
h-vetinari Nov 2, 2021
12923c2
TST: skip coverage of large elements in sincos_float32 for ancient glibc
h-vetinari Nov 2, 2021
22448b4
TST: parametrize glibc-version check in test_umath.py
h-vetinari Nov 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
8000
Diff view
33 changes: 14 additions & 19 deletions numpy/core/tests/test_umath.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ def get_glibc_version():


glibcver = get_glibc_version()
glibc_newerthan_2_17 = pytest.mark.xfail(
glibcver != '0.0' and glibcver < '2.17',
reason="Older glibc versions may not raise appropriate FP exceptions")
glibc_older_than = lambda x: (glibcver != '0.0' and glibcver < x)

Copy link
Member

Choose a reason for hiding this comment

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

The use of alphabetical order works, but it bothers me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to agree, but then, it was this way before already, and I didn't want to overengineer something for 3 tests...

What would you prefer? Comparing tuples? Using something like distutils.version.LooseVersion?

def on_powerpc():
""" True if we are running on a Power PC platform."""
Expand All @@ -50,14 +48,6 @@ def bad_arcsinh():
# The eps for float128 is 1-e33, so this is way bigger
return abs((v1 / v2) - 1.0) > 1e-23

if platform.machine() == 'aarch64' and bad_arcsinh():
skip_longcomplex_msg = ('Trig functions of np.longcomplex values known to be '
'inaccurate on aarch64 for some compilation '
'configurations, should be fixed by building on a '
'platform using glibc>2.17')
else:
skip_longcomplex_msg = ''


class _FilterInvalids:
def setup(self):
Expand Down Expand Up @@ -1022,9 +1012,11 @@ def test_exp_values(self):
yf = np.array(y, dtype=dt)
assert_equal(np.exp(yf), xf)

# Older version of glibc may not raise the correct FP exceptions
# See: https://github.com/numpy/numpy/issues/19192
@glibc_newerthan_2_17
@pytest.mark.xfail(
glibc_older_than("2.17"),
reason="Older glibc versions may not raise appropriate FP exceptions"
)
def test_exp_exceptions(self):
with np.errstate(over='raise'):
assert_raises(FloatingPointError, np.exp, np.float32(100.))
Expand Down Expand Up @@ -1405,8 +1397,10 @@ def test_sincos_float32(self):
M = np.int_(N/20)
index = np.random.randint(low=0, high=N, size=M)
x_f32 = np.float32(np.random.uniform(low=-100.,high=100.,size=N))
# test coverage for elements > 117435.992f for which glibc is used
x_f32[index] = np.float32(10E+10*np.random.rand(M))
if not glibc_older_than("2.17"):
# test coverage for elements > 117435.992f for which glibc is used
# this is known to be problematic on old glibc, so skip it there
x_f32[index] = np.float32(10E+10*np.random.rand(M))
x_f64 = np.float64(x_f32)
assert_array_max_ulp(np.sin(x_f32), np.float32(np.sin(x_f64)), maxulp=2)
assert_array_max_ulp(np.cos(x_f32), np.float32(np.cos(x_f64)), maxulp=2)
Expand Down Expand Up @@ -3439,13 +3433,14 @@ def check(x, rtol):
x_series = np.logspace(-20, -3.001, 200)
x_basic = np.logspace(-2.999, 0, 10, endpoint=False)

if dtype is np.longcomplex:
if glibc_older_than("2.19") and dtype is np.longcomplex:
Copy link
Member

Choose a reason for hiding this comment

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

"2.19" or "2.18"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what that would do? If it's older than 2.18, ít's definitely older than 2.19?

Copy link
Member
@charris charris Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I was looking at the older comment that it was OK for glibc > 2.17.

Copy link
Member

Choose a reason for hiding this comment

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

    skip_longcomplex_msg = ('Trig functions of np.longcomplex values known to be '
                            'inaccurate on aarch64 for some compilation '
                            'configurations, should be fixed by building on a '
                            'platform using glibc>2.17')

Copy link
Contributor Author
@h-vetinari h-vetinari Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For the if-branches here, the pertinent part (being deleted) is:

            # Can use 2.1 for > Ubuntu LTS Trusty (2014), glibc = 2.19.
            if skip_longcomplex_msg:
                pytest.skip(skip_longcomplex_msg)

I take from this comment that anything from glibc 2.19 onwards should be fine to take the else-clause (with the 2.1*eps). I cannot very (easily or quickly) how far back in terms of glibc-versions things would work, so I didn't try to change the version-boundaries as stated (or commented) in the code.

Copy link
Contributor Author
@h-vetinari h-vetinari Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

[...] should be fixed by building on a platform using glibc>2.17'

Right, I couldn't evaluate that corner case (2.18), so I went with the conservative choice, of still running the check (resp. the looser tolerance) with 2.18.

Before the last commit, I actually had this as glibc_older_than_2_17, but then realized that this would do the wrong thing (due to glibc > 2.17) for manylinux 2014.

Copy link
Member
@charris charris Nov 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

OK, probably not worth worrying about. Manylinux2014 itself is getting towards end of life.

if (platform.machine() == 'aarch64' and bad_arcsinh()):
pytest.skip("Trig functions of np.longcomplex values known "
"to be inaccurate on aarch64 for some compilation "
"configurations.")
# It's not guaranteed that the system-provided arc functions
# are accurate down to a few epsilons. (Eg. on Linux 64-bit)
# So, give more leeway for long complex tests here:
# Can use 2.1 for > Ubuntu LTS Trusty (2014), glibc = 2.19.
if skip_longcomplex_msg:
pytest.skip(skip_longcomplex_msg)
check(x_series, 50.0*eps)
else:
check(x_series, 2.1*eps)
Expand Down
0