Skip to content

Commit

Permalink
Resolve #412 (#428)
Browse files Browse the repository at this point in the history
* Add pytest option --level and corresponding stratification of tests in test_radon_svmbir.py

* Test level option determines alpha range

* Test level option determines dtype range

* Tests internally depend on level parameter

* Fix style violation

* Set test level on osx

* Add note on level semantics and set levels in CI tests

* Add note on level semantics and set levels in CI tests

* Move Diagonal tests to separate file

* Add pytest version requirement

* Update sbmodule

* Use new durations files with pytest-split

* Remove old test durations file

* Debugging attempt

* Debugging attempt

* Move pytest_addoption call to root conftest.py

* Restore --level options

* Remove secondary conftest.py

* Clean up

* Remove unused function

* Update docs

* Update developer list
  • Loading branch information
bwohlberg committed Jun 28, 2023
1 parent 5c7f6ba commit 523d662
Show file tree
Hide file tree
Showing 15 changed files with 341 additions and 3,559 deletions.
5 changes: 4 additions & 1 deletion .github/workflows/pytest_macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ jobs:
run: pip install -e .
# Run unit tests
- name: Run main unit tests
run: pytest -x --splits 5 --group ${{ matrix.group }}
run: |
DURATIONS_FILE=$(mktemp)
bzcat data/pytest/durations_macos.bz2 > $DURATIONS_FILE
pytest -x --durations-path $DURATIONS_FILE --splits 5 --group ${{ matrix.group }} --level 1
# Run doc tests
- name: Run doc tests
if: matrix.group == 1
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/pytest_ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ jobs:
run: pip install -e .
# Run unit tests
- name: Run main unit tests
run: pytest -x --cov --splits 5 --group ${{ matrix.group }}
run: |
DURATIONS_FILE=$(mktemp)
bzcat data/pytest/durations_ubuntu.bz2 > $DURATIONS_FILE
pytest -x --cov --durations-path $DURATIONS_FILE --splits 5 --group ${{ matrix.group }} --level 2
# Upload coverage data
- name: Upload coverage
uses: actions/upload-artifact@v3
Expand Down
3,225 changes: 0 additions & 3,225 deletions .test_durations

This file was deleted.

16 changes: 15 additions & 1 deletion conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
because `np` is used in doc strings for jax functions
(e.g. `linear_transpose`) that get pulled into `scico/__init__.py`.
Also allows `snp` to be used without explicitly importing
Also allow `snp` to be used without explicitly importing, and add
`level` parameter.
"""

import numpy as np
Expand All @@ -18,3 +19,16 @@ def add_modules(doctest_namespace):
"""Add common modules for use in docstring examples."""
doctest_namespace["np"] = np
doctest_namespace["snp"] = snp


def pytest_addoption(parser, pluginmanager):
"""Add --level pytest option.
Level definitions:
1 Critical tests only
2 Skip tests that do have a significant impact on coverage
3 All tests
"""
parser.addoption(
"--level", action="store", default=3, type=int, help="Set test level to be run"
)
2 changes: 1 addition & 1 deletion data
2 changes: 1 addition & 1 deletion dev_requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
pylint
pytest
pytest>=7.3.0
pytest-runner
packaging
pre-commit
Expand Down
14 changes: 10 additions & 4 deletions docs/source/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,20 @@ version of ``scico`` by
pytest --pyargs scico

When any significant changes are made to the test suite, the ``pytest-split`` test
time database file ``.test_durations`` in the repository root directory should be
updated
time database files in ``data/pytest`` should be updated using

::

pytest --store-durations
pytest --store-durations --durations-path data/pytest/durations_ubuntu --level 2

and the changes should be committed into the repository.
(for Ubuntu CI), and

::

pytest --store-durations --durations-path data/pytest/durations_macos --level 1

(for MacOS CI). These updated files should be bzipped and committed into the
``scico-data`` repository, replacing the current versions.


Test Coverage
Expand Down
11 changes: 8 additions & 3 deletions docs/source/team.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,18 @@ Developers
Core Developers
---------------

- `Cristina Garcia Cardona <https://github.com/crstngc>`_
- `Michael McCann <https://github.com/Michael-T-McCann>`_
- `Brendt Wohlberg <https://github.com/bwohlberg>`_


Emeritus Developers
-------------------

- `Thilo Balke <https://github.com/tbalke>`_
- `Fernando Davis <https://github.com/FernandoDavis>`_
- `Cristina Garcia Cardona <https://github.com/crstngc>`_
- `Soumendu Majee <https://github.com/smajee>`_
- `Michael McCann <https://github.com/Michael-T-McCann>`_
- `Luke Pfister <https://github.com/lukepfister>`_
- `Brendt Wohlberg <https://github.com/bwohlberg>`_


Contributors
Expand Down
2 changes: 1 addition & 1 deletion scico/flax/examples/data_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def generate_foam2_images(seed: float, size: int, ndata: int) -> Array:
foam = Foam2(size_range=[0.075, 0.0025], gap=1e-3, porosity=1)
saux[i, ..., 0] = discrete_phantom(foam, size=size)

# Normalize
# normalize
saux = saux / np.max(saux, axis=(1, 2), keepdims=True)

return saux
Expand Down
7 changes: 0 additions & 7 deletions scico/linop/radon_svmbir.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,3 @@ def __init__(
raise ValueError(
"Parameter is_masked must be False for the TomographicProjector in SVMBIRSquaredL2Loss."
)


def _unsqueeze(x: snp.Array, input_shape: Shape) -> snp.Array:
"""If x is 2D, make it 3D according to the SVMBIR convention."""
if len(input_shape) == 2:
x = x[snp.newaxis, :, :]
return x
40 changes: 23 additions & 17 deletions scico/test/functional/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,23 @@
from scico.random import randn

NO_BLOCK_ARRAY = [functional.L21Norm, functional.L1MinusL2Norm, functional.NuclearNorm]
NO_COMPLEX = [
functional.NonNegativeIndicator,
]
NO_COMPLEX = [functional.NonNegativeIndicator]


def pytest_generate_tests(metafunc):
level = int(metafunc.config.getoption("--level"))
alpha_range = [1e-2, 1e-1, 1e0, 1e1]
dtype_range = [np.float32, np.complex64, np.float64, np.complex128]
if level == 2:
alpha_range = [1e-2, 1e1]
dtype_range = [np.float32, np.complex64, np.float64]
elif level < 2:
alpha_range = [1e-2]
dtype_range = [np.float32, np.complex64]
if "alpha" in metafunc.fixturenames:
metafunc.parametrize("alpha", alpha_range)
if "test_dtype" in metafunc.fixturenames:
metafunc.parametrize("test_dtype", dtype_range)


class ProxTestObj:
Expand All @@ -31,9 +45,9 @@ def __init__(self, dtype):
self.vz = snp.zeros((3, 4), dtype=dtype)


@pytest.fixture(params=[np.float32, np.complex64, np.float64, np.complex128])
def test_prox_obj(request):
return ProxTestObj(request.param)
@pytest.fixture
def test_prox_obj(test_dtype):
return ProxTestObj(test_dtype)


class SeparableTestObject:
Expand All @@ -51,9 +65,9 @@ def __init__(self, dtype):
self.vb = snp.blockarray([self.v1, self.v2])


@pytest.fixture(params=[np.float32, np.complex64, np.float64, np.complex128])
def test_separable_obj(request):
return SeparableTestObject(request.param)
@pytest.fixture
def test_separable_obj(test_dtype):
return SeparableTestObject(test_dtype)


def test_separable_eval(test_separable_obj):
Expand Down Expand Up @@ -102,7 +116,6 @@ def __init__(self, delta=1.0):

class TestNormProx:

alphalist = [1e-2, 1e-1, 1e0, 1e1]
normlist = [
functional.L0Norm,
functional.L1Norm,
Expand All @@ -119,15 +132,13 @@ class TestNormProx:
normlist_blockarray_ready = set(normlist.copy()) - set(NO_BLOCK_ARRAY)

@pytest.mark.parametrize("norm", normlist)
@pytest.mark.parametrize("alpha", alphalist)
def test_prox(self, norm, alpha, test_prox_obj):
nrmobj = norm()
nrm = nrmobj.__call__
prx = nrmobj.prox
pf = prox_test(test_prox_obj.v, nrm, prx, alpha)

@pytest.mark.parametrize("norm", normlist)
@pytest.mark.parametrize("alpha", alphalist)
def test_conj_prox(self, norm, alpha, test_prox_obj):
nrmobj = norm()
v = test_prox_obj.v
Expand All @@ -137,7 +148,6 @@ def test_conj_prox(self, norm, alpha, test_prox_obj):
np.testing.assert_allclose(lhs, rhs, rtol=1e-6, atol=0.0)

@pytest.mark.parametrize("norm", normlist_blockarray_ready)
@pytest.mark.parametrize("alpha", alphalist)
def test_prox_blockarray(self, norm, alpha, test_prox_obj):
nrmobj = norm()
nrm = nrmobj.__call__
Expand Down Expand Up @@ -168,7 +178,6 @@ def test_scaled_attrs(self, norm, test_prox_obj):
assert scaled.scale == test_prox_obj.scalar

@pytest.mark.parametrize("norm", normlist)
@pytest.mark.parametrize("alpha", alphalist)
def test_scaled_eval(self, norm, alpha, test_prox_obj):

unscaled = norm()
Expand All @@ -179,7 +188,6 @@ def test_scaled_eval(self, norm, alpha, test_prox_obj):
np.testing.assert_allclose(a, b)

@pytest.mark.parametrize("norm", normlist)
@pytest.mark.parametrize("alpha", alphalist)
def test_scaled_prox(self, norm, alpha, test_prox_obj):
# Test prox
unscaled = norm()
Expand Down Expand Up @@ -237,7 +245,6 @@ class TestProj:

cnstrlist = [functional.NonNegativeIndicator, functional.L2BallIndicator]
sdistlist = [functional.SetDistance, functional.SquaredSetDistance]
alphalist = [1e-2, 1e-1, 1e0, 1e1]

@pytest.mark.parametrize("cnstr", cnstrlist)
def test_prox(self, cnstr, test_proj_obj):
Expand All @@ -264,7 +271,6 @@ def test_prox_scale_invariance(self, cnstr, test_proj_obj):

@pytest.mark.parametrize("sdist", sdistlist)
@pytest.mark.parametrize("cnstr", cnstrlist)
@pytest.mark.parametrize("alpha", alphalist)
def test_setdistance(self, sdist, cnstr, alpha, test_proj_obj):
if cnstr in NO_COMPLEX and snp.util.is_complex_dtype(test_proj_obj.v.dtype):
return
Expand Down
Loading

0 comments on commit 523d662

Please sign in to comment.