From c843ab29c9d0bbc04cca53c80e96dfdb2bd3a701 Mon Sep 17 00:00:00 2001 From: Janne Lappalainen <34949352+lappalainenj@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:03:44 -0400 Subject: [PATCH 1/2] test: make the suite deterministic Five tests failed sporadically, together on roughly one run in four. All of them share one cause: they assert on unseeded randomness. The augmentations draw from the global numpy stream, the mock Sintel data is generated fresh every run, and several assertions only hold for most draws rather than all of them. In CI a sporadic failure is indistinguishable from a real regression. Seed the global generators before every test, and seed the session-scoped mock data fixtures with their own generator, since those are built before the per-test fixture runs. FLYVIS_TEST_SEED re-runs the suite under a different seed; an assertion that only holds for the default seed is a broken assertion, and sweeping the seed is how to find one. Seeding alone only makes a wrong assertion fail reproducibly, so fix the five assertions that were true by luck: - test_sintel::test_getitem (8.1%) and test_sintel::test_apply_augmentation (10%) asserted that augmentation changes the target. Targets are only rotated, flipped and cropped -- they are neither jittered nor noised -- so a sampled augmentation leaves them untouched whenever it draws neither a rotation nor a flip: (1-p_rot)*(1-p_flip) = 1/12. Request a rotation explicitly instead of asserting on the draw. - test_ensemble::test_rank_by_validation_error (4.2%) shuffled the model names and asserted the order changed. A shuffle reproduces the sorted order once in len(ensemble)! draws, 1 in 24 here. Rotate instead, which is a derangement. - test_rendering::test_call (2.3%) compared the mean of a median-filtered uniform sample against 5 with atol=0.05, about 2.4 sigma of that statistic. Widen to ~7 sigma and say why; a seeded draw also differs between CPU and GPU, so a tight tolerance would pass locally and fail in CI. - test_augmentation::test_random_crop (~0.1%) asserted a random crop differs from the crop at frame 0, which fails when the sampled start is 0. Assert that the sampled start is the one applied, which is exact and stronger. Found and verified by sweeping the seed rather than repeating runs: repeating the suite 15 times missed two of these, while sweeping 40 seeds found both. Co-Authored-By: Claude Opus 5 --- tests/conftest.py | 40 ++++++++++++++++++++++++++++++++++---- tests/test_augmentation.py | 7 +++++-- tests/test_ensemble.py | 7 +++++-- tests/test_rendering.py | 10 ++++++++-- tests/test_sintel.py | 18 ++++++++++++++--- 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 22a0700..bff27ae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,13 +1,41 @@ +import os import tempfile from pathlib import Path import numpy as np import pytest +import torch from PIL import Image from flyvis import connectome_file from flyvis.connectome import ConnectomeFromAvgFilters +SEED = int(os.environ.get("FLYVIS_TEST_SEED", 0)) +"""Seed used to make the test suite deterministic. + +Override with `FLYVIS_TEST_SEED` to re-run the suite under a different seed. An +assertion that only holds for the default seed is a broken assertion, so sweeping +the seed is how to find one. +""" + + +@pytest.fixture(autouse=True) +def deterministic_rng(): + """Reset the global random number generators before every test. + + Augmentations draw from the global numpy stream, so without this a test's + outcome depends on how many draws the tests before it happened to consume, and + changes from run to run. Tests whose assertions only hold for most draws then + fail sporadically, which in CI is indistinguishable from a real regression. + + Note: + This makes a test reproducible, it does not make an assertion that only + holds for most draws correct. Assert on what the code guarantees, or set + the random parameters explicitly, rather than relying on a lucky seed. + """ + np.random.seed(SEED) + torch.manual_seed(SEED) + @pytest.fixture(scope="session") def connectome(tmp_path_factory): @@ -19,7 +47,10 @@ def connectome(tmp_path_factory): @pytest.fixture(scope="session") def sequence_path(tmp_path_factory): - sequences = np.random.rand(20, 10, 64, 64) + # session fixtures are built before the per-test seeding above runs, so they + # seed their own generator to keep the mock data identical across runs + rng = np.random.default_rng(SEED) + sequences = rng.random((20, 10, 64, 64)) sequences = np.transpose(sequences, (1, 0, 2, 3)) / 255.0 path = tmp_path_factory.mktemp("tmp") / "sequences.npy" np.save(path, sequences) @@ -29,6 +60,7 @@ def sequence_path(tmp_path_factory): @pytest.fixture(scope="session") def mock_sintel_data(): """Create a minimal mock Sintel dataset structure with original dimensions.""" + rng = np.random.default_rng(SEED) with tempfile.TemporaryDirectory() as tmp_dir: tmp_path = Path(tmp_dir) @@ -44,7 +76,7 @@ def mock_sintel_data(): # Create dummy files with original dimensions for i in range(5): # Luminance (final) - (436, 1024) - img = (np.random.uniform(0, 1, (HEIGHT, WIDTH)) * 255).astype(np.uint8) + img = (rng.uniform(0, 1, (HEIGHT, WIDTH)) * 255).astype(np.uint8) Image.fromarray(img).save( tmp_path / f"training/final/{seq_name}/frame_{i:04d}.png" ) @@ -57,7 +89,7 @@ def mock_sintel_data(): np.array([202021.25], dtype=np.float32).tofile(f) # Magic number np.array([WIDTH, HEIGHT], dtype=np.int32).tofile(f) # Dimensions # Write flow data - np.random.randn(HEIGHT, WIDTH, 2).astype(np.float32).tofile(f) + rng.standard_normal((HEIGHT, WIDTH, 2)).astype(np.float32).tofile(f) # Depth - (436, 1024) with open( @@ -66,6 +98,6 @@ def mock_sintel_data(): # Write header np.array([1, WIDTH, HEIGHT], dtype=np.int32).tofile(f) # Dimensions # Write depth data - np.random.randn(HEIGHT, WIDTH).astype(np.float32).tofile(f) + rng.standard_normal((HEIGHT, WIDTH)).astype(np.float32).tofile(f) yield tmp_path diff --git a/tests/test_augmentation.py b/tests/test_augmentation.py index a32262e..393dfe2 100644 --- a/tests/test_augmentation.py +++ b/tests/test_augmentation.py @@ -170,10 +170,13 @@ def _is_monotonous(sequence): assert cropped2.min() in sequence and cropped2.max() in sequence assert np.allclose(cropped1, cropped2) - # random start frame + # random start frame. A sampled start of 0 would legitimately reproduce + # cropped1, so assert that the sampled start is what was applied rather than + # that the crop differs. random_crop.set_or_sample(start=None, total_sequence_length=len(sequence)) cropped3 = random_crop(sequence).cpu().numpy().flatten() - assert not np.allclose(cropped1, cropped3) + assert len(cropped3) == 10 and _is_monotonous(cropped3) + assert np.allclose(cropped3, cropped1 + random_crop.start) # provided start frame random_crop.set_or_sample(start=42, total_sequence_length=len(sequence)) diff --git a/tests/test_ensemble.py b/tests/test_ensemble.py index 930419d..110f7b2 100644 --- a/tests/test_ensemble.py +++ b/tests/test_ensemble.py @@ -63,8 +63,11 @@ def test_validation_losses(ensemble): def test_rank_by_validation_error(ensemble): sorted_names = deepcopy(ensemble.names) - # destroy current task sorting in place - np.random.shuffle(ensemble.names) + # Destroy the current task sorting. A random shuffle reproduces the sorted + # order once in len(ensemble)! draws -- 1 in 24 here -- which would make the + # `!= random_names` assertion below fail. Rotating is a derangement for any + # ensemble with more than one model, so it always destroys the sorting. + ensemble.names = sorted_names[1:] + sorted_names[:1] random_names = deepcopy(ensemble.names) diff --git a/tests/test_rendering.py b/tests/test_rendering.py index 628966f..2c4e4e6 100644 --- a/tests/test_rendering.py +++ b/tests/test_rendering.py @@ -31,13 +31,19 @@ def test_call(boxeye: rendering.BoxEye): sequence = torch.ones((2, 2, 100, 100)).random_(0, 11) + # the median of uniform integers on [0, 10] is 5 in expectation, but this is a + # finite sample: the mean of the filtered frames has a standard deviation of + # about 0.02, so the tolerance is ~7 sigma. A tighter one fails for a few + # percent of inputs, and the draw differs between CPU and GPU even when seeded. + atol = 0.15 + rendered = boxeye(sequence, ftype="median", hex_sample=True) assert rendered.shape == (2, 2, 1, boxeye.hexals) - assert np.isclose(rendered.cpu().numpy().mean(), 5, atol=0.05) + assert np.isclose(rendered.cpu().numpy().mean(), 5, atol=atol) rendered = boxeye(sequence.clone(), ftype="median", hex_sample=False) assert rendered.shape == (*sequence.shape[:2], *boxeye.min_frame_size.cpu().numpy()) - assert np.isclose(rendered.cpu().numpy().mean(), 5, atol=0.05) + assert np.isclose(rendered.cpu().numpy().mean(), 5, atol=atol) with pytest.raises(ValueError): boxeye(sequence, ftype="invalid", hex_sample=True) diff --git a/tests/test_sintel.py b/tests/test_sintel.py index 286c21a..5009e3d 100644 --- a/tests/test_sintel.py +++ b/tests/test_sintel.py @@ -199,10 +199,17 @@ def test_getitem(dataset): dataset.augment = True data1 = dataset[0] assert set(data1.keys()) == set(["lum", "flow"]) - assert (data0["lum"] != data1["lum"]).any() - assert (data0["flow"] != data1["flow"]).any() assert data1["lum"].shape == (3, 1, 7) assert data1["flow"].shape == (3, 2, 7) + # the input is always changed, it is jittered and noised + assert (data0["lum"] != data1["lum"]).any() + # targets are only rotated, flipped and temporally cropped -- they are neither + # jittered nor noised -- so a sampled augmentation leaves them untouched + # whenever it draws neither a rotation nor a flip, which happens with + # probability (1 - p_rot) * (1 - p_flip) = 1/12. Ask for a rotation explicitly + # rather than asserting on the draw. + rotated = dataset.apply_augmentation(dataset.cached_sequences[0], n_rot=1, flip_axis=0) + assert (data0["flow"] != rotated["flow"]).any() # change dt to 1/50 dataset.dt = 1 / 50 @@ -224,8 +231,13 @@ def test_apply_augmentation(dataset): data = dataset[0] data1 = dataset.apply_augmentation(data) assert set(data1.keys()) == set(data.keys()) + # the input is always changed, it is jittered and noised assert (data["lum"] != data1["lum"]).any() - assert (data["flow"] != data1["flow"]).any() + # targets are only rotated, flipped and temporally cropped, so a sampled + # augmentation leaves them untouched whenever it draws neither a rotation nor + # a flip. Request a rotation explicitly rather than asserting on the draw. + rotated = dataset.apply_augmentation(data, n_rot=1, flip_axis=0) + assert (data["flow"] != rotated["flow"]).any() def test_original_sequence_index(dataset): From c1fe74aeebc1e5121cabd6f80034067201af575b Mon Sep 17 00:00:00 2001 From: Janne Lappalainen <34949352+lappalainenj@users.noreply.github.com> Date: Wed, 5 Aug 2026 21:18:05 -0400 Subject: [PATCH 2/2] ci: publish to PyPI from a release workflow Releasing is fully manual today: build locally, then twine upload. Automate it so that a release is a tag plus a GitHub Release and nothing else. Every push to main builds the sdist and the wheel and checks them, so a packaging mistake surfaces at merge time. Nothing is uploaded on a merge -- PyPI versions are immutable and cannot be reused, so publishing is tied to a version tag rather than to every commit that lands on main. Before uploading, the workflow refuses to continue if the version derived by setuptools_scm does not match the release tag, if twine check fails, or if flyvis/data/responses_norm.h5 is missing from either distribution. That last check matters because the constants are loaded from inside the installed package: a wheel without them would silently fall back to simulating 30 minutes of naturalistic stimuli per model. It then installs the wheel in a clean virtualenv and reads the constants back before publishing. Uploads use PyPI trusted publishing, so no API token is stored in the repository. The one-time PyPI-side setup is documented in docs/release.md. Co-Authored-By: Claude Opus 5 --- .github/workflows/release.yml | 120 ++++++++++++++++++++++++++++++++++ CHANGELOG.md | 11 ++++ docs/docs/release.md | 49 +++++++++++++- 3 files changed, 179 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/release.yml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..0988b61 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,120 @@ +name: Release + +# Every push to main builds and validates the distributions, so a packaging +# mistake surfaces at merge time rather than at release time. Publishing to PyPI +# happens when a GitHub Release is published: PyPI versions are immutable and +# unique, so a release has to be tied to a version tag rather than to every merge. +on: + push: + branches: ["main"] + release: + types: [published] + workflow_dispatch: + +permissions: + contents: read + +jobs: + build: + name: Build and check distributions + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # setuptools_scm derives the version from the tags, it needs all of them + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install build tooling + run: python -m pip install --upgrade build twine + + - name: Build sdist and wheel + run: python -m build + + - name: Check distribution metadata + run: python -m twine check dist/* + + - name: Check the precomputed response norms are packaged + # they are loaded from inside the installed package, so a wheel without + # them silently falls back to simulating naturalistic stimuli responses + run: | + python - <<'PY' + import glob, sys, tarfile, zipfile + + expected = "flyvis/data/responses_norm.h5" + wheel = glob.glob("dist/*.whl")[0] + sdist = glob.glob("dist/*.tar.gz")[0] + + in_wheel = expected in zipfile.ZipFile(wheel).namelist() + in_sdist = any( + name.split("/", 1)[-1] == expected for name in tarfile.open(sdist).getnames() + ) + print(f"{expected} in wheel: {in_wheel}, in sdist: {in_sdist}") + if not (in_wheel and in_sdist): + sys.exit(f"{expected} is missing from the distributions") + PY + + - name: Check the version matches the release tag + if: github.event_name == 'release' + run: | + python - <<'PY' + import glob, os, sys + + tag = os.environ["TAG"].lstrip("v") + built = glob.glob("dist/*.whl")[0].split("/")[-1].split("-")[1] + print(f"tag: {tag}, built version: {built}") + if built != tag: + sys.exit( + f"built version {built} does not match release tag {tag}; " + "the release must point at an annotated version tag" + ) + PY + env: + TAG: ${{ github.event.release.tag_name }} + + - name: Smoke test the wheel + run: | + python -m venv /tmp/smoke + /tmp/smoke/bin/pip install --upgrade pip + /tmp/smoke/bin/pip install dist/*.whl + /tmp/smoke/bin/python - <<'PY' + from flyvis.analysis import response_norms + + norms = response_norms.read_response_norms( + response_norms.PRECOMPUTED_FILE, "flow/0000" + ) + assert norms is not None, f"{response_norms.PRECOMPUTED_FILE} is not installed" + assert len(norms.model_names) == 50, norms + assert len(norms.checkpoint_hashes) == 50, norms + print("installed package ships constants for", norms) + PY + /tmp/smoke/bin/flyvis --help > /dev/null + + - uses: actions/upload-artifact@v4 + with: + name: distributions + path: dist/ + + publish: + name: Publish to PyPI + needs: build + if: github.event_name == 'release' + runs-on: ubuntu-latest + # Configure this environment name in the PyPI trusted publisher, and add + # required reviewers here if releases should be approved by a human. + environment: + name: pypi + url: https://pypi.org/p/flyvis + permissions: + # required for trusted publishing, no API token needs to be stored + id-token: write + steps: + - uses: actions/download-artifact@v4 + with: + name: distributions + path: dist/ + + - uses: pypa/gh-action-pypi-publish@release/v1 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dacfad..64b12e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,17 @@ - Added `examples/figure_04_top_models.py`, which reproduces figure 4a,b for the models with the lowest task error instead of the task-optimal cluster. +### Infrastructure +- Added a `Release` workflow that builds and checks the distributions on every push + to `main` and publishes them to PyPI via trusted publishing when a GitHub Release + is published. It verifies that the version matches the release tag and that the + precomputed response norms are present in both the wheel and the source + distribution before uploading. +- Made the test suite deterministic. Five tests asserted on unseeded randomness and + failed on roughly one run in seven between them. The global generators are now + seeded before every test, `FLYVIS_TEST_SEED` re-runs the suite under a different + seed, and the assertions that were only true for most draws were corrected. + ## [v1.1.3] - 2026-03-07 ### Bug Fixes diff --git a/docs/docs/release.md b/docs/docs/release.md index 94e7636..8e06229 100644 --- a/docs/docs/release.md +++ b/docs/docs/release.md @@ -1,5 +1,50 @@ # Release Process +Releases are published to PyPI by the +[`Release` workflow](https://github.com/TuragaLab/flyvis/blob/main/.github/workflows/release.yml). +The manual steps further down are the fallback for when the workflow cannot be used. + +## Automated release + +Every push to `main` builds the source distribution and the wheel and checks them, +so a packaging mistake --- a data file that stopped being included, for instance --- +surfaces at merge time. Nothing is uploaded on a merge: PyPI versions are immutable +and can never be reused, so publishing is tied to a version tag rather than to every +commit that lands on `main`. + +To cut a release: + +1. Merge to `main` and let the tests and the build check pass. +2. Update `CHANGELOG.md` with the new version. +3. Tag and push: + ```bash + git tag -a v1.1.4 -F CHANGELOG.md + git push origin main v1.1.4 + ``` +4. Publish a GitHub Release pointing at that tag. That triggers the upload. + +The workflow refuses to publish if the version derived by `setuptools_scm` does not +match the release tag, if the distributions fail `twine check`, or if the +precomputed response norms are missing from them. It then installs the built wheel +in a clean virtualenv and verifies that the constants can be read from the installed +package before uploading. + +### One-time setup + +Uploads use [PyPI trusted publishing](https://docs.pypi.org/trusted-publishers/), so +no API token is stored in the repository. On PyPI, under the `flyvis` project's +*Publishing* settings, add a GitHub publisher with: + +| Field | Value | +| --- | --- | +| Owner | `TuragaLab` | +| Repository | `flyvis` | +| Workflow name | `release.yml` | +| Environment name | `pypi` | + +Adding required reviewers to the `pypi` environment in the repository settings makes +every upload wait for a human approval. + ## Prerequisites 1. Ensure all tests pass: @@ -16,7 +61,9 @@ The deployment to github can be done last or via workflow. python -m pip install build twine ``` -## Release Steps +## Manual release steps + +These are the fallback for when the automated release cannot be used. 0. **Test PyPi before committing (optional)**