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

TST: adjust other very slow tests #20487

Merged
merged 10 commits into from Apr 30, 2024
2 changes: 2 additions & 0 deletions scipy/_lib/tests/test_import_cycles.py
@@ -1,3 +1,4 @@
import pytest
import sys
import subprocess

Expand All @@ -7,6 +8,7 @@
# Check that all modules are importable in a new Python process.
# This is not necessarily true if there are import cycles present.

@pytest.mark.slow
def test_public_modules_importable():
pids = [subprocess.Popen([sys.executable, '-c', f'import {module}'])
for module in PUBLIC_MODULES]
Expand Down
1 change: 1 addition & 0 deletions scipy/integrate/tests/test_quadpack.py
Expand Up @@ -342,6 +342,7 @@ def simpfunc(z, y, x, t): # Note order of arguments.
(2.,)),
2*8/3.0 * (b**4.0 - a**4.0))

@pytest.mark.xslow
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are plenty of tests for 2D improper integrals, and there is other coverage of 3D integrals. Both of these functions just call nquad, so I think it's safe enough to run these only selectively, like when work is done on the functions (rare).

@pytest.mark.parametrize(
"x_lower, x_upper, y_lower, y_upper, z_lower, z_upper, expected",
[
Expand Down
2 changes: 1 addition & 1 deletion scipy/interpolate/tests/test_gil.py
Expand Up @@ -28,7 +28,7 @@ def run(self):

return WorkerThread()

@pytest.mark.slow
@pytest.mark.xslow
Copy link
Member

Choose a reason for hiding this comment

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

This one seems fine to mark as xslow indeed. Kinda pointless to run really slow tests that are marked as xfail.

@pytest.mark.xfail(reason='race conditions, may depend on system load')
def test_rectbivariatespline(self):
def generate_params(n_points):
Expand Down
2 changes: 1 addition & 1 deletion scipy/interpolate/tests/test_rgi.py
Expand Up @@ -476,7 +476,7 @@ def f(x, y):
])
def test_descending_points_nd(self, method, ndims, func):

if ndims == 5 and method in {"cubic", "quintic"}:
if ndims >= 4 and method in {"cubic", "quintic"}:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Plenty of other coverage.

pytest.skip("too slow; OOM (quintic); or nearly so (cubic)")

rng = np.random.default_rng(42)
Expand Down
2 changes: 1 addition & 1 deletion scipy/optimize/tests/test__differential_evolution.py
Expand Up @@ -1339,7 +1339,7 @@ def c1(x):
assert_(np.all(res.x >= np.array(bounds)[:, 0]))
assert_(np.all(res.x <= np.array(bounds)[:, 1]))

@pytest.mark.slow
@pytest.mark.xslow
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just one of many tests. This one happens to be very slow, and it fails on some platforms, so probably no harm in making it xslow.

@pytest.mark.xfail(platform.machine() == 'ppc64le',
reason="fails on ppc64le")
def test_L8(self):
Expand Down
1 change: 0 additions & 1 deletion scipy/optimize/tests/test_extending.py
Expand Up @@ -5,7 +5,6 @@

from scipy._lib._testutils import _test_cython_extension, cython


@pytest.mark.skipif(platform.machine() in ["wasm32", "wasm64"],
mdhaber marked this conversation as resolved.
Show resolved Hide resolved
reason="Can't start subprocess")
@pytest.mark.skipif(cython is None, reason="requires cython")
Expand Down
129 changes: 66 additions & 63 deletions scipy/optimize/tests/test_minimize_constrained.py
Expand Up @@ -2,7 +2,7 @@
import pytest
from scipy.linalg import block_diag
from scipy.sparse import csc_matrix
from numpy.testing import (TestCase, assert_array_almost_equal,
from numpy.testing import (assert_array_almost_equal,
assert_array_less, assert_, assert_allclose,
suppress_warnings)
from scipy.optimize import (NonlinearConstraint,
Expand Down Expand Up @@ -443,67 +443,70 @@ def hess(x, v):
return NonlinearConstraint(fun, -np.inf, 0, jac, hess)


class TestTrustRegionConstr(TestCase):

@pytest.mark.slow
def test_list_of_problems(self):
list_of_problems = [Maratos(),
Maratos(constr_hess='2-point'),
Maratos(constr_hess=SR1()),
Maratos(constr_jac='2-point', constr_hess=SR1()),
MaratosGradInFunc(),
HyperbolicIneq(),
HyperbolicIneq(constr_hess='3-point'),
HyperbolicIneq(constr_hess=BFGS()),
HyperbolicIneq(constr_jac='3-point',
constr_hess=BFGS()),
Rosenbrock(),
IneqRosenbrock(),
EqIneqRosenbrock(),
BoundedRosenbrock(),
Elec(n_electrons=2),
Elec(n_electrons=2, constr_hess='2-point'),
Elec(n_electrons=2, constr_hess=SR1()),
Elec(n_electrons=2, constr_jac='3-point',
constr_hess=SR1())]

for prob in list_of_problems:
for grad in (prob.grad, '3-point', False):
for hess in (prob.hess,
'3-point',
SR1(),
BFGS(exception_strategy='damp_update'),
BFGS(exception_strategy='skip_update')):

# Remove exceptions
if grad in ('2-point', '3-point', 'cs', False) and \
hess in ('2-point', '3-point', 'cs'):
continue
if prob.grad is True and grad in ('3-point', False):
continue
with suppress_warnings() as sup:
sup.filter(UserWarning, "delta_grad == 0.0")
result = minimize(prob.fun, prob.x0,
method='trust-constr',
jac=grad, hess=hess,
bounds=prob.bounds,
constraints=prob.constr)

if prob.x_opt is not None:
assert_array_almost_equal(result.x, prob.x_opt,
decimal=5)
# gtol
if result.status == 1:
assert_array_less(result.optimality, 1e-8)
# xtol
if result.status == 2:
assert_array_less(result.tr_radius, 1e-8)

if result.method == "tr_interior_point":
assert_array_less(result.barrier_parameter, 1e-8)
# max iter
if result.status in (0, 3):
raise RuntimeError("Invalid termination condition.")
class TestTrustRegionConstr:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simply converted the for loops to parametrize.

Copy link
Member

Choose a reason for hiding this comment

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

Fine to merge this, but in the future I'd avoid this since it's a very large diff and doesn't actually change the total runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's much smaller and easier to read with "Hide whitespace changes" (mostly indentation changes), and it allowed me to see that no tests are particularly slow. Most are under 0.05s; there are just a lot of them. If we want to speed things up, how about flipping a coin like in the lobpcg tests?

list_of_problems = [Maratos(),
Maratos(constr_hess='2-point'),
Maratos(constr_hess=SR1()),
Maratos(constr_jac='2-point', constr_hess=SR1()),
MaratosGradInFunc(),
HyperbolicIneq(),
HyperbolicIneq(constr_hess='3-point'),
HyperbolicIneq(constr_hess=BFGS()),
HyperbolicIneq(constr_jac='3-point',
constr_hess=BFGS()),
Rosenbrock(),
IneqRosenbrock(),
EqIneqRosenbrock(),
BoundedRosenbrock(),
mdhaber marked this conversation as resolved.
Show resolved Hide resolved
Elec(n_electrons=2),
Elec(n_electrons=2, constr_hess='2-point'),
Elec(n_electrons=2, constr_hess=SR1()),
Elec(n_electrons=2, constr_jac='3-point',
constr_hess=SR1())]

@pytest.mark.parametrize('prob', list_of_problems)
@pytest.mark.parametrize('grad', ('prob.grad', '3-point', False))
@pytest.mark.parametrize('hess', ("prob.hess", '3-point', SR1(),
BFGS(exception_strategy='damp_update'),
BFGS(exception_strategy='skip_update')))
def test_list_of_problems(self, prob, grad, hess):
grad = prob.grad if grad == "prob.grad" else grad
hess = prob.hess if hess == "prob.hess" else hess
# Remove exceptions
if (grad in {'2-point', '3-point', 'cs', False} and
hess in {'2-point', '3-point', 'cs'}):
pytest.skip("Numerical Hessian needs analytical gradient")
if prob.grad is True and grad in {'3-point', False}:
pytest.skip("prob.grad incompatible with grad in {'3-point', False}")
sensitive = (isinstance(prob, BoundedRosenbrock) and grad == '3-point'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Started getting a failure in just one case on just the Accelerate platform. I thought it was just a matter of the guess x0 changing, but that doesn't seem to be the case. Not sure what's happening, but it's not too concerning, so I'll just open an issue when this merges.

Copy link
Member

Choose a reason for hiding this comment

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

CI was skipped on the last push, so I'm not sure if this will start to fail if we merge now. That would not be helpful - can you check it's fixed, and if not either drop this whole change or try to resolve it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part of the diff skips the problematic test case so that CI would not fail anymore. The proposal is to merge the PR with the problematic test skipped, then I'll open a separate issue about the failure.
It doesn't seem like something that should hold up this PR - the failure appears to be caused by a minor change in how the test is configured, and it seems to be exposing an issue that has always been here; we might have just been lucky/unlucky not to see before.

Copy link
Member

Choose a reason for hiding this comment

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

This sounds good to me.

and isinstance(hess, BFGS))
if sensitive:
pytest.xfail("Seems sensitive to initial conditions w/ Accelerate")
with suppress_warnings() as sup:
sup.filter(UserWarning, "delta_grad == 0.0")
result = minimize(prob.fun, prob.x0,
method='trust-constr',
jac=grad, hess=hess,
bounds=prob.bounds,
constraints=prob.constr)

if prob.x_opt is not None:
assert_array_almost_equal(result.x, prob.x_opt,
decimal=5)
# gtol
if result.status == 1:
assert_array_less(result.optimality, 1e-8)
# xtol
if result.status == 2:
assert_array_less(result.tr_radius, 1e-8)

if result.method == "tr_interior_point":
assert_array_less(result.barrier_parameter, 1e-8)

# check for max iter
message = f"Invalid termination condition: {result.status}."
assert result.status not in {0, 3}, message


def test_default_jac_and_hess(self):
def fun(x):
Expand Down Expand Up @@ -641,7 +644,7 @@ def obj(x):

assert result['success']

class TestEmptyConstraint(TestCase):
class TestEmptyConstraint:
"""
Here we minimize x^2+y^2 subject to x^2-y^2>1.
The actual minimum is at (0, 0) which fails the constraint.
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/linalg/_eigen/lobpcg/tests/test_lobpcg.py
Expand Up @@ -533,7 +533,7 @@ def test_maxit():
assert_allclose(np.shape(r_h), np.shape(np.asarray(r_h)))


@pytest.mark.slow
@pytest.mark.xslow
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option would be to @parametrize rather than looping and reduce the number of cases.

Copy link
Member

Choose a reason for hiding this comment

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

Can also just reduce the number of cases if that helps.

@pytest.mark.parametrize("n", [15])
@pytest.mark.parametrize("m", [1, 2])
@pytest.mark.filterwarnings("ignore:Exited at iteration")
Expand Down
1 change: 1 addition & 0 deletions scipy/sparse/linalg/tests/test_propack.py
Expand Up @@ -89,6 +89,7 @@ def test_svdp(ctor, dtype, irl, which):
check_svdp(n, m, ctor, dtype, k, irl, which)


@pytest.mark.xslow
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 think I wrote this test, and I don't think there's anything very special about it; it's just one that happened to be distributed with PROPACK. We have other tests that don't take as long.

@pytest.mark.parametrize('dtype', _dtypes)
@pytest.mark.parametrize('irl', (False, True))
@pytest.mark.timeout(120) # True, complex64 > 60 s: prerel deps cov 64bit blas
Expand Down
2 changes: 1 addition & 1 deletion scipy/spatial/tests/test_qhull.py
Expand Up @@ -344,7 +344,7 @@ def test_degenerate_barycentric_transforms(self):
# Check the transforms
self._check_barycentric_transforms(tri)

@pytest.mark.slow
@pytest.mark.xslow
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The inclusion of "more" in the name suggests that it's not essential.

Copy link
Contributor

Choose a reason for hiding this comment

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

slow seems like the right call to me instead of xslow here. Checking locally:

python dev.py test -m full -t scipy/spatial/tests/test_qhull.py::TestUtilities::test_more_barycentric_transforms -- --durations=2

0.67s call build-install/lib/python3.11/site-packages/scipy/spatial/tests/test_qhull.py::TestUtilities::test_more_barycentric_transforms

That seems reasonable, under a second, and only when full is requested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fine. Note that the criterion for inclusion here was that these tests took >5s on my machine.

mdhaber marked this conversation as resolved.
Show resolved Hide resolved
def test_more_barycentric_transforms(self):
# Triangulate some "nasty" grids

Expand Down