diff --git a/.github/workflows/draft-pdf.yml b/.github/workflows/draft-pdf.yml deleted file mode 100644 index 76310246..00000000 --- a/.github/workflows/draft-pdf.yml +++ /dev/null @@ -1,23 +0,0 @@ -on: [push] - -jobs: - paper: - runs-on: ubuntu-latest - name: Paper Draft - steps: - - name: Checkout - uses: actions/checkout@v2 - - name: Build draft PDF - uses: openjournals/openjournals-draft-action@master - with: - journal: joss - # This should be the path to the paper within your repo. - paper-path: paper/paper.md - - name: Upload - uses: actions/upload-artifact@v1 - with: - name: paper - # This is the output path where Pandoc will write the compiled - # PDF. Note, this should be the same directory as the input - # paper.md - path: paper/paper.pdf diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index b97518a2..0e5d93e9 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -1,28 +1,199 @@ +--- name: build -on: [push, pull_request] +on: + push: + branches: [main, master, 'release/**'] + pull_request: + schedule: + # Weekly, Monday 06:00 UTC. Catches upstream dependency breakage before a + # user does, which is how the NumPy 2 and pandas 3 failures reached release. + - cron: '0 6 * * 1' + workflow_dispatch: + +# Superseded runs on the same ref are pointless; cancel them. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read jobs: - build: - name: Build (${{ matrix.os }}, Python ${{ matrix.python-version }}) + test: + name: Test (${{ matrix.os }}, Python ${{ matrix.python-version }}) runs-on: ${{ matrix.os }} strategy: + # Report every platform failure, not just the first. + fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] - python-version: ["3.9", "3.10", "3.11"] + python-version: ['3.10', '3.11', '3.12', '3.13'] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 + with: + # This repository carries a large history relative to its working + # tree; a blobless checkout avoids transferring it. + filter: blob:none + fetch-depth: 1 + - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - - name: Install dependencies + cache: pip + cache-dependency-path: requirements.txt + + - name: Install + run: | + python -m pip install --upgrade pip + python -m pip install -e ".[dev]" + + - name: Run offline test suite + # No network and no package data: capability-gated tests skip visibly. + run: python -m pytest tests -v --cov=stitches --cov-report=xml --cov-report=term + + - name: Upload coverage + # Previously coverage.xml was generated and then discarded. + if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.11' + uses: codecov/codecov-action@v4 + with: + files: ./coverage.xml + fail_ci_if_error: false + + regression: + name: Output invariance + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + filter: blob:none + fetch-depth: 1 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: pip + cache-dependency-path: requirements.txt + + - name: Install + run: | + python -m pip install --upgrade pip + python -m pip install -e ".[dev]" + + - name: Assert outputs are unchanged + run: python -m pytest tests/regression -v -m regression + + - name: Assert golden artifacts were not silently modified + # --update-golden must never run in CI. If the working tree is dirty + # after the suite, something rewrote a golden artifact, which would let + # an output change pass review unnoticed. + run: | + if ! git diff --quiet -- tests/regression/golden; then + echo "::error::Golden artifacts were modified during the test run." + git diff --stat -- tests/regression/golden + exit 1 + fi + + benchmark: + name: Performance + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + filter: blob:none + fetch-depth: 1 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: pip + cache-dependency-path: requirements.txt + + - name: Install + run: | + python -m pip install --upgrade pip + python -m pip install -e ".[dev]" + python -m pip install pytest-benchmark + + - name: Run benchmarks + # Informational: shared runners are far too noisy to gate a merge on. + # The saved JSON is what makes trends reviewable over time. + run: | + python -m pytest benchmarks --benchmark-only \ + --benchmark-json=benchmark-results.json \ + --benchmark-columns=mean,stddev,rounds \ + --benchmark-sort=mean + + - name: Upload benchmark results + uses: actions/upload-artifact@v4 + with: + name: benchmark-results-${{ github.sha }} + path: benchmark-results.json + retention-days: 90 + + integration: + name: Network and package data + # Requires the Zenodo download and live Pangeo access, so it is too slow and + # too flaky on third-party availability to run on every push. + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 90 + steps: + - uses: actions/checkout@v4 + with: + filter: blob:none + fetch-depth: 1 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: pip + cache-dependency-path: requirements.txt + + - name: Install run: | python -m pip install --upgrade pip - python -m pip install -r requirements.txt - python -m pip install . - - name: Test and generate coverage report on Linux + python -m pip install -e ".[dev]" + + - name: Cache Zenodo package data + uses: actions/cache@v4 + with: + path: stitches/data + # Keyed on the Zenodo record so a data revision busts the cache. + key: stitches-pkgdata-8367628 + + - name: Run network and package-data tests run: | - pip install pytest - pip install pytest-cov - pytest --cov=./ --cov-report=xml + python -m pytest tests -v --network --slow --package-data + + latest-deps: + name: Latest dependencies + # Resolves dependencies without the repository pins to surface upstream + # breakage early. Allowed to fail so it warns rather than blocks. + if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + continue-on-error: true + steps: + - uses: actions/checkout@v4 + with: + filter: blob:none + fetch-depth: 1 + + - uses: actions/setup-python@v5 + with: + python-version: '3.13' + + - name: Install with newest resolvable dependencies + run: | + python -m pip install --upgrade pip + python -m pip install --upgrade \ + matplotlib xarray numpy pandas intake intake-esm nc_time_axis \ + scikit-learn 'fsspec[gcs]' tqdm requests pytest pyarrow + python -m pip install -e . --no-deps + + - name: Report resolved versions + run: python -m pip list + + - name: Run offline suite against latest dependencies + run: python -m pytest tests -v diff --git a/.gitignore b/.gitignore index 2a4239d2..cd16f566 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,17 @@ tests/.DS_Store # pycharm .idea +# vscode +.vscode + +# local development virtualenvs +.venv/ +venv/ +env/ + +# pytest-benchmark results +.benchmarks/ + # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3b600402..de66ad16 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,13 +9,36 @@ repos: rev: v3.8.0 hooks: - id: pyupgrade - args: ['--py39-plus'] + args: ['--py310-plus'] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer - id: debug-statements + - id: check-merge-conflict + - id: check-yaml + - id: check-toml + # Repository history is ~140x the size of the working tree because large + # generated artifacts were committed and later deleted. This blocks the + # next one. See plans/repo-clone-performance.md. + # Golden regression artifacts are excluded: they are intentionally + # committed, small, and individually well under this limit. + - id: check-added-large-files + args: ['--maxkb=512'] + + # Jupyter notebooks are the single largest recurring contributor to repository + # growth: stitches-quickstart.ipynb alone accumulated 38.5 MB across 31 + # revisions because base64-encoded PNG outputs are committed. Re-running a + # notebook rewrites every image byte-for-byte on one enormous JSON line, which + # git cannot delta-compress, so each commit stores a full new copy. + # + # Stripping outputs means GitHub's static renderer shows no figures; the + # executed notebooks are published through the nbsphinx docs build instead. + - repo: https://github.com/kynan/nbstripout + rev: 0.7.1 + hooks: + - id: nbstripout - repo: https://github.com/pre-commit/pygrep-hooks rev: v1.10.0 hooks: @@ -25,7 +48,7 @@ repos: rev: 23.3.0 hooks: - id: black - args: ['--target-version=py39'] + args: ['--target-version=py310'] - repo: https://github.com/pycqa/flake8 rev: 6.0.0 hooks: @@ -40,11 +63,11 @@ repos: rev: 1.7.0 hooks: - id: nbqa-pyupgrade - args: ['--py39-plus'] + args: ['--py310-plus'] additional_dependencies: ['pyupgrade==3.8.0'] - id: nbqa-black additional_dependencies: ['black==23.3.0'] - args: ['--line-length=88', '--target-version=py39'] + args: ['--line-length=88', '--target-version=py310'] - id: nbqa-isort additional_dependencies: ['isort==5.12.0'] args: ['--profile=black'] @@ -57,7 +80,7 @@ repos: hooks: - id: blackdoc additional_dependencies: ['black==23.3.0'] - args: ['--target-version=py39'] + args: ['--target-version=py310'] - repo: https://github.com/adrienverge/yamllint.git rev: v1.32.0 hooks: diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..9f5a40cc --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,131 @@ +# Changelog + +All notable changes to `stitches` are documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Output-change policy + +`stitches` produces scientific data, so a change in output is a change in +published results. Any modification that alters output must be recorded under a +`### Changed — outputs` heading, must state which defect it corrects, and must be +accompanied by regenerated golden artifacts under `tests/regression/golden/`. +See [`plans/benchmarks-and-regression-testing.md`](plans/benchmarks-and-regression-testing.md). + +--- + +## [Unreleased] + +### Fixed + +- **`make_tas_archive` could not write its output files.** Two defects in the + same three lines: + 1. The path was built as `tas_data_dir + "/" + name + "_tas.csv"`, but + `importlib.resources.files(...)` returns a `Traversable`, so + `Traversable + str` raises `TypeError`. + 2. Grouping used `groupby(["model"])`. With a single-element *list* pandas 2.0+ + yields a one-element tuple as the group name, so even with the path fixed + the filenames would have been `('BCC-CSM2-MR',)_tas.csv`. + + The file-writing loop is now the separately testable + `write_tas_data_by_model`, using `os.path.join` (also correcting the hardcoded + `/` separator on Windows) and `groupby("model")`. This code was previously + unreachable in CI because exercising it requires a multi-hour download of the + full CMIP6 archive, which is why the bug shipped. +- **`install_package_data` hardened.** It buffered the entire multi-hundred- + megabyte archive in memory via `BytesIO`, sent no `timeout` (so a hung server + blocked forever), never called `raise_for_status` (so an HTML error page was + written out and later failed as "not a zip file"), imported `tqdm` without + using it, and used `os.mkdir` (which fails when a parent is missing). It now + streams to a temporary file with a progress bar, sets connect/read timeouts, + raises on HTTP errors, uses `os.makedirs(exist_ok=True)`, reports a clear error + for a corrupt archive, and returns the list of files written instead of `None`. +- **`temp-data` archive members are now extracted to `temp-data`.** Only paths + containing `tas-data` were re-nested, so `temp-data` files were flattened into + the top level even though the directory was created for them. +- **An unregistered package-data version now warns.** Previously the fallback URL + was substituted silently, so a release without a registered dataset would + quietly download a possibly mismatched archive. + +- **`get_chunk_info` crashed on NumPy 2.** The per-chunk rate of change was + extracted with `float(model.coef_[0])`. Because `LinearRegression` is fitted + against a column vector, `coef_` has shape `(1, 1)` and `coef_[0]` is a + one-element array, not a scalar. Converting a one-element array via `float()` + was deprecated in NumPy 1.25 and raises `TypeError` in NumPy 2, making the + function — and therefore all archive generation and matching — unusable on + current NumPy. Now uses `float(model.coef_.ravel()[0])`, verified to produce + values identical to the legacy behavior across 200 randomized fits. +- **`calculate_rolling_mean` crashed on pandas 3.** The call + `drop(columns="value", axis=1)` passes both `columns` and `axis`, which pandas + 3.0 rejects with `ValueError: Cannot specify both 'axis' and 'index'/'columns'`. + The `axis=1` was redundant because `columns=` already selects the column axis; + it has been removed with no change in behavior. + +### Added + +- **PEP 621 `pyproject.toml`** replacing `setup.py` and `setup.cfg`. The version + is now single-sourced from `stitches/_version.py` via `dynamic = ["version"]` + rather than parsed with a regex at build time. +- **`requests` declared as a runtime dependency.** It is imported by + `stitches/install_pkgdata.py` but was never declared, so a clean install could + fail at `install_package_data()` if `requests` happened not to be pulled in + transitively. +- **Extras split into `test`, `docs`, and `dev`,** with `pytest-cov`, + `pytest-benchmark`, and `pyarrow` now declared instead of being installed + ad hoc by CI. +- **Performance benchmark suite** (`benchmarks/`, 33 benchmarks) parametrized by + input size to expose scaling rather than single timings. Excluded from the + default `pytest` run; see + [`plans/benchmarks-and-regression-testing.md`](plans/benchmarks-and-regression-testing.md). +- **CI jobs for output invariance and benchmarks,** plus a guard that fails the + build if golden artifacts are modified during a test run, and scheduled + `integration` (network + package data) and `latest-deps` (unpinned resolve) + jobs so upstream breakage is caught before release. + +- **Golden-output regression suite** (`tests/regression/`) asserting that + refactors and dependency upgrades do not change scientific output. Artifacts + are stored as Parquet with per-quantity tolerances (exact for indices, years + and labels; `1e-12` for match distances; `1e-10` for temperature values). + Regenerate deliberately with `pytest tests/regression --update-golden`. + Validated by mutation testing: a one-part-in-10⁹ perturbation of `dist_l2` is + detected. +- **Explicit `seed` parameters** on `shuffle_function`, + `permute_stitching_recipes`, and `make_recipe`, so reproducible output no + longer requires the `testing=True` flag. Fully backward compatible: + `seed=None` reproduces the previous behavior exactly, and `testing=True` + remains equivalent to `seed=1`. +- **Pytest capability markers** (`network`, `slow`, `package_data`, + `regression`, `benchmark`) with `--network`, `--slow`, and `--package-data` + opt-in flags and matching `STITCHES_TEST_*` environment variables. + +### Changed + +- **Dropped Python 3.9** (end of life); the floor is now 3.10. The CI matrix + covers 3.10, 3.11, 3.12, and 3.13 on Linux, macOS, and Windows, replacing the + previous 3.9–3.11 matrix. +- **CI modernized:** `actions/checkout@v4` and `actions/setup-python@v5` + (previously v3/v4), pip caching, blobless checkout, concurrency cancellation, + `fail-fast: false` so every platform failure is reported, and coverage actually + uploaded rather than generated and discarded. + +- **Tests no longer download package data implicitly.** `tests/conftest.py` + previously installed the full Zenodo archive in a session-scoped `autouse` + fixture, so every `pytest` invocation downloaded hundreds of megabytes. Package + data is now an opt-in `package_data` fixture; the offline tier runs in seconds. +- `tests/test_pangeo.py` and `tests/test_stitch.py` converted from + `unittest.TestCase` to pytest functions, with the monolithic test methods split + into single-behavior tests. + +### Removed + +- **The `RUN = "ci"` test gate.** `test_pangeo.py` and `test_stitch.py` guarded + their real assertions behind a hardcoded class attribute whose false branch ran + `self.assertEqual(0, 0)`. These tests reported as *passing* in CI while + exercising no code, leaving the Pangeo and gridded-stitching paths effectively + untested. They are now marked and skip visibly instead. + +### Documentation + +- Added `plans/` with a development plan, a git clone-performance analysis, and + the benchmark/regression design. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 995eb7f8..f234e32d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,6 +3,85 @@ We welcome third-party patches, which are essential for advancing the science and architecture of STITCHES. But there are a few guidelines that we ask contributors to follow, guidelines that ease the maintainers' organizational and logistical duties, while encouraging development by others. All contributors agree to abide by the code of conduct. +## Development Environment + +Because this repository has a large history relative to its working tree, prefer a +blobless clone; it fetches file contents on demand and is substantially faster: + +```bash +git clone --filter=blob:none https://github.com/JGCRI/stitches.git +cd stitches + +python -m venv .venv && source .venv/bin/activate # Python >= 3.10 +python -m pip install -e ".[dev]" +pre-commit install +``` + +## Testing + +The suite is organized in tiers so the common case is fast and offline. + +```bash +pytest # offline correctness, a few seconds +pytest tests/regression # golden-output invariance +pytest --network # additionally hit Pangeo +pytest --package-data # additionally download the Zenodo archive +pytest --network --slow --package-data # everything +``` + +Capability-gated tests are declared with markers and **skip visibly** when the +capability is not enabled; they never pass vacuously. Each flag has an environment +variable equivalent (`STITCHES_TEST_NETWORK`, `STITCHES_TEST_SLOW`, +`STITCHES_TEST_PACKAGE_DATA`) for CI use. + +### Output invariance: the most important rule + +`stitches` produces scientific data, so **changing its output changes published +results**. `tests/regression/` compares the current code against golden artifacts +recorded from a known-good baseline. Any refactor, dependency bump, or +optimization must leave these unchanged. + +If a regression test fails, first assume you have introduced a bug. Only if the +recorded output is genuinely *wrong* should you regenerate: + +```bash +pytest tests/regression --update-golden +``` + +A pull request that modifies anything under `tests/regression/golden/` **must**: + +1. add a `CHANGELOG.md` entry under `### Fixed` or `### Changed — outputs` + naming the defect the new output corrects, and +2. be reviewed by a domain maintainer, not only a code reviewer. + +CI fails the build if golden artifacts change during a test run, so an accidental +`--update-golden` cannot slip through. + +When adding a function that produces data, add an invariance test for it. + +## Benchmarks + +Performance is tracked separately from correctness, and is excluded from the +default `pytest` run: + +```bash +pytest benchmarks --benchmark-only --benchmark-save=baseline +pytest benchmarks --benchmark-only --benchmark-compare=baseline +``` + +Run benchmarks on a single fixed OS and Python version; numbers from different +machines, or from different CI matrix jobs, are not comparable. Optimization work +is only acceptable when the benchmarks improve **and** the invariance suite still +passes. Current baseline measurements and the scaling analysis derived from them +are in [`plans/benchmarks-and-regression-testing.md`](plans/benchmarks-and-regression-testing.md). + +## Notebooks + +Notebook outputs are stripped automatically on commit by `nbstripout`. This is +deliberate: committed base64 image outputs were the largest single source of +repository growth. Do not re-add them, and do not commit generated data files. +See [`plans/repo-clone-performance.md`](plans/repo-clone-performance.md). + ## Getting Started * Make sure you have a [GitHub account](https://github.com/signup/free). @@ -20,8 +99,10 @@ But there are a few guidelines that we ask contributors to follow, guidelines th * We will never accept pull requests to the `master` branch. * Check for unnecessary whitespace with `git diff --check` before committing. * Make sure your commit messages are descriptive but succinct, describing what was changed and why, and **reference the relevant issue number**. Make commits of logical units. -* Make sure you have added the necessary tests for your changes. Tests should be included in the root `tests` directory and are facilitated using `pytest` which is installed with the development version of STITCHES. See more info on using `pytest` here: https://docs.pytest.org/en/7.4.x/contents.html +* Make sure you have added the necessary tests for your changes. Tests belong in the root `tests` directory and run under `pytest`, which is installed with the development extra. See the [`pytest` documentation](https://docs.pytest.org/). +* If your change touches code that produces data, add or update an invariance test in `tests/regression/` (see **Output invariance** above). * Run _all_ the tests to assure nothing else was accidentally broken. +* Record notable changes in `CHANGELOG.md`. Anything that alters output **must** be recorded there. ## Submitting Changes diff --git a/MANIFEST.in b/MANIFEST.in index 6f2407a9..357c75e3 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,7 +1,29 @@ include README.md +include CHANGELOG.md include LICENSE include DISCLAIMER -include notebooks/*.* +include CITATION.cff +include requirements.txt +include pytest.ini + +# Package data. Also declared in pyproject.toml [tool.setuptools.package-data] +# so the wheel carries it; these entries cover the sdist. +include stitches/data/README.md include stitches/data/tas-data/README.md include stitches/data/temp-data/README.md include stitches/data/example/*.csv + +# Ship the tests and the golden regression artifacts so that an installed +# distribution can be verified against the recorded outputs. +recursive-include tests *.py *.csv +recursive-include tests/regression/golden *.parquet *.json +recursive-include benchmarks *.py + +# The notebooks are tutorial material; exclude their large embedded outputs from +# the distribution. See plans/repo-clone-performance.md. +exclude notebooks/*.ipynb +prune notebooks +prune docs +prune paper +prune plans +prune .github diff --git a/README.md b/README.md index e98d9a7e..c0db5541 100644 --- a/README.md +++ b/README.md @@ -11,8 +11,9 @@ Amalgamate existing climate data to create monthly climate variable fields. Jupyter notebooks hosted on `stitches` use functionality that is contained within the accompanying Python package. > **NOTE** -> Ensure you are using Python >= 3.9. Calling `python` may use a different instance. Some users may need to use `python3` or the like instead. +> Ensure you are using Python >= 3.10. Calling `python` may use a different instance. Some users may need to use `python3` or the like instead. +`stitches` is tested on Python 3.10–3.13 across Linux, macOS, and Windows. ### Installation To install for use, run the following: @@ -42,6 +43,46 @@ but note that this will take several hours to run. | `stitches-quickstart.ipynb` | Simple tutorial to demonstrate how `stitches` can be used as an emulator. | ### Contributing -`stitches` users and developers must agree to our community guidelines outlines in our community guidelines outlines in our +`stitches` users and developers must agree to the community guidelines set out in our [Contributor Guidelines](CONTRIBUTING.md) and [Code of Conduct](CODE_OF_CONDUCT.md). -Open an issue to ask for help or report an issue ([how to open a GitHub issue](https://docs.github.com/en/enterprise-server@3.1/issues/tracking-your-work-with-issues/creating-an-issue)). +Open an issue to ask for help or report a problem ([how to open a GitHub issue](https://docs.github.com/en/enterprise-server@3.1/issues/tracking-your-work-with-issues/creating-an-issue)). + +#### Cloning for development + +This repository's history is large relative to its working tree, so a full clone +transfers far more than you need. Use a blobless clone, which fetches file +contents on demand: + +```bash +git clone --filter=blob:none https://github.com/JGCRI/stitches.git +``` + +Then install the development extra and enable the hooks: + +```bash +python -m pip install -e ".[dev]" +pre-commit install +``` + +#### Testing + +```bash +pytest # offline suite, runs in seconds +pytest tests/regression # assert scientific outputs are unchanged +pytest --network --slow --package-data # full suite, downloads data +``` + +`stitches` produces scientific data, so output-changing modifications are held to +a higher bar: `tests/regression/` pins outputs against recorded golden artifacts, +and any intentional change must be justified in [`CHANGELOG.md`](CHANGELOG.md). +See [Contributor Guidelines](CONTRIBUTING.md) for the full workflow. + +### Development plans + +Ongoing modernization work is tracked in [`plans/`](plans/): + +| Document | Purpose | +|---|---| +| [`development-plan.md`](plans/development-plan.md) | Current-state assessment, known defects, and prioritized workstreams | +| [`benchmarks-and-regression-testing.md`](plans/benchmarks-and-regression-testing.md) | Output-invariance harness, benchmark baseline, and scaling analysis | +| [`repo-clone-performance.md`](plans/repo-clone-performance.md) | Why clones are slow, with measurements and remediation options | diff --git a/benchmarks/__init__.py b/benchmarks/__init__.py new file mode 100644 index 00000000..de1ac485 --- /dev/null +++ b/benchmarks/__init__.py @@ -0,0 +1,31 @@ +"""Performance benchmarks for the stitches package. + +These are **not** correctness tests. They measure wall-clock time so that +performance work can be shown to help, and so that a refactor intended to be +neutral can be shown not to have made things slower. + +Correctness during optimization is the job of ``tests/regression``: the two +suites are meant to be used together. A performance change is acceptable only +when the benchmarks improve *and* the golden-output suite still passes. + +Running +------- + +Benchmarks are excluded from the default ``pytest`` run. To use them:: + + # Record a baseline, ideally on a tagged commit + pytest benchmarks --benchmark-only --benchmark-save=baseline + + # Compare a change against that baseline and fail on a big regression + pytest benchmarks --benchmark-only \\ + --benchmark-compare=baseline \\ + --benchmark-compare-fail=mean:25% + +Interpretation +-------------- + +Run benchmarks on one fixed OS and Python version. Comparing numbers across +machines, or across matrix jobs, is meaningless -- shared CI runners vary by +well over the regression threshold from run to run. Treat any benchmark whose +mean is under roughly 10ms as indicative only. +""" diff --git a/benchmarks/bench_match.py b/benchmarks/bench_match.py new file mode 100644 index 00000000..ee978aac --- /dev/null +++ b/benchmarks/bench_match.py @@ -0,0 +1,87 @@ +"""Benchmarks for the matching functions. + +`match_neighborhood` is the primary optimization target identified in the +development plan: it loops in Python over each target window group and calls +`internal_dist` per group, then concatenates. The parametrized archive sizes here +exist to reveal how that cost scales, which is what justifies (or fails to +justify) vectorizing it. +""" + +import pytest + +from stitches.fx_match import ( + drop_hist_false_duplicates, + internal_dist, + match_neighborhood, + shuffle_function, +) + +from .conftest import make_archive, make_chunk_frame + + +@pytest.mark.parametrize("tol", [0.0, 0.1, 0.5]) +def test_bench_match_neighborhood_example(benchmark, example_target, example_archive, tol): + """Benchmark `match_neighborhood` on the committed example data.""" + result = benchmark(match_neighborhood, example_target, example_archive, tol=tol) + + assert len(result) > 0 + + +@pytest.mark.parametrize("n_windows", [28, 112, 280]) +def test_bench_match_neighborhood_scaling(benchmark, n_windows): + """Benchmark `match_neighborhood` as the number of target windows grows. + + The window counts correspond to roughly 1x, 4x, and 10x a single CMIP6 + trajectory. Comparing the three reveals whether the per-group Python loop + scales linearly or worse. + """ + target = make_chunk_frame(n_windows) + archive = make_archive(n_windows, n_members=4) + + result = benchmark(match_neighborhood, target, archive, tol=0.1) + + assert len(result) > 0 + + +@pytest.mark.parametrize("n_members", [2, 8, 16]) +def test_bench_match_neighborhood_archive_width(benchmark, n_members): + """Benchmark `match_neighborhood` as the archive gains ensemble members. + + Archive size grows independently of target size, so this isolates the cost of + searching a wider archive from the cost of iterating more target windows. + """ + target = make_chunk_frame(28) + archive = make_archive(28, n_members=n_members) + + result = benchmark(match_neighborhood, target, archive, tol=0.1) + + assert len(result) > 0 + + +def test_bench_internal_dist(benchmark, example_target): + """Benchmark `internal_dist`, the innermost distance computation.""" + result = benchmark( + internal_dist, example_target.fx[0], example_target.dx[0], example_target + ) + + assert len(result) > 0 + + +def test_bench_drop_hist_false_duplicates(benchmark, example_match_with_duplicates): + """Benchmark `drop_hist_false_duplicates`. + + Another per-group loop with a terminal `pd.concat`, and a candidate for the + same vectorization treatment as `match_neighborhood`. + """ + result = benchmark( + drop_hist_false_duplicates, example_match_with_duplicates.copy() + ) + + assert len(result) > 0 + + +def test_bench_shuffle_function(benchmark, multi_member_series): + """Benchmark `shuffle_function` on a large frame.""" + result = benchmark(shuffle_function, multi_member_series, seed=1) + + assert len(result) == len(multi_member_series) diff --git a/benchmarks/bench_processing.py b/benchmarks/bench_processing.py new file mode 100644 index 00000000..0bad7967 --- /dev/null +++ b/benchmarks/bench_processing.py @@ -0,0 +1,82 @@ +"""Benchmarks for the time-series processing functions. + +`calculate_rolling_mean` and `chunk_ts` run over the entire CMIP6 archive during +package-data generation, which the documentation notes takes several hours. They +are therefore the functions where a constant-factor improvement translates into +the largest absolute saving. +""" + +import pytest + +from stitches.fx_processing import ( + calculate_rolling_mean, + chunk_ts, + get_chunk_info, + subset_archive, +) + +from .conftest import make_series + + +@pytest.mark.parametrize("size", [3, 9, 21]) +def test_bench_calculate_rolling_mean(benchmark, multi_member_series, size): + """Benchmark `calculate_rolling_mean` across window sizes. + + The implementation wraps a lambda in ``groupby.transform``; a wider window + should not change the cost much, so a strong dependence on ``size`` would + itself be a finding. + """ + result = benchmark(calculate_rolling_mean, multi_member_series.copy(), size) + + assert len(result) == len(multi_member_series) + + +@pytest.mark.parametrize("n_groups", [4, 20, 100]) +def test_bench_calculate_rolling_mean_scaling(benchmark, n_groups): + """Benchmark `calculate_rolling_mean` as the number of groups grows. + + Group count, not row count, is what drives the grouped-transform overhead. + """ + import pandas as pd + + frames = [ + make_series(experiment=f"ssp{i:03d}", ensemble=f"r{i}i1p1f1") + for i in range(n_groups) + ] + data = pd.concat(frames).reset_index(drop=True) + + result = benchmark(calculate_rolling_mean, data.copy(), 9) + + assert len(result) == len(data) + + +@pytest.mark.parametrize("n", [5, 9, 20]) +def test_bench_chunk_ts(benchmark, long_series, n): + """Benchmark `chunk_ts` across chunk sizes.""" + result = benchmark(chunk_ts, long_series.copy(), n) + + assert "chunk" in result.columns + + +def test_bench_get_chunk_info(benchmark, long_series): + """Benchmark `get_chunk_info`. + + Fits a scikit-learn ``LinearRegression`` per chunk inside a Python loop and + grows the result with repeated ``pd.concat``, so it is a prime candidate for + both a closed-form slope and a single terminal concat. + """ + chunked = chunk_ts(long_series.copy(), 9) + + result = benchmark(get_chunk_info, chunked) + + assert len(result) > 0 + + +def test_bench_subset_archive(benchmark, long_series): + """Benchmark `subset_archive`.""" + info = get_chunk_info(chunk_ts(long_series.copy(), 9)) + end_years = sorted(info["end_yr"].unique())[:10] + + result = benchmark(subset_archive, info, end_years) + + assert len(result) <= len(info) diff --git a/benchmarks/bench_recipe.py b/benchmarks/bench_recipe.py new file mode 100644 index 00000000..bad10dad --- /dev/null +++ b/benchmarks/bench_recipe.py @@ -0,0 +1,125 @@ +"""Benchmarks for recipe construction. + +`permute_stitching_recipes` is the most expensive pure-Python routine in the +package. It runs a while loop that repeatedly samples candidate recipes and +rejects those violating the duplicate and collapse constraints, so its cost +depends on how often it has to retry -- which in turn depends on the matching +tolerance and on how many realizations are requested. + +All calls pass an explicit ``seed`` so the amount of work done is identical from +run to run. Without that, retry counts would vary between runs and the timings +would be too noisy to compare. +""" + +import pytest + +from stitches.fx_match import match_neighborhood +from stitches.fx_recipe import ( + get_num_perms, + handle_final_period, + handle_transition_periods, + permute_stitching_recipes, + remove_duplicates, +) + +from .conftest import make_archive, make_chunk_frame + + +@pytest.fixture(scope="module") +def scenario(): + """Return a target/archive pair large enough to be worth timing.""" + target = make_chunk_frame(28) + archive = make_archive(28, n_members=6) + return target, archive + + +@pytest.fixture(scope="module") +def matched(scenario): + """Return matched data for the benchmark scenario.""" + target, archive = scenario + return match_neighborhood(target, archive, tol=0.1) + + +def test_bench_get_num_perms(benchmark, matched): + """Benchmark `get_num_perms`, several chained groupby aggregations.""" + result = benchmark(get_num_perms, matched) + + assert len(result) == 2 + + +def test_bench_remove_duplicates(benchmark, scenario): + """Benchmark `remove_duplicates` on nearest-neighbor matches.""" + target, archive = scenario + nn = match_neighborhood(target, archive, tol=0.0) + + result = benchmark(remove_duplicates, md=nn.copy(), archive=archive) + + assert len(result) > 0 + + +@pytest.mark.parametrize("n_matches", [1, 2, 5]) +def test_bench_permute_stitching_recipes(benchmark, matched, scenario, n_matches): + """Benchmark `permute_stitching_recipes` as more realizations are requested. + + Cost is expected to grow faster than linearly in ``N_matches``, because each + additional realization must avoid every archive point already consumed by the + previous ones, raising the rejection rate. + """ + _, archive = scenario + + result = benchmark( + permute_stitching_recipes, + N_matches=n_matches, + matched_data=matched, + archive=archive, + seed=1, + ) + + assert len(result) > 0 + + +@pytest.mark.parametrize("tol", [0.05, 0.1, 0.3]) +def test_bench_permute_by_tolerance(benchmark, scenario, tol): + """Benchmark `permute_stitching_recipes` across matching tolerances. + + A wider tolerance means more candidates per window: more choice, but also more + matched rows to carry through the constraint checks. + """ + target, archive = scenario + matched_data = match_neighborhood(target, archive, tol=tol) + + result = benchmark( + permute_stitching_recipes, + N_matches=2, + matched_data=matched_data, + archive=archive, + seed=1, + ) + + assert len(result) > 0 + + +def test_bench_handle_transition_periods(benchmark, matched, scenario): + """Benchmark `handle_transition_periods`.""" + _, archive = scenario + messy = permute_stitching_recipes( + N_matches=1, matched_data=matched, archive=archive, seed=1 + ) + + result = benchmark(handle_transition_periods, messy.copy()) + + assert len(result) > 0 + + +def test_bench_handle_final_period(benchmark, matched, scenario): + """Benchmark `handle_final_period`.""" + _, archive = scenario + messy = handle_transition_periods( + permute_stitching_recipes( + N_matches=1, matched_data=matched, archive=archive, seed=1 + ) + ) + + result = benchmark(handle_final_period, messy.copy()) + + assert len(result) > 0 diff --git a/benchmarks/conftest.py b/benchmarks/conftest.py new file mode 100644 index 00000000..d78da5a8 --- /dev/null +++ b/benchmarks/conftest.py @@ -0,0 +1,149 @@ +"""Fixtures and configuration for the performance benchmarks. + +Provides synthetic data at several scales so benchmarks can show how each +function grows with input size, not just how long it takes on one fixed input. +Scaling behavior is what identifies the functions worth optimizing: a routine +whose cost grows quadratically with the number of target windows is a much better +target than one that is merely slow but linear. +""" + +from importlib import resources +from pathlib import Path + +import numpy as np +import pandas as pd +import pytest + +# Every benchmark in this directory carries the `benchmark` marker. +pytestmark = pytest.mark.benchmark + + +def pytest_configure(config): + """Register the markers this directory uses. + + Declared here as well as in ``pytest.ini`` so that ``pytest benchmarks`` works + when invoked directly from this directory, where the root config may not be + picked up. + """ + for marker in ( + "benchmark: performance measurement, not a correctness assertion", + "slow: long-running benchmark (>30s)", + "network: requires internet access", + "package_data: requires the full Zenodo-minted package data", + ): + config.addinivalue_line("markers", marker) + + +def make_series(n_years=251, model="test_model", experiment="ssp245", ensemble="r1i1p1f1"): + """Build a deterministic synthetic annual temperature series. + + :param n_years: Number of annual values to generate. + :return: A DataFrame with the columns the processing functions require. + :rtype: pandas.DataFrame + """ + years = np.arange(1850, 1850 + n_years) + rng = np.random.default_rng(20240101) + + return pd.DataFrame( + { + "year": years, + "value": 0.00012 * (years - 1850) ** 2 + rng.normal(0, 0.05, size=n_years), + "variable": "tas", + "model": model, + "experiment": experiment, + "ensemble": ensemble, + "unit": "K", + } + ) + + +def make_chunk_frame(n_windows, ensemble="r1i1p1f1", experiment="ssp245", offset=0.0): + """Build a synthetic chunked archive with ``n_windows`` time windows. + + Mirrors the shape produced by ``get_chunk_info``: one row per 9-year window + carrying the level (``fx``) and rate of change (``dx``). + + :param n_windows: Number of windows to generate. + :param offset: Constant added to ``fx``, used to make archive members differ. + :return: A DataFrame shaped like a matching archive. + :rtype: pandas.DataFrame + """ + starts = 1850 + 9 * np.arange(n_windows) + rng = np.random.default_rng(abs(hash((ensemble, experiment))) % (2**32)) + + return pd.DataFrame( + { + "experiment": experiment, + "variable": "tas", + "ensemble": ensemble, + "model": "test_model", + "start_yr": starts, + "end_yr": starts + 8, + "year": starts + 4, + "fx": np.linspace(-1.3, 3.0, n_windows) + offset, + "dx": rng.normal(0.02, 0.01, size=n_windows), + } + ) + + +def make_archive(n_windows, n_members=4): + """Build a synthetic archive spanning several ensemble members. + + :param n_windows: Windows per member. + :param n_members: Number of ensemble members. + :return: A concatenated archive DataFrame. + :rtype: pandas.DataFrame + """ + frames = [ + make_chunk_frame( + n_windows, ensemble=f"r{i + 1}i1p1f1", offset=0.01 * i + ) + for i in range(n_members) + ] + return pd.concat(frames).reset_index(drop=True) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="session") +def example_dir(): + """Return the directory of committed example CSVs.""" + return Path(str(resources.files("stitches") / "data" / "example")) + + +@pytest.fixture(scope="session") +def example_target(example_dir): + """Return the committed example target data.""" + return pd.read_csv(example_dir / "test-target_dat.csv") + + +@pytest.fixture(scope="session") +def example_archive(example_dir): + """Return the committed example archive data.""" + return pd.read_csv(example_dir / "test-archive_dat.csv") + + +@pytest.fixture(scope="session") +def example_match_with_duplicates(example_dir): + """Return committed match data that still contains historical duplicates.""" + return pd.read_csv(example_dir / "test-match_w_dup.csv") + + +@pytest.fixture(scope="session") +def long_series(): + """Return a single 251-year series, the full CMIP6 1850-2100 span.""" + return make_series() + + +@pytest.fixture(scope="session") +def multi_member_series(): + """Return a multi-experiment, multi-member series for grouped operations.""" + frames = [ + make_series(experiment=exp, ensemble=ens) + for exp in ("historical", "ssp126", "ssp245", "ssp585") + for ens in (f"r{i}i1p1f1" for i in range(1, 6)) + ] + return pd.concat(frames).reset_index(drop=True) diff --git a/plans/benchmarks-and-regression-testing.md b/plans/benchmarks-and-regression-testing.md new file mode 100644 index 00000000..eec87c0b --- /dev/null +++ b/plans/benchmarks-and-regression-testing.md @@ -0,0 +1,445 @@ +# Benchmarks and Output-Invariance Regression Testing + +> **Implementation status.** The harness described here is implemented and green. +> See [§0](#0-implemented-baseline-2026-08-28) for the recorded baseline and the +> scaling findings it produced. + +--- + +## 0. Implemented baseline (2026-08-28) + +Environment: macOS, Python 3.13.3, pandas 2.2.3 → verified also under pandas +3.0.5 / NumPy 2.5.2 / scikit-learn 1.9.0. + +Suite state: + +``` +tests/ 98 passed, 13 skipped (network/package_data), ~8s +tests/regression/ 70 golden-output tests, 34 artifacts, 384K +benchmarks/ 33 benchmarks, ~39s, saved as 0001_baseline.json +``` + +### 0.1 Bugs found by adding the suite + +Writing the regression tests immediately surfaced two crashes that make the +package unusable on a current scientific Python stack. Both are recorded in +[`CHANGELOG.md`](../CHANGELOG.md): + +| Function | Failure | Cause | +|---|---|---| +| `get_chunk_info` | `TypeError: only 0-dimensional arrays can be converted to Python scalars` | `float()` on the one-element array `model.coef_[0]`; removed in NumPy 2 | +| `calculate_rolling_mean` | `ValueError: Cannot specify both 'axis' and 'index'/'columns'` | `drop(columns="value", axis=1)` passes both selectors; rejected by pandas 3 | + +This is the argument for building the suite first: neither bug was visible from +the existing tests. + +### 0.2 Baseline timings (mean, sorted) + +Slowest twelve, which is where optimization effort belongs: + +| Mean | Benchmark | +|---:|---| +| 646.5 ms | `match_neighborhood_scaling[280]` | +| 234.7 ms | `match_neighborhood_scaling[112]` | +| 82.8 ms | `match_neighborhood_archive_width[16]` | +| 78.7 ms | `permute_by_tolerance[0.3]` | +| 67.4 ms | `match_neighborhood_archive_width[8]` | +| 59.5 ms | `match_neighborhood_scaling[28]` | +| 58.1 ms | `match_neighborhood_example[0.1]` | +| 50.6 ms | `permute_by_tolerance[0.1]` | +| 49.3 ms | `permute_stitching_recipes[5]` | +| 49.3 ms | `permute_stitching_recipes[2]` | +| 45.2 ms | `calculate_rolling_mean_scaling[100]` | +| 27.6 ms | `permute_by_tolerance[0.05]` | + +### 0.3 Scaling findings + +These are the conclusions the parametrization was designed to produce. + +**`match_neighborhood` is superlinear in target windows — the top optimization target.** + +| Target windows | Mean | Factor vs. 28 | +|---:|---:|---:| +| 28 | 59.5 ms | 1.0× | +| 112 (4×) | 234.7 ms | 3.9× | +| 280 (10×) | 646.5 ms | **10.9×** | + +Growth is slightly worse than linear (10.9× cost for 10× input). Combined with +its already-dominant absolute cost, this makes the per-group Python loop in +[`stitches/fx_match.py`](../stitches/fx_match.py:150) the clear first target for +vectorization. + +**Archive width costs less than target count.** Going from 2 to 16 ensemble +members (8×) raises cost only 1.6× (53.2 → 82.8 ms), so the bottleneck is +iterating target groups, not scanning the archive. Optimization should attack the +loop, not the search. + +**`calculate_rolling_mean` is dominated by group count, not row count.** + +| Groups | Mean | Per group | +|---:|---:|---:| +| 4 | 2.75 ms | 688 µs | +| 20 | 9.62 ms | 481 µs | +| 100 | 45.2 ms | 452 µs | + +Roughly 450–700 µs of fixed overhead per group, and window size is irrelevant +(3/9/21 all ≈ 9.65 ms at 20 groups). That overhead is the `groupby.transform` +lambda, confirming it as a worthwhile target. + +**`permute_stitching_recipes` saturates in `N_matches` but grows with tolerance.** +N=2 and N=5 are indistinguishable (49.28 vs 49.31 ms) because the synthetic +archive cannot support five collapse-free realizations, so the loop exits early. +Tolerance, by contrast, drives cost steadily (27.6 → 50.6 → 78.7 ms for +0.05/0.1/0.3) as more candidate rows flow through the constraint checks. A +realistic `N_matches` scaling benchmark needs a wider archive. + +**`get_chunk_info` costs 20.1 ms for 28 chunks** — about 700 µs per chunk, spent +constructing a scikit-learn `LinearRegression` per chunk and growing the result +with repeated `pd.concat`. A closed-form slope would remove nearly all of it, and +the golden artifacts make that change safe to attempt. + +### 0.4 Reproducing + +```bash +python -m pytest tests -q # offline correctness, ~8s +python -m pytest tests/regression -q # golden-output invariance +python -m pytest benchmarks --benchmark-only --benchmark-save=baseline +python -m pytest benchmarks --benchmark-only \ + --benchmark-compare=baseline --benchmark-compare-fail=mean:25% +``` + +--- + +Purpose: guarantee that modernization work described in [`plans/development-plan.md`](development-plan.md) does **not** alter scientific outputs, and that performance does not silently regress. + +Two distinct suites: + +| Suite | Question it answers | Gate | +|---|---|---| +| **Invariance / golden-output** | Did the numbers change? | Hard fail on any unexplained diff | +| **Performance benchmark** | Did it get slower or use more memory? | Fail beyond a configured threshold | + +--- + +## 1. Baseline Capture + +### 1.1 Freeze the reference point + +```bash +git tag baseline/v0.13 +git push origin baseline/v0.13 +``` + +Record an exact environment so the baseline is reproducible: + +```bash +pip freeze > tests/regression/baseline-env-py311.txt +python -c "import platform,sys; print(platform.platform(), sys.version)" \ + >> tests/regression/baseline-env-py311.txt +``` + +### 1.2 Proposed layout + +``` +tests/ + regression/ + __init__.py + conftest.py # tolerance config, golden dir resolution, --update-golden flag + generate_golden.py # CLI: rebuild golden artifacts from current code + baseline-env-py311.txt + golden/ + match/ + match_neighborhood_tol0.parquet + match_neighborhood_tol0.1_nodup.parquet + match_neighborhood_tol0.1_dedup.parquet + recipe/ + permute_N1_seed1.parquet + permute_N5_seed1.parquet + make_recipe_ssp245_bcc.parquet + generate_gridded_recipe_mon.parquet + processing/ + rolling_mean_w9.parquet + chunk_ts_n9.parquet + get_chunk_info.parquet + stitch/ + gmat_stitching_bcc.parquet + gridded_stitching_bcc.sha256 # checksum only; NetCDF too large to vendor + archive/ + make_matching_archive_w9.parquet.sha256 + tas_archive_filenames.json + test_invariance_match.py + test_invariance_recipe.py + test_invariance_processing.py + test_invariance_stitch.py +benchmarks/ + __init__.py + conftest.py + bench_match.py + bench_recipe.py + bench_processing.py + bench_stitch.py + bench_io.py + asv.conf.json # optional: airspeed-velocity history tracking +``` + +Golden artifacts use **Parquet** (stable, typed, compact) rather than CSV to avoid float-formatting drift masking or creating diffs. Where an artifact would exceed ~1 MB, store a SHA-256 of a canonicalized serialization instead of the payload. + +--- + +## 2. Invariance Suite + +### 2.1 Functions that must be pinned + +Ordered by risk. Everything reachable from [`stitches/__init__.py`](../stitches/__init__.py) is public API and must be covered. + +| Target | Module | Determinism notes | +|---|---|---| +| `match_neighborhood` | [`stitches/fx_match.py`](../stitches/fx_match.py:235) | Deterministic; test `tol=0`, `tol=0.1`, both `drop_hist_duplicates` values | +| `internal_dist` | [`stitches/fx_match.py`](../stitches/fx_match.py:15) | Pure numeric | +| `drop_hist_false_duplicates` | [`stitches/fx_match.py`](../stitches/fx_match.py:95) | Tie-breaking on `min(idvalue)` is order-sensitive — pin explicitly | +| `shuffle_function` | [`stitches/fx_match.py`](../stitches/fx_match.py:81) | **Randomized** — must accept/lock a seed | +| `calculate_rolling_mean` | [`stitches/fx_processing.py`](../stitches/fx_processing.py:12) | `min_periods=1` edge behavior is the whole point; pin window ends | +| `chunk_ts`, `get_chunk_info` | [`stitches/fx_processing.py`](../stitches/fx_processing.py:63) | Deterministic | +| `subset_archive` | [`stitches/fx_processing.py`](../stitches/fx_processing.py:196) | Deterministic | +| `get_num_perms`, `remove_duplicates` | [`stitches/fx_recipe.py`](../stitches/fx_recipe.py:13) | Deterministic | +| `permute_stitching_recipes` | [`stitches/fx_recipe.py`](../stitches/fx_recipe.py:250) | **Randomized** via `group.sample(...)`; has a `testing=True` path that sets `random_state=1` — use it | +| `handle_transition_periods`, `handle_final_period` | [`stitches/fx_recipe.py`](../stitches/fx_recipe.py:678) | Deterministic | +| `generate_gridded_recipe`, `make_recipe` | [`stitches/fx_recipe.py`](../stitches/fx_recipe.py:931) | Depends on `pangeo_table.csv` — pin the data version | +| `gmat_stitching` | [`stitches/fx_stitch.py`](../stitches/fx_stitch.py:411) | Reads local `tas-data`; deterministic given data version | +| `gridded_stitching` | [`stitches/fx_stitch.py`](../stitches/fx_stitch.py:245) | Network (Pangeo) + writes NetCDF; mark `network`/`slow`, compare values not file bytes | +| `global_mean`, `get_ds_meta` | [`stitches/fx_data.py`](../stitches/fx_data.py:25) | Latitude-weighting math — pin against a synthetic grid | +| `calculate_anomaly`, `paste_historical_data` | [`stitches/make_tas_archive.py`](../stitches/make_tas_archive.py:104) | Deterministic; pin on a small synthetic frame | +| `make_matching_archive` | [`stitches/make_matching_archive.py`](../stitches/make_matching_archive.py:17) | Deterministic; checksum the output archive | +| `combine_df`, `anti_join`, `remove_obs_from_match`, `selstr` | [`stitches/fx_util.py`](../stitches/fx_util.py) | Cheap, fully deterministic — pure unit tests | + +### 2.2 Handling nondeterminism + +Two sources of randomness exist and both must be neutralized: + +1. [`stitches/fx_recipe.py`](../stitches/fx_recipe.py:446) — `group.sample(1, replace=False)` vs. `random_state=1` under `testing=True`. +2. [`stitches/fx_match.py`](../stitches/fx_match.py:81) — `shuffle_function`. + +Actions: + +- Invariance tests always call with `testing=True` / an explicit seed. +- **Add a `seed: int | None = None` parameter** to the public randomized entry points (a strictly additive, backward-compatible change) so users can reproduce results, and so tests do not depend on a `testing` flag that also alters other behavior. +- Additionally assert **statistical** invariance for the unseeded path: run K=200 unseeded draws, compare the distribution of `dist_l2` and the set of selected archive points against the baseline using a fixed-tolerance comparison of summary statistics. This catches changes to the *sampling space* even when individual draws differ. + +### 2.3 Comparison helpers + +```python +# tests/regression/conftest.py (sketch) +import pandas as pd +from pandas.testing import assert_frame_equal + +# Sorting removes any dependence on groupby/concat ordering, which is exactly +# the kind of incidental change a refactor is allowed to make. +def canonicalize(df: pd.DataFrame) -> pd.DataFrame: + df = df.reindex(sorted(df.columns), axis=1) + return df.sort_values(list(df.columns), kind="mergesort").reset_index(drop=True) + +def assert_matches_golden(actual, golden_path, rtol=0.0, atol=0.0): + expected = pd.read_parquet(golden_path) + assert_frame_equal( + canonicalize(actual), + canonicalize(expected), + check_dtype=False, # int64/int32 platform differences are acceptable + check_like=False, + rtol=rtol, + atol=atol, + ) +``` + +Tolerance policy: + +| Comparison | rtol / atol | +|---|---| +| Recipe tables, matches, chunk metadata (indices, years, labels) | exact (`0.0`) | +| Distances (`dist_l2`, `dist_dx`, `dist_fx`) | `rtol=1e-12` | +| Global-mean temperature values | `rtol=1e-10` | +| Gridded NetCDF field values | `rtol=1e-6`, `atol=1e-9` | + +Rationale: anything integral or categorical must be bit-exact; floating-point reductions may legitimately reassociate across NumPy/pandas versions, so a tight-but-nonzero tolerance avoids false alarms while still catching real algorithmic change. + +### 2.4 Test skeleton + +```python +# tests/regression/test_invariance_match.py (sketch) +import pandas as pd +import pytest +from stitches.fx_match import match_neighborhood +from .conftest import assert_matches_golden + +@pytest.mark.parametrize( + "tol,dedup,golden", + [ + (0.0, True, "match/match_neighborhood_tol0.parquet"), + (0.1, False, "match/match_neighborhood_tol0.1_nodup.parquet"), + (0.1, True, "match/match_neighborhood_tol0.1_dedup.parquet"), + ], +) +def test_match_neighborhood_invariant(golden_dir, tol, dedup, golden): + target = pd.read_csv("tests/test-target_dat.csv") + archive = pd.read_csv("tests/test-archive_dat.csv") + out = match_neighborhood(target, archive, tol=tol, drop_hist_duplicates=dedup) + assert_matches_golden(out, golden_dir / golden, rtol=1e-12) +``` + +### 2.5 Regenerating golden files + +```bash +# Explicit, never automatic +python -m tests.regression.generate_golden --all +python -m tests.regression.generate_golden --only recipe +pytest tests/regression --update-golden # equivalent, opt-in flag +``` + +Policy: a PR that changes any file under `tests/regression/golden/` **must** include a `CHANGELOG.md` entry under `### Changed — outputs` explaining which defect the new output corrects, and must be reviewed by a domain maintainer, not just a code reviewer. + +### 2.6 Fixtures and the data dependency + +`tests/conftest.py` currently calls `stitches.install_package_data()` in a session fixture, which downloads the entire Zenodo archive. + +- Pin the expected data record explicitly (e.g. Zenodo `8367628`) and assert it, so an upstream data change cannot be mistaken for a code regression. +- Cache the download in CI keyed on the data version: + + ```yaml + - uses: actions/cache@v4 + with: + path: ~/.cache/stitches-data + key: stitches-data-8367628 + ``` + +- Add an offline tier: small committed fixtures (the existing `tests/test-*.csv`) cover `fx_match`, `fx_processing`, `fx_recipe`, and `fx_util` invariance with no network at all. Only `gridded_stitching` and the Pangeo tests need the network. +- Existing test CSVs are duplicated in both [`tests/`](../tests/) and [`stitches/data/example/`](../stitches/data/example/) — consolidate to one location referenced by both. + +--- + +## 3. Performance Benchmark Suite + +### 3.1 Tooling + +- `pytest-benchmark` for in-suite micro/meso benchmarks with JSON output and `--benchmark-compare-fail`. +- Optionally `asv` (airspeed velocity) for long-run history graphs across commits. +- `memory_profiler` / `tracemalloc` peak-RSS assertions for `gridded_stitching`, which is the memory-bound path. + +### 3.2 What to benchmark + +| Benchmark | Target | Why | +|---|---|---| +| `bench_match_neighborhood` | [`match_neighborhood`](../stitches/fx_match.py:235) at 1/10/50 target windows | Per-group Python loop, a WS-6 vectorization target | +| `bench_drop_hist_duplicates` | [`drop_hist_false_duplicates`](../stitches/fx_match.py:95) | Loop + `pd.concat` | +| `bench_permute_recipes` | [`permute_stitching_recipes`](../stitches/fx_recipe.py:250) at N=1/5/20 | Most algorithmically complex function in the package | +| `bench_make_recipe` | [`make_recipe`](../stitches/fx_recipe.py:1023) | End-to-end user path | +| `bench_rolling_mean` | [`calculate_rolling_mean`](../stitches/fx_processing.py:12) | `groupby.transform` with a lambda | +| `bench_chunk_ts` | [`chunk_ts`](../stitches/fx_processing.py:63) | Hot inner helper | +| `bench_matching_archive` | [`make_matching_archive`](../stitches/make_matching_archive.py:17) | Nested `for offset × groupby` loop | +| `bench_gmat_stitching` | [`gmat_stitching`](../stitches/fx_stitch.py:411) | Reads all `tas-data` CSVs | +| `bench_load_data_files` | [`load_data_files`](../stitches/fx_util.py:205) | CSV→Parquet migration candidate | +| `bench_pangeo_table_read` | [`fx_recipe.py`](../stitches/fx_recipe.py:980) repeated `read_csv` | Caching candidate | +| `bench_gridded_stitching` | [`gridded_stitching`](../stitches/fx_stitch.py:245) | `slow`+`network`; nightly only, tracks wall time and peak RSS | + +### 3.3 Example + +```python +# benchmarks/bench_match.py (sketch) +import pandas as pd +import pytest +from stitches.fx_match import match_neighborhood + +@pytest.fixture(scope="module") +def frames(): + return ( + pd.read_csv("tests/test-target_dat.csv"), + pd.read_csv("tests/test-archive_dat.csv"), + ) + +@pytest.mark.parametrize("tol", [0.0, 0.1, 0.5]) +def test_bench_match_neighborhood(benchmark, frames, tol): + target, archive = frames + result = benchmark(match_neighborhood, target, archive, tol=tol) + assert len(result) > 0 +``` + +### 3.4 Thresholds and CI wiring + +```bash +# Store the baseline once, on the tagged commit +pytest benchmarks --benchmark-only --benchmark-save=baseline + +# On every PR +pytest benchmarks --benchmark-only \ + --benchmark-compare=baseline \ + --benchmark-compare-fail=mean:25% +``` + +- Fail a PR at **>25% mean regression** on any benchmark (generous, because GitHub runners are noisy). +- Run benchmarks on a **single fixed OS/Python combination** (ubuntu-latest / 3.11) to keep numbers comparable; never gate on the full matrix. +- Use `--benchmark-min-rounds` and warmup to reduce variance; treat sub-10ms benchmarks as informational only. +- Nightly scheduled job runs the `slow`/`network` benchmarks and posts results as an artifact. + +### 3.5 Marker configuration + +Add to `pyproject.toml`: + +```toml +[tool.pytest.ini_options] +testpaths = ["tests"] +markers = [ + "network: requires internet access (Pangeo, Zenodo)", + "slow: long-running (>30s)", + "regression: golden-output invariance test", + "benchmark: performance measurement, not correctness", +] +addopts = "-m 'not benchmark'" +``` + +This replaces the current `RUN = "ci"` class attributes in [`tests/test_stitch.py`](../tests/test_stitch.py:28) and [`tests/test_pangeo.py`](../tests/test_pangeo.py:17), which make tests *pass vacuously* (`self.assertEqual(0, 0)`) rather than skip visibly. + +--- + +## 4. CI Integration + +```mermaid +flowchart TD + PR[Pull request] --> L[Lint / pre-commit] + PR --> U[Unit tests, matrix, offline fixtures] + PR --> R[Invariance suite, ubuntu + 3.11, cached data] + PR --> B[Benchmarks vs baseline, ubuntu + 3.11] + L --> G{All green} + U --> G + R --> G + B --> G + G -->|yes| M[Merge] + N[Nightly schedule] --> NW[network + slow + gridded benchmarks] + N --> ND[latest-dependencies resolve] +``` + +Jobs to add to [`.github/workflows/workflow.yml`](../.github/workflows/workflow.yml) (or split into separate workflows): + +1. `test` — existing, upgraded to `actions/checkout@v4` / `setup-python@v5`, with `cache: pip`, filtered checkout, and coverage uploaded to Codecov. +2. `regression` — installs cached package data, runs `pytest tests/regression -m "regression and not network"`. +3. `benchmark` — runs `pytest benchmarks --benchmark-compare`, uploads the JSON artifact. +4. `nightly` — cron; network + slow + gridded, plus an unpinned dependency resolve to catch upstream breakage. + +Add `concurrency: { group: ${{ github.ref }}, cancel-in-progress: true }` to stop stacked runs. + +--- + +## 5. Implementation Order + +1. Add pytest markers and delete the `RUN = "ci"` vacuous-pass pattern. +2. Add `tests/regression/` scaffolding, `canonicalize`/`assert_matches_golden`, and the `--update-golden` flag. +3. Add `seed` parameters to the randomized entry points (additive only, no behavior change when omitted). +4. Generate golden artifacts at `baseline/v0.13` and commit them. +5. Add `benchmarks/` and save the `baseline` benchmark run. +6. Wire the `regression` and `benchmark` CI jobs; require them on `main`. +7. Only then begin WS-2 onward from [`plans/development-plan.md`](development-plan.md). + +## 6. Definition of Done + +- Every function exported from [`stitches/__init__.py`](../stitches/__init__.py) has at least one invariance test. +- Zero vacuously-passing tests; all skips are marker-driven and reported in the summary. +- Golden files regenerate reproducibly from a clean checkout via one documented command. +- Benchmark baseline committed; PR comparison gate active. +- The regeneration policy (changelog entry + domain review) is documented in [`docs/source/reference/contributing.rst`](../docs/source/reference/contributing.rst). diff --git a/plans/development-plan.md b/plans/development-plan.md new file mode 100644 index 00000000..37ae1298 --- /dev/null +++ b/plans/development-plan.md @@ -0,0 +1,244 @@ +# `stitches` Development Plan + +Status: WS-1 through WS-4 substantially delivered; see §0 + +--- + +## 0. Progress (2026-08-28, branch `release/v1`) + +| Workstream | Status | +|---|---| +| WS-1 Baseline & regression safety net | **Done** — 70 golden-output tests, 34 artifacts, 33 benchmarks with a saved baseline | +| WS-2 Packaging modernization | **Done** — PEP 621 `pyproject.toml`, `requests` declared, version single-sourced | +| WS-3 Python & dependency matrix | **Done** — 3.9 dropped, CI covers 3.10–3.13 on three OSes, scheduled unpinned resolve | +| WS-4 Correctness fixes | **Done** — all four confirmed defects fixed, each with tests | +| WS-5 Code quality & structure | Not started (ruff consolidation, type hints, `fx_recipe` split) | +| WS-6 Performance | Not started; benchmarks and scaling analysis now in place to guide it | +| WS-7 Repository size | Partial — prevention landed; history rewrite awaits maintainer sign-off | +| WS-8 Docs & community | Partial — `CHANGELOG.md`, CONTRIBUTING, README done; quickstart restructure outstanding | + +Test suite: **126 passed, 13 skipped** offline in ~8s. Previously a bare `pytest` +downloaded hundreds of megabytes before it could run at all. + +### Defects found and fixed + +Three of the five were **not** in the original assessment; they surfaced only once +the regression suite existed, which is the argument for building it first. + +| Defect | Effect | Found by | +|---|---|---| +| `get_chunk_info` used `float()` on a 1-element array | `TypeError` on NumPy 2; broke all archive generation and matching | New regression tests | +| `calculate_rolling_mean` passed `columns=` and `axis=` together | `ValueError` on pandas 3 | New regression tests | +| `make_tas_archive` built paths with `Traversable + str` and `groupby([key])` | Could not write output at all; filenames would be `('MODEL',)_tas.csv` | Original assessment | +| `requests` imported but never declared | Clean installs could fail at `install_package_data()` | Original assessment | +| `install_pkgdata` buffered whole archive, no timeout, no `raise_for_status`, `temp-data` never populated | Memory pressure, indefinite hangs, misleading errors | Audit during the fix | +| `pytest -m regression` selected nothing | Would have reported the CI regression job green while running zero tests | Verifying the CI wiring | + +The last one is worth noting: the marker bug was itself an instance of the +silent-pass failure mode this work set out to eliminate. + +### Deferred, with reasons + +- **Hot-path optimization (WS-6).** Deliberately not attempted yet. The + benchmarks now identify the targets precisely — `match_neighborhood` is + superlinear in target windows and dominates total cost — and the golden + artifacts make the work verifiable. Doing it before that existed would have + been unverifiable. +- **Git history rewrite (WS-7 §3.4).** Measured as worth roughly an order of + magnitude, but it rewrites every commit SHA and needs a coordinated freeze, + fork re-clones, and care around the Zenodo/JOSS citation trail. Requires + maintainer sign-off, not a unilateral decision. +- **Deleting committed docs renders.** They are referenced by six `.. image::` + directives in `quickstarter.rst`; removing them requires converting that page + to an `nbsphinx`-executed notebook. +- **Stripping outputs from existing notebooks.** The hook prevents further + growth; rewriting 7 MB of existing notebook content changes what readers see on + GitHub and belongs in its own reviewed commit. + +--- + +## 1. Original Assessment + +Status: draft for review +Scope: modernization, correctness, performance, repository hygiene, and regression protection for the `stitches-emulator` package (current version `0.13`). + +Related documents: + +- [`plans/repo-clone-performance.md`](repo-clone-performance.md) — why cloning is slow and how to fix it +- [`plans/benchmarks-and-regression-testing.md`](benchmarks-and-regression-testing.md) — output-invariance harness and performance benchmarks + +--- + +## 1. Current State Assessment + +### 1.1 Package layout + +| Area | Observation | +|---|---| +| Build system | Legacy [`setup.py`](../setup.py) + near-empty [`setup.cfg`](../setup.cfg) + [`MANIFEST.in`](../MANIFEST.in); no `pyproject.toml`, no PEP 517/518 build backend | +| Version | [`stitches/_version.py`](../stitches/_version.py) pins `0.13`, hand-maintained and duplicated in [`stitches/install_pkgdata.py`](../stitches/install_pkgdata.py) URL map and in [`CITATION.cff`](../CITATION.cff) | +| Python support | `python_requires=">=3.9.0"`; CI matrix is 3.9–3.11 only. 3.9 is end-of-life; 3.12/3.13 untested | +| Dependencies | [`requirements.txt`](../requirements.txt) uses open-ended `>=` pins only; **`requests` is imported by [`stitches/install_pkgdata.py`](../stitches/install_pkgdata.py:11) but is not declared anywhere** | +| CI | [`.github/workflows/workflow.yml`](../.github/workflows/workflow.yml) uses `actions/checkout@v3` and `actions/setup-python@v4` (outdated), computes coverage but never uploads it, has no pip cache, no package-data cache, and no concurrency cancellation | +| Pre-commit | [`.pre-commit-config.yaml`](../.pre-commit-config.yaml) hooks are pinned to mid-2023 revisions (black 23.3.0, flake8 6.0.0, isort 5.12.0, pyupgrade v3.8.0). No `ruff`; three overlapping linters instead | +| Tests | 6 test modules, `unittest`-style, gated by hardcoded `RUN = "ci"` flags that silently skip real assertions; session fixture in [`tests/conftest.py`](../tests/conftest.py:16) downloads the full Zenodo archive on every run | +| Docs | Sphinx site present; API reference and quickstarter exist but reference an older workflow; no changelog | +| Data | All science data lives off-repo on Zenodo (record `8367628`) and is installed at runtime into `stitches/data/` | + +### 1.2 Confirmed defects to fix (behavior-changing, currently incorrect) + +These are bugs where "changing the output" is the *point*. Each needs a regression test written **before** the fix. + +1. **`make_tas_archive` writes bad filenames / crashes.** In [`stitches/make_tas_archive.py`](../stitches/make_tas_archive.py:453): + ```python + for name, group in data.groupby(["model"]): + path = tas_data_dir + "/" + name + "_tas.csv" + ``` + - `tas_data_dir` is an `importlib.resources` `Traversable`, so `Traversable + str` raises `TypeError`. + - With pandas ≥ 2.0, grouping by a **list** of one key yields a one-element **tuple** as `name`, so even after fixing the path join the filenames would become `('BCC-CSM2-MR',)_tas.csv`. + - Fix: `os.path.join(str(tas_data_dir), f"{name}_tas.csv")` and `groupby("model")` (or unpack the tuple). +2. **Missing runtime dependency `requests`.** Fresh installs fail at `stitches.install_package_data()` unless `requests` happens to be present transitively. +3. **`install_pkgdata` robustness.** In [`stitches/install_pkgdata.py`](../stitches/install_pkgdata.py:53): + - `os.mkdir` instead of `os.makedirs(..., exist_ok=True)`. + - `requests.get(...)` has no `timeout`, no `raise_for_status()`, and buffers the entire (~GB-scale) zip into memory via `BytesIO`. + - `tqdm` is imported but unused; the "progress" message is a bare `print`. + - The version→URL dict must be manually extended for every release; a `DEFAULT_VERSION` fallback silently downloads a possibly mismatched dataset. + - Only files whose path contains `tas-data` are re-nested; `temp-data` is created but never populated. +4. **Unused/dead imports and `# noqa`-adjacent lint debt** — flake8 is currently configured to ignore `F401`, masking real dead imports. +5. **Pandas/xarray/NumPy deprecation exposure** — grouped `.agg` with lambdas, `groupby(list_of_one)`, positional `dim` args to `xarray` reductions, and `intake-esm` pinned to a 2021 release. These will break under newer stacks even though outputs are currently correct. + +### 1.3 Repository clone problem (summary) + +The working tree is small; the **git history and binary blobs** are the cost. Primary suspects, in order of impact: + +1. Jupyter notebooks committed **with base64-encoded PNG outputs** — [`notebooks/stitches-quickstart.ipynb`](../notebooks/stitches-quickstart.ipynb), [`notebooks/preparing-input-data.ipynb`](../notebooks/preparing-input-data.ipynb), [`notebooks/stitches_training_GCAMAnnualMeeting2023.ipynb`](../notebooks/stitches_training_GCAMAnnualMeeting2023.ipynb), [`notebooks/stitches_takehome_GCAMAnnualMeeting2023.ipynb`](../notebooks/stitches_takehome_GCAMAnnualMeeting2023.ipynb). Every re-run rewrites megabytes of unrelated base64, so **each historical commit stores a full new copy**. +2. Historical commits of package data (`stitches/data/tas-data/*.csv`, `*.nc`, `matching_archive*.csv`) that were later `.gitignore`d — ignoring a file does not remove it from history. +3. Duplicated raster assets (`stitches_diagram.jpg` stored three times under [`docs/source/getting-started/`](../docs/source/getting-started/), [`notebooks/figs/`](../notebooks/figs/), and [`paper/`](../paper/)) plus committed Sphinx `output_*.png` renders. + +Full diagnosis and remediation options are in [`plans/repo-clone-performance.md`](repo-clone-performance.md). + +--- + +## 2. Guiding Principle: Output Invariance + +> Any refactor, dependency bump, or performance change must produce **bit-comparable or tolerance-comparable outputs** to the current release, except where the current output is provably wrong. + +Enforcement mechanism: + +1. Freeze "golden" outputs from the current code at a tagged baseline commit (`baseline/v0.13`). +2. Store golden artifacts as small, compressed fixtures (or checksums for large ones) under `tests/regression/`. +3. Every PR runs the invariance suite; deliberate output changes require updating the golden fixture **in the same PR** with a written justification in the changelog. + +```mermaid +flowchart LR + A[Tag baseline v0.13] --> B[Record golden outputs] + B --> C[Refactor / modernize] + C --> D[Run invariance suite] + D -->|identical| E[Merge] + D -->|differs| F[Is current output wrong?] + F -->|yes| G[Update golden + changelog entry] + F -->|no| H[Fix the regression] + G --> E + H --> D +``` + +--- + +## 3. Workstreams + +### WS-1 — Baseline and regression safety net (must land first) + +- Tag `baseline/v0.13` and pin a fully resolved environment lockfile for reproducibility. +- Build the golden-output harness and performance benchmarks per [`plans/benchmarks-and-regression-testing.md`](benchmarks-and-regression-testing.md). +- Remove the `RUN = "ci"` escape hatches in [`tests/test_stitch.py`](../tests/test_stitch.py:28) and [`tests/test_pangeo.py`](../tests/test_pangeo.py:17); replace with `pytest` markers (`-m "not network"`, `-m "not slow"`) so skips are explicit and countable. +- Cache the Zenodo package data in CI so [`tests/conftest.py`](../tests/conftest.py) does not re-download per job. + +### WS-2 — Packaging and build modernization + +- Replace [`setup.py`](../setup.py)/[`setup.cfg`](../setup.cfg) with a PEP 621 `pyproject.toml` (setuptools backend), moving dependencies, `extras_require`, classifiers, and metadata inline. +- Single-source the version: keep `stitches/_version.py` as the authority (`dynamic = ["version"]`) or adop`setuptools-scm`; then derive `CITATION.cff` and docs `conf.py` from it. +- Declare `requests` as a runtime dependency; add upper bounds or a tested-versions matrix for `pandas`, `xarray`, `numpy`, `intake-esm`. +- Convert `MANIFEST.in` content to `[tool.setuptools.package-data]` where possible; verify wheel/sdist contents with `check-manifest` and `twine check`. +- Replace `twine~=3.4.1` dev pin with a current release; add a `build`+`twine` release workflow triggered on tags with PyPI Trusted Publishing. + +### WS-3 — Python and dependency support matrix + +- Drop Python 3.9; support 3.10–3.13. Update `pyupgrade`/`black`/`ruff` target versions accordingly. +- Expand the CI matrix to 3.10/3.11/3.12/3.13 on ubuntu + macos + windows, with a reduced "full matrix" on `main` and a fast subset on PRs. +- Add a scheduled weekly "latest dependencies" job (unpinned resolve) to catch upstream breakage early. + +### WS-4 — Correctness fixes + +- Fix the `make_tas_archive` filename/path bug (§1.2.1) with a unit test asserting produced filenames. +- Harden `install_pkgdata`: streaming download with `stream=True` + `tqdm`, `timeout`, `raise_for_status`, `os.makedirs(exist_ok=True)`, checksum verification, resume/skip-if-present, and an explicit error (not silent fallback) when a version has no registered dataset. +- Move the version→Zenodo-URL map out of code into a small data file (`stitches/data/zenodo_registry.json` or a `concept DOI` that always resolves to the latest record). +- Audit every `groupby([...])` call site for the pandas 2.x single-key-tuple behavior: [`fx_match.py`](../stitches/fx_match.py:150), [`fx_recipe.py`](../stitches/fx_recipe.py:514), [`make_matching_archive.py`](../stitches/make_matching_archive.py:60), [`make_tas_archive.py`](../stitches/make_tas_archive.py:453). +- Replace `print` diagnostics throughout with the `logging` module and a package-level logger. + +### WS-5 — Code quality and structure + +- Consolidate linting on `ruff` (+`ruff format`) replacing flake8/isort/pyupgrade/pydocstyle; keep `blackdoc`/`nbqa` equivalents or migrate to `ruff`'s notebook support. +- Bump all `.pre-commit-config.yaml` revs and enable `pre-commit autoupdate` on a schedule. +- Add type hints to the public API (`match_neighborhood`, `make_recipe`, `gridded_stitching`, `gmat_stitching`) and introduce `mypy` in non-blocking mode. +- Split the very large [`stitches/fx_recipe.py`](../stitches/fx_recipe.py) (~1100 lines) into `recipe/permute.py`, `recipe/transitions.py`, `recipe/gridded.py`, preserving the public import surface in [`stitches/__init__.py`](../stitches/__init__.py) — a pure-refactor PR that must be output-identical. +- Replace the `fx_*` module naming with descriptive names, keeping deprecation shims for one minor release. + +### WS-6 — Performance + +Only after WS-1 benchmarks exist. Candidate targets, all validated as output-identical: + +- Vectorize the per-group Python loops in [`stitches/fx_match.py`](../stitches/fx_match.py:150) and [`stitches/make_matching_archive.py`](../stitches/make_matching_archive.py:92) (nested `for offset ... for key, d in groupby` is O(window × groups)). +- Avoid repeated `pd.concat` in loops (build lists then concat once — mostly done, but verify [`fx_stitch.py`](../stitches/fx_stitch.py:494)). +- Cache `pangeo_table.csv` / `matching_archive.csv` reads instead of re-reading per call ([`fx_recipe.py`](../stitches/fx_recipe.py:980), [`fx_recipe.py`](../stitches/fx_recipe.py:1126)). +- Consider Parquet instead of CSV for the shipped archives (faster load, smaller download) — output-identical after read. +- Evaluate `dask`/chunked reads in [`gridded_stitching`](../stitches/fx_stitch.py:245) for memory ceilings on large recipes. + +### WS-7 — Repository size and clone speed + +Implemented per [`plans/repo-clone-performance.md`](repo-clone-performance.md): + +- Add `nbstripout` (or `jupytext` paired scripts) to pre-commit so notebook outputs never enter history again. +- Deduplicate image assets; move large tutorial figures to the docs build or an external asset release. +- Decide on history rewrite (`git filter-repo`) vs. non-destructive mitigations (shallow/partial clone guidance, `git gc --aggressive`). + +### WS-8 — Documentation and community files + +- Add `CHANGELOG.md` (Keep a Changelog format) and start recording output-affecting changes explicitly. +- Regenerate the quickstart notebook against the current API; publish executed docs via `nbsphinx` at build time rather than committing rendered PNGs. +- Document the benchmark/regression workflow in [`docs/source/reference/contributing.rst`](../docs/source/reference/contributing.rst). +- Refresh badges and installation instructions in [`README.md`](../README.md); add a "supported Python versions" and "data version" table. +- Update `CITATION.cff` version/date and add a `SECURITY.md`. + +--- + +## 4. Sequencing + +```mermaid +flowchart TD + WS1[WS-1 Baseline + regression harness] --> WS2[WS-2 Packaging] + WS1 --> WS4[WS-4 Correctness fixes] + WS2 --> WS3[WS-3 Python matrix] + WS3 --> WS5[WS-5 Code quality] + WS4 --> WS5 + WS5 --> WS6[WS-6 Performance] + WS1 --> WS7[WS-7 Repo size] + WS6 --> WS8[WS-8 Docs and release] + WS7 --> WS8 +``` + +Ordering rules: + +1. Nothing that can change numerical output merges before WS-1 is green. +2. WS-7 history rewrite, if chosen, happens at a coordinated cut-over point and is announced to collaborators. +3. Release `0.14.0` after WS-2/3/4; release `1.0.0` after WS-5/6 once the API is typed and stable. + +--- + +## 5. Definition of Done + +- `pyproject.toml`-based build; `pip install stitches-emulator` works on Python 3.10–3.13 on all three OSes. +- `pytest` suite runs with zero silent skips; network/slow tests explicitly marked. +- Golden-output invariance suite passes; every intentional output change has a changelog entry citing the defect it corrects. +- Benchmark suite reports no regression beyond agreed thresholds. +- A fresh `git clone` completes in a fraction of the current time, with a documented measurement before/after. +- Docs build clean; changelog, citation, and README all reflect the released version. diff --git a/plans/repo-clone-performance.md b/plans/repo-clone-performance.md new file mode 100644 index 00000000..4b46cf49 --- /dev/null +++ b/plans/repo-clone-performance.md @@ -0,0 +1,332 @@ +# Why Cloning `stitches` Is Slow — Diagnosis and Solutions + +The checked-out working tree is modest (a dozen Python modules, a handful of small CSVs, a Sphinx site). The clone cost is therefore **almost entirely in `.git`**: object history, not the current snapshot. + +--- + +## 0. Measured Results (2026-08-28, branch `release/v1`) + +``` +working tree ~2 MB +.git 288 MB (size-pack 274.44 MiB, 5702 objects in 1 pack) +total blobs 784.1 MB uncompressed across all history +``` + +Blob bytes grouped by category: + +| Uncompressed | Category | In HEAD? | +|---:|---|---| +| 346.2 MB | `stitches/data/**` package data | No — `.gitignore`d | +| 188.3 MB | `notebooks/quickstart-ncs/*.nc` | No — deleted | +| 127.2 MB | `notebooks/stitches_dev/**` (HTML + CSV) | No — deleted | +| 82.9 MB | `*.ipynb` with embedded outputs | Partly | +| 20.2 MB | images / PDF | Partly | +| 16.3 MB | everything else | Mostly | +| 2.9 MB | HTML | No | + +**722.6 MB of 784.1 MB (92%) is blobs for paths that no longer exist in `HEAD`.** + +Worst individual offenders: + +| Size | Revs | Path | +|---:|---:|---| +| 94.16 MB | 1 | `notebooks/quickstart-ncs/stitched_CanESM5_tas_ssp245~r1i1p1f1~1.nc` | +| 94.16 MB | 1 | `notebooks/quickstart-ncs/stitched_CanESM5_pr_ssp245~r1i1p1f1~1.nc` | +| 77.10 MB | 1 | `stitches/data/pangeo_comparison_table.csv` | +| 56.47 MB | 5 | `stitches/data/pangeo_table.csv` | +| 49.89 MB | 3 | `stitches/data/matching_archive_staggered.csv` | +| 41.93 MB | 1 | `notebooks/stitches_dev/inputs/main_raw_pasted_tgav_anomaly_all_pangeo_list_models.csv` | +| 38.52 MB | **31** | `notebooks/stitches-quickstart.ipynb` | +| 28.33 MB | 12 | `notebooks/stitches_dev/Notebook6_throwout_Duplicates-across-ensemble-members.html` | +| 21.63 MB | 8 | `stitches/data/matching_archive.csv` | +| 20.53 MB | 2 | `stitches/data/created_data/main_tgav_all_pangeo_list_models.csv` | +| 11.51 MB | 3 | `stitches/data/tas-data/ACCESS-ESM1-5_tas.csv` | +| 10.86 MB | 1 | `notebooks/figs/Tutorial_2023_tgavex.tiff` | +| 10.14 MB | 8 | `notebooks/GCAM_AnnualMeeting2023.ipynb` | +| 7.85 MB | 2 | `stitches/data/tas_values.pkl` | + +Reproduce with: + +```bash +git rev-list --objects --all \ + | git cat-file --batch-check='%(objecttype) %(objectname) %(objectsize) %(rest)' \ + | awk '$1=="blob" {print $3, $4}' | sort -k2 \ + | awk '{s[$2]+=$1; n[$2]++} END {for (p in s) printf "%12.2f MB %4d rev %s\n", s[p]/1048576, n[p], p}' \ + | sort -rn | head -30 +``` + +**Conclusion: a history rewrite (§3.4) is clearly justified.** Removing the deleted-path blobs alone should reduce the clone by roughly an order of magnitude. Notebook output stripping (§3.2) is still required to stop the problem recurring — `stitches-quickstart.ipynb` alone has accumulated 38.5 MB across 31 revisions of an ~1 MB file. + +--- + +## 1. Root Causes + +### Cause A — Notebooks committed with embedded base64 outputs (primary) + +Four notebooks contain inline `image/png` payloads: + +| Notebook | Embedded PNG outputs found | +|---|---| +| [`notebooks/stitches-quickstart.ipynb`](../notebooks/stitches-quickstart.ipynb) | 6 | +| [`notebooks/stitches_takehome_GCAMAnnualMeeting2023.ipynb`](../notebooks/stitches_takehome_GCAMAnnualMeeting2023.ipynb) | 10 | +| [`notebooks/stitches_training_GCAMAnnualMeeting2023.ipynb`](../notebooks/stitches_training_GCAMAnnualMeeting2023.ipynb) | 6 | +| [`notebooks/preparing-input-data.ipynb`](../notebooks/preparing-input-data.ipynb) | 1 | + +Why this is disproportionately expensive: + +- Base64 inflates binary PNGs by ~33%. +- The payloads sit on **single enormous JSON lines**, so git's line-oriented delta compression is ineffective. +- Re-executing a notebook changes every image byte-for-byte (matplotlib metadata, font hinting, timestamps), plus `execution_count` and cell ids. Each commit that touches a notebook therefore stores an essentially **new full copy** of every figure in it. +- The training/takehome notebooks were produced with matplotlib 3.5.1 and the quickstart with 3.8.2, which is direct evidence of multiple re-execution generations living in history. + +Net effect: history accumulates N copies of every figure for N notebook commits. + +### Cause B — Package data and generated artifacts committed and later deleted (largest total) + +Confirmed by §0: 346.2 MB of `stitches/data/**`, 188.3 MB of stitched NetCDF outputs under `notebooks/quickstart-ncs/`, and 127.2 MB of rendered development notebooks under `notebooks/stitches_dev/`. None of these paths exist in `HEAD`. + +[`.gitignore`](../.gitignore:1) begins with a block of *external data* exclusions: + +``` +stitches/data/tas-data/*.* +stitches/data/temp-data/*.* +stitches/data/*.nc +stitches/data/matching_archive.csv +stitches/data/matching_archive_staggered.csv +stitches/data/pangeo_comparison_table.csv +stitches/data/pangeo_table.csv +stitches/data/*.csv +``` + +Rules that specific are typically written *after* the files caused a problem. If any of those CSV/NetCDF files (the archive is Zenodo record `8367628`, a multi-hundred-MB `data.zip`) were ever committed, `.gitignore` removes them from the *working tree* only — the blobs stay in history forever and are still transferred on clone. + +### Cause C — Duplicated and rendered binary assets + +- `stitches_diagram.jpg` exists three times: [`docs/source/getting-started/stitches_diagram.jpg`](../docs/source/getting-started/stitches_diagram.jpg), [`notebooks/figs/stitches_diagram.jpg`](../notebooks/figs/stitches_diagram.jpg), [`paper/stitches_diagram.jpg`](../paper/stitches_diagram.jpg). +- Committed Sphinx renders: `docs/source/getting-started/output_13_1.png`, `output_16_0.png`, `output_22_0.png`, `output_25_0.png`, `output_40_0.png`, `output_40_1.png` — these are *generated* artifacts that should be produced at docs-build time. +- Additional JPEGs under [`notebooks/figs/`](../notebooks/figs/) (`Tutorial_2023_T_dT_example.jpg`, `Tutorial_2023_tgavex.jpg`) and [`docs/source/images/`](../docs/source/images/). + +Binary assets never delta-compress well, and every revision of them is a full new blob. + +--- + +## 2. Measure Before You Act + +Run these and record the numbers in the PR that implements the fix. + +```bash +# Total repo size and pack breakdown +du -sh .git +git count-objects -vH + +# 20 largest blobs actually reachable in history, with their paths +git rev-list --objects --all \ + | git cat-file --batch-check='%(objecttype) %(objectname) %(objectsize) %(rest)' \ + | awk '$1=="blob"' \ + | sort -k3 -n -r \ + | head -20 + +# Size contribution grouped by path +git rev-list --objects --all \ + | git cat-file --batch-check='%(objecttype) %(objectname) %(objectsize) %(rest)' \ + | awk '$1=="blob" {print $3, $4}' \ + | sort -k2 \ + | awk '{s[$2]+=$1} END {for (p in s) print s[p], p}' \ + | sort -n -r | head -30 + +# Clone wall-clock baseline (cold) +time git clone --no-local /tmp/stitches-clonetest +``` + +`git-sizer` and `git filter-repo --analyze` both produce a nicer report: + +```bash +brew install git-sizer git-filter-repo +git-sizer --verbose +git filter-repo --analyze # writes .git/filter-repo/analysis/*.txt +``` + +Note: `git filter-repo --analyze` refuses to run in a repo with uncommitted changes and, like all `filter-repo` invocations, expects a fresh clone. Run it against a throwaway `--mirror` clone. + +The `analysis/path-all-sizes.txt` output directly confirms or refutes Causes A/B/C with hard numbers. + +--- + +## 3. Solutions + +### 3.1 Immediate relief for users (non-destructive, zero coordination) + +Document these in [`README.md`](../README.md) and the contributor guide: + +```bash +# Blobless clone — full history graph, blobs fetched on demand. Usually the best default. +git clone --filter=blob:none https://github.com/JGCRI/stitches.git + +# Shallow clone — for CI or read-only use +git clone --depth 1 https://github.com/JGCRI/stitches.git + +# Single branch, shallow — smallest +git clone --depth 1 --single-branch --branch main https://github.com/JGCRI/stitches.git +``` + +Also apply `--filter=blob:none` in CI checkouts: + +```yaml +- uses: actions/checkout@v4 + with: + filter: blob:none + fetch-depth: 1 +``` + +This mitigates the symptom immediately without touching history. + +### 3.2 Stop the bleeding (required, do this first) + +1. **Strip notebook outputs on commit.** Add to [`.pre-commit-config.yaml`](../.pre-commit-config.yaml): + + ```yaml + - repo: https://github.com/kynan/nbstripout + rev: 0.7.1 + hooks: + - id: nbstripout + ``` + + Optionally add a `.gitattributes` filter as a second line of defense: + + ``` + *.ipynb filter=nbstripout + ``` + +2. **Guard against large files.** Add `check-added-large-files` to the existing `pre-commit-hooks` block: + + ```yaml + - id: check-added-large-files + args: ['--maxkb=512'] + ``` + +3. **Stop committing generated docs images.** `docs/source/getting-started/output_*.png` is 1.65 MB across six files. + + Verified caveat: they are **currently referenced** by six `.. image::` + directives in + [`docs/source/getting-started/quickstarter.rst`](../docs/source/getting-started/quickstarter.rst), + so deleting them breaks the docs build. Removing them properly means + converting `quickstarter.rst` into an `nbsphinx`-executed notebook + (`nbsphinx_execute = "always"`, with the package data cached) so the figures + are generated at build time. That is a docs-restructuring job, tracked under + WS-8, not a drive-by deletion. + +4. **Deduplicate `stitches_diagram.jpg`.** Confirmed byte-identical in all three + locations (md5 `fba15a18445d1bba43eee5bdff95d51e`), roughly 200 KB each: + `docs/source/getting-started/`, `notebooks/figs/`, `paper/`. + + Caveat: the `paper/` copy belongs to the JOSS submission, which builds + separately via [`draft-pdf.yml`](../.github/workflows/draft-pdf.yml) and should + stay reproducible as published. Deduplicate the `docs/` and `notebooks/` + copies and leave `paper/` alone. At ~400 KB of potential saving this is minor + next to §3.4, and not worth risking the paper build. + +Trade-off to accept: stripped notebooks show no output on GitHub's static renderer. Mitigate by publishing executed notebooks in the hosted docs (already wired via `nbsphinx`) and linking to them from the README. + +Measured effect of stripping outputs from the four notebooks currently in the +working tree: + +| Notebook | Now | Stripped | +|---|---:|---:| +| `stitches-quickstart.ipynb` | 2694.6 KB | 29.0 KB | +| `stitches_takehome_GCAMAnnualMeeting2023.ipynb` | 2428.2 KB | 47.7 KB | +| `stitches_training_GCAMAnnualMeeting2023.ipynb` | 1372.1 KB | 29.0 KB | +| `preparing-input-data.ipynb` | 515.1 KB | 16.8 KB | +| **Total** | **7010.1 KB** | **122.5 KB (98% smaller)** | + +That 7 MB is re-stored in full on every commit that touches a notebook, which is +how a ~1 MB file accumulated 38.5 MB of history across 31 revisions. + +### 3.3 Structural options for the notebooks + +| Option | Clone cost | GitHub preview | Effort | +|---|---|---|---| +| `nbstripout` (outputs removed) | Low | No outputs shown | Low | +| `jupytext` paired `.py:percent` + generated `.ipynb` untracked | Lowest | No notebook in repo | Medium | +| Keep outputs but move figures to external files referenced by path | Medium | Outputs shown | Medium | +| Keep as-is | High | Outputs shown | None | + +Recommendation: `nbstripout` now; consider `jupytext` pairing for the two GCAM-meeting notebooks, which are historical training artifacts and arguably belong in a tagged release asset rather than `main`. + +### 3.4 History rewrite (optional, destructive, highest payoff) + +Only worth doing if §2 shows that historical blobs dominate. This is the only way to actually shrink an existing clone. + +```bash +# Work on a fresh mirror, never your working clone +git clone --mirror https://github.com/JGCRI/stitches.git stitches-mirror +cd stitches-mirror + +# Drop historical package data, stitched outputs, and dev notebook renders. +# Paths below are the confirmed offenders from section 0; none exist in HEAD, +# so removing them cannot affect the current tree. +git filter-repo \ + --path 'notebooks/quickstart-ncs' \ + --path 'notebooks/stitches_dev' \ + --path 'stitches/data/created_data' \ + --path-glob 'stitches/data/tas-data/*' \ + --path-glob 'stitches/data/temp-data/*' \ + --path-glob 'stitches/data/*.nc' \ + --path-glob 'stitches/data/*.pkl' \ + --path-glob 'stitches/data/matching_archive*.csv' \ + --path-glob 'stitches/data/pangeo_*table.csv' \ + --path-glob '*.tiff' \ + --path-glob 'docs/source/getting-started/output_*.png' \ + --invert-paths + +# Verify nothing in HEAD was removed +git diff --stat HEAD # expect: empty + +# Strip notebook outputs from every historical revision +git filter-repo --force \ + --path-glob '*.ipynb' \ + --blob-callback ' +import json +try: + nb = json.loads(blob.data) +except Exception: + pass +else: + if isinstance(nb, dict) and "cells" in nb: + for c in nb["cells"]: + c["outputs"] = [] + c["execution_count"] = None + blob.data = json.dumps(nb, indent=1).encode() +' + +git reflog expire --expire=now --all +git gc --prune=now --aggressive +``` + +Consequences that must be planned for: + +- **Every commit SHA changes.** All open PRs must be rebased or recreated; all forks must re-clone or re-base. +- Zenodo DOI archives and the JOSS paper reference specific commits/tags — verify tags survive the rewrite (`git filter-repo` preserves them but re-points them) and keep a **read-only archived mirror** of the pre-rewrite history for citation integrity. +- Requires a force-push to a protected branch: temporarily lift protection, announce a freeze window, then restore. + +Pre-flight checklist: + +- [ ] §2 measurements recorded, showing the expected savings +- [ ] Archived mirror pushed to a `stitches-history-archive` repository +- [ ] All maintainers notified with a freeze window and re-clone instructions +- [ ] Open PR inventory captured +- [ ] Tags and releases verified post-rewrite on the mirror before force-push +- [ ] Post-rewrite clone time re-measured and recorded + +### 3.5 If large files must live in the repo + +Prefer *not* to. The package already downloads science data from Zenodo at runtime via [`stitches/install_pkgdata.py`](../stitches/install_pkgdata.py), which is the correct pattern. If any binary must be versioned, use Git LFS or a GitHub Release asset rather than a normal blob. + +--- + +## 4. Recommended Plan of Record + +1. Measure (§2) and publish the numbers. +2. Land the preventive hooks and asset cleanup (§3.2) — safe, immediate, no coordination. +3. Document `--filter=blob:none` for users and switch CI to filtered/shallow checkout (§3.1). +4. Decide on the rewrite (§3.4) based on the §2 evidence; if the historical blobs are under ~50 MB, skip it and rely on partial clone. +5. Re-measure clone time and record the before/after in `CHANGELOG.md`. diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..730cfd81 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,96 @@ +[build-system] +requires = ["setuptools>=64", "wheel"] +build-backend = "setuptools.build_meta" + +[project] +name = "stitches-emulator" +description = "Amalgamate existing climate data to create monthly climate variable fields" +readme = "README.md" +license = { text = "BSD-2-Clause" } +# 3.9 reached end of life; 3.10 is the floor. See plans/development-plan.md. +requires-python = ">=3.10" +authors = [ + { name = "Abigail Snyder", email = "abigail.snyder@pnnl.gov" }, + { name = "Kalyn Dorheim", email = "kalyn.dorheim@pnnl.gov" }, + { name = "Claudia Tebaldi", email = "claudia.tebaldi@pnnl.gov" }, +] +keywords = ["climate", "emulator", "CMIP6", "downscaling", "temperature"] +classifiers = [ + "Development Status :: 4 - Beta", + "Intended Audience :: Science/Research", + "License :: OSI Approved :: BSD License", + "Natural Language :: English", + "Operating System :: OS Independent", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3 :: Only", + "Topic :: Scientific/Engineering :: Atmospheric Science", +] + +dependencies = [ + "matplotlib>=3.3.2", + "xarray>=2022.9.0", + "numpy>=1.23.3", + "pandas>=2.1.0", + "intake>=0.6.6", + "intake-esm>=2021.8.17", + "nc_time_axis>=1.4.1", + "scikit-learn>=1.1", + "fsspec[gcs]>=2022.5.0", + "tqdm>=4.64.1", + # `requests` is imported by stitches/install_pkgdata.py but was never + # declared, so a clean install could fail at install_package_data(). + "requests>=2.28", +] + +# Single source of truth for the version: stitches/_version.py. +dynamic = ["version"] + +[project.optional-dependencies] +test = [ + "pytest>=7.4.3", + "pytest-cov>=4.1", + "pytest-benchmark>=4.0", + # Golden regression artifacts are stored as Parquet. + "pyarrow>=14.0", +] +docs = [ + "nbsphinx>=0.8.6", + "sphinx>=7", + "sphinx_book_theme>=1", + "sphinx-click>=5.1", + "sphinx_copybutton>=0.5", +] +dev = [ + "stitches-emulator[test,docs]", + "build>=1.0", + "setuptools>=64", + "twine>=5.0", + "pre-commit>=3.6.0", +] + +[project.urls] +Homepage = "https://github.com/JGCRI/stitches" +Documentation = "https://jgcri.github.io/stitches/" +Repository = "https://github.com/JGCRI/stitches" +Issues = "https://github.com/JGCRI/stitches/issues" + +[tool.setuptools] +include-package-data = true + +[tool.setuptools.packages.find] +include = ["stitches*"] + +[tool.setuptools.dynamic] +version = { attr = "stitches._version.__version__" } + +[tool.setuptools.package-data] +stitches = [ + "data/README.md", + "data/example/*.csv", + "data/tas-data/README.md", + "data/temp-data/README.md", +] diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 00000000..ad81673c --- /dev/null +++ b/pytest.ini @@ -0,0 +1,27 @@ +[pytest] +# Test discovery. +# `testpaths` only sets the DEFAULT search path, so a bare `pytest` run collects +# tests/ and skips benchmarks/. Passing an explicit path (`pytest benchmarks`) +# overrides it. `bench_*.py` is listed in python_files so benchmark modules are +# discovered when that path is given. +testpaths = tests +python_files = test_*.py bench_*.py +python_classes = Test* +python_functions = test_* + +# Benchmarks live in benchmarks/ and are excluded from the default run because +# they measure performance rather than correctness. Run them explicitly with: +# pytest benchmarks --benchmark-only +addopts = + --strict-markers + -ra + +markers = + network: requires internet access (Pangeo catalog, Zenodo data download). Skipped unless --network is passed or STITCHES_TEST_NETWORK=1 is set. + slow: long-running test (>30s wall clock). Skipped unless --slow is passed or STITCHES_TEST_SLOW=1 is set. + package_data: requires the full Zenodo-minted package data to be installed. + regression: golden-output invariance test; asserts scientific outputs are unchanged. + benchmark: performance measurement, not a correctness assertion. + +filterwarnings = + default diff --git a/setup.cfg b/setup.cfg deleted file mode 100644 index 08aedd7e..00000000 --- a/setup.cfg +++ /dev/null @@ -1,2 +0,0 @@ -[metadata] -description_file = README.md diff --git a/setup.py b/setup.py deleted file mode 100644 index feb8201f..00000000 --- a/setup.py +++ /dev/null @@ -1,68 +0,0 @@ -""" -The setup.py file for the stitches package. - -The stitches package provides tools for stitching together climate model output -into a single, coherent dataset. -""" - - -import re - -from setuptools import find_packages, setup - - -def requirements(): - """ - Read and parse the 'requirements.txt' file. - - Returns a list of requirements specified in the 'requirements.txt' file. - - :return: A list of package requirements. - :rtype: list - """ - with open("requirements.txt") as f: - return f.read().split() - - -version = re.search( - r"__version__ = ['\"]([^'\"]*)['\"]", open("stitches/_version.py").read(), re.M -).group(1) - -setup( - name="stitches-emulator", - version=version, - packages=find_packages(), - url="https://github.com/JGCRI/stitches", - license="BSD 2-Clause", - author="Abigail Snyder, Kalyn Dorheim, Claudia Tebaldi", - author_email="abigail.snyder@pnnl.gov, kalyn.dorheim@pnnl.gov, claudia.tebaldi@pnnl.gov", - description="Amalgamate existing climate data to create monthly climate variable fields", - python_requires=">=3.9.0", - include_package_data=True, - install_requires=requirements(), - extras_require={ - "dev": [ - "pytest>=7.4.3", - "build>=0.5.1", - "nbsphinx>=0.8.6", - "setuptools>=57.0.0", - "sphinx>=7", - "sphinx_book_theme>=1", - "sphinx-click>=5.1", - "sphinx_copybutton>=0.5", - "twine~=3.4.1", - "pre-commit>=3.6.0", - ] - }, - classifiers=[ - "Development Status :: 4 - Beta", - "Intended Audience :: Science/Research", - "License :: OSI Approved :: BSD License", - "Natural Language :: English", - "Operating System :: OS Independent", - "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.9", - "Programming Language :: Python :: 3 :: Only", - "Topic :: Scientific/Engineering :: Atmospheric Science", - ], -) diff --git a/stitches/fx_match.py b/stitches/fx_match.py index d417b125..38c651e1 100644 --- a/stitches/fx_match.py +++ b/stitches/fx_match.py @@ -78,15 +78,20 @@ def internal_dist(fx_pt, dx_pt, archivedata, tol=0): # Internal fx -def shuffle_function(dt): +def shuffle_function(dt, seed=None): """ Randomly shuffle the deck to assist with the matching process. :param dt: A DataFrame of archive values used in the matching process. + :param seed: Optional seed for the random number generator. When ``None`` + (the default) the shuffle is nondeterministic, preserving the + historical behavior. Pass an integer to obtain a reproducible + shuffle without mutating global random state. + :type seed: int or None :return: A DataFrame with rows in random order. """ nrow = dt.shape[0] - out = dt.sample(nrow, replace=False) + out = dt.sample(nrow, replace=False, random_state=seed) out = out.reset_index(drop=True) return out diff --git a/stitches/fx_processing.py b/stitches/fx_processing.py index b8c88ff6..f8c80c2d 100644 --- a/stitches/fx_processing.py +++ b/stitches/fx_processing.py @@ -51,8 +51,11 @@ def calculate_rolling_mean(data, size): # dplyr group_by %>% mutate() call. # rename so that value is the smoothed data: + # Note: `axis=1` must not be passed alongside `columns=`; pandas 3.0 raises + # "Cannot specify both 'axis' and 'index'/'columns'" for that combination. + # `columns=` already implies the column axis, so the intent is unchanged. rslt = ( - rslt.drop(columns="value", axis=1) + rslt.drop(columns="value") .rename(columns={"rollingAvg": "value"}) .reset_index(drop=True) ) @@ -160,7 +163,12 @@ def get_chunk_info(df): # stored in a data frame. model = LinearRegression() model.fit(x_input, y_input) - dx = float(model.coef_[0]) + # `y_input` is a column vector, so `coef_` has shape (1, 1) and + # `coef_[0]` is a 1-element array rather than a scalar. Converting a + # 1-element array with `float()` was deprecated in NumPy 1.25 and raises + # TypeError in NumPy 2. `ravel()[0]` selects the single coefficient + # explicitly and yields the identical value. + dx = float(model.coef_.ravel()[0]) # Format the the chunk data into a pandas data frame. row = pd.DataFrame( diff --git a/stitches/fx_recipe.py b/stitches/fx_recipe.py index cecbbe07..9d51801b 100644 --- a/stitches/fx_recipe.py +++ b/stitches/fx_recipe.py @@ -248,7 +248,12 @@ def remove_duplicates(md, archive): def permute_stitching_recipes( - N_matches: int, matched_data, archive, optional=None, testing: bool = False + N_matches: int, + matched_data, + archive, + optional=None, + testing: bool = False, + seed=None, ): """ Sample from `matched_data` to produce permutations of stitching recipes. @@ -267,11 +272,24 @@ def permute_stitching_recipes( to avoid re-making (this is not implemented). :param testing: When True, the behavior can be reliably replicated without setting global seeds. - Defaults to False. + Equivalent to ``seed=1``. Defaults to False. :type testing: bool + :param seed: Optional seed for the per-window archive draw. When ``None`` + (the default) the seed is taken from ``testing`` for backward + compatibility: ``testing=True`` behaves as ``seed=1`` and + ``testing=False`` leaves the draw nondeterministic. Prefer + passing ``seed`` explicitly; it makes results reproducible + without the other connotations of a "testing" flag. + :type seed: int or None + :return: A data frame with the same structure as the raw matched data, with duplicate matches replaced. """ + # Resolve the effective random_state once so the draw below has a single + # source of truth. `testing=True` historically meant `random_state=1`, so + # that mapping is preserved exactly when `seed` is not supplied. + if seed is None and testing: + seed = 1 # Check inputs util.check_columns( matched_data, @@ -441,12 +459,12 @@ def permute_stitching_recipes( # For each target window group, # Randomly select one of the archive matches to use. # This creates one_one_match, a candidate recipe. + # Note: `seed` is applied per group rather than to a single shared + # generator. This reproduces the original `random_state=1` behavior + # exactly; changing it would alter published outputs. one_one_match = [] for name, group in grouped_targets: - if testing: - one_one_match.append(group.sample(1, replace=False, random_state=1)) - else: - one_one_match.append(group.sample(1, replace=False)) + one_one_match.append(group.sample(1, replace=False, random_state=seed)) one_one_match = pd.concat(one_one_match) one_one_match = one_one_match.reset_index(drop=True).copy() @@ -1028,6 +1046,7 @@ def make_recipe( tol: float = 0.1, non_tas_variables: [str] = None, reproducible: bool = False, + seed=None, ): """ Generate a stitching recipe from target and archive data. @@ -1040,13 +1059,17 @@ def make_recipe( :param non_tas_variables: List of variables other than tas to stitch together; defaults to None, which stitches tas only. :param reproducible: If True, ensures reproducible behavior by using the testing=True argument - in permute_stitching_recipes(); defaults to False. + in permute_stitching_recipes(); defaults to False. Equivalent to ``seed=1``. + :param seed: Optional seed forwarded to `permute_stitching_recipes`. When + ``None`` (the default) the seed is derived from ``reproducible`` for + backward compatibility. Prefer passing ``seed`` explicitly. :type N_matches: int :type res: str :type tol: float :type non_tas_variables: list[str] :type reproducible: bool + :type seed: int or None :return: A pandas DataFrame of a formatted recipe. """ @@ -1166,20 +1189,13 @@ def make_recipe( # Match the archive & target data together. match_df = match.match_neighborhood(target_data, archive_data, tol=tol) - if reproducible: - unformatted_recipe = permute_stitching_recipes( - N_matches=N_matches, - matched_data=match_df, - archive=archive_data, - testing=True, - ) - else: - unformatted_recipe = permute_stitching_recipes( - N_matches=N_matches, - matched_data=match_df, - archive=archive_data, - testing=False, - ) + unformatted_recipe = permute_stitching_recipes( + N_matches=N_matches, + matched_data=match_df, + archive=archive_data, + testing=reproducible, + seed=seed, + ) # Format the recipe into the dataframe that can be used by the stitching functions. recipe = generate_gridded_recipe(unformatted_recipe, res=res) diff --git a/stitches/install_pkgdata.py b/stitches/install_pkgdata.py index 0fc071cb..62f35abd 100644 --- a/stitches/install_pkgdata.py +++ b/stitches/install_pkgdata.py @@ -1,18 +1,32 @@ -"""This module contains the InstallPackageData class for downloading and unpacking example data from Zenodo.""" +"""Download and unpack the Zenodo-minted package data for stitches. -import importlib +The data archive is large (hundreds of megabytes), so the download is streamed to +a temporary file rather than buffered in memory, and progress is reported through +``tqdm``. +""" + +import logging import os import shutil import tempfile import zipfile from importlib import resources -from io import BytesIO as BytesIO import requests from tqdm import tqdm from ._version import __version__ +logger = logging.getLogger(__name__) + +#: Seconds to wait for the server to respond before giving up. Applies to the +#: connection and to each read, not to the transfer as a whole, so a slow but +#: progressing download is not killed. +DEFAULT_TIMEOUT = (10, 60) + +#: Bytes per streamed chunk. +CHUNK_SIZE = 1024 * 1024 + class InstallPackageData: """ @@ -24,6 +38,8 @@ class InstallPackageData: :param data_dir: Optional. Full path to the directory where you wish to store the data. If not specified, the data will be installed in the data directory of the package. :type data_dir: str + :param timeout: Optional. ``(connect, read)`` timeout in seconds passed to + ``requests``. """ # URL for DOI minted example data hosted on Zenodo @@ -40,78 +56,181 @@ class InstallPackageData: DEFAULT_VERSION = "https://zenodo.org/records/8367628/files/data.zip?download=1" - def __init__(self, data_dir=None): + #: Subdirectories that must exist before extraction. + SUBDIRECTORIES = ("tas-data", "temp-data") + + #: File extensions extracted from the archive. + KEEP_EXTENSIONS = (".csv", ".nc") + + def __init__(self, data_dir=None, timeout=DEFAULT_TIMEOUT): """ Initialize the InstallPackageData class. :param data_dir: The directory where the data will be stored. If None, the data will be installed in the package's data directory. :type data_dir: str, optional + :param timeout: ``(connect, read)`` timeout in seconds. """ self.data_dir = data_dir + self.timeout = timeout - def fetch_zenodo(self): - """Download and unpack the Zenodo minted data for the current stitches distribution.""" - # full path to the stitches root directory where the example dir will be stored - if self.data_dir is None: - data_directory = resources.files("stitches") / "data" - else: - data_directory = self.data_dir - - # build needed subdirectories if they do not already exist - tas_data_path = os.path.join(data_directory, "tas-data") - temp_data_path = os.path.join(data_directory, "temp-data") - if not os.path.exists(tas_data_path): - os.mkdir(tas_data_path) - if not os.path.exists(temp_data_path): - os.mkdir(temp_data_path) + def resolve_url(self, version=None): + """Return the data URL registered for ``version``. - # get the current version of stitches that is installed - current_version = __version__ + :param version: Version to look up; defaults to the installed version. + :return: The download URL. + :rtype: str + """ + version = version or __version__ try: - data_link = InstallPackageData.DATA_VERSION_URLS[current_version] - + return self.DATA_VERSION_URLS[version] except KeyError: - msg = f"Link to data missing for current version: {current_version}." - msg += f" Using default version: {InstallPackageData.DEFAULT_VERSION}" + # Previously this silently substituted DEFAULT_VERSION, so a version + # with no registered dataset would quietly download a possibly + # mismatched archive. Warn loudly instead; the fallback is retained + # so that development versions remain usable. + logger.warning( + "No data archive is registered for stitches version %s. " + "Falling back to %s, which may not match this code. " + "Register the correct URL in InstallPackageData.DATA_VERSION_URLS.", + version, + self.DEFAULT_VERSION, + ) + return self.DEFAULT_VERSION + + def target_directory(self): + """Return the directory the data will be written into. + + :rtype: str + """ + if self.data_dir is None: + return str(resources.files("stitches") / "data") + return str(self.data_dir) - data_link = InstallPackageData.DEFAULT_VERSION + def download(self, url, destination): + """Stream ``url`` to ``destination`` with a progress bar. - print(msg) + The archive is written to disk incrementally rather than accumulated in + memory. The previous implementation held the entire response in a + ``BytesIO``, which needed hundreds of megabytes of RAM and produced no + progress output despite importing ``tqdm``. - # retrieve content from URL - print( - f"Downloading example data for stitches version {current_version}. This may take a few minutes..." - ) - response = requests.get(data_link) + :param url: URL to fetch. + :param destination: Local file path to write to. + :raises requests.HTTPError: If the server returns an error status. + """ + with requests.get(url, stream=True, timeout=self.timeout) as response: + # Without this, an HTML error page would be happily written out and + # then fail later with a confusing "not a zip file" error. + response.raise_for_status() + + total = int(response.headers.get("content-length", 0)) + + with open(destination, "wb") as handle, tqdm( + total=total or None, + unit="B", + unit_scale=True, + unit_divisor=1024, + desc="Downloading stitches data", + ) as progress: + for chunk in response.iter_content(chunk_size=CHUNK_SIZE): + if not chunk: + continue + handle.write(chunk) + progress.update(len(chunk)) + + def extract(self, archive_path, data_directory): + """Extract the wanted members of ``archive_path`` into ``data_directory``. + + Only ``.csv`` and ``.nc`` members are kept, and the archive's directory + nesting is flattened except that ``tas-data`` and ``temp-data`` members + are placed in the matching subdirectory. + + :param archive_path: Path to the downloaded zip file. + :param data_directory: Directory to extract into. + :return: The list of files written. + :rtype: list[str] + """ + written = [] + + # Create the destination layout here rather than relying on the caller + # having done it. `extract` is usable on its own, and a missing + # subdirectory would otherwise surface as an opaque FileNotFoundError + # from deep inside shutil.copy. + os.makedirs(data_directory, exist_ok=True) + for subdirectory in self.SUBDIRECTORIES: + os.makedirs(os.path.join(data_directory, subdirectory), exist_ok=True) + + with zipfile.ZipFile(archive_path) as zipped: + members = [ + name + for name in zipped.namelist() + if os.path.splitext(name)[-1] in self.KEEP_EXTENSIONS + ] + + for name in tqdm(members, desc="Extracting", unit="file"): + basename = os.path.basename(name) + + # Preserve the subdirectory layout the package expects. The + # original code handled tas-data only, so temp-data members were + # flattened into the top level even though the directory was + # created for them. + for subdirectory in self.SUBDIRECTORIES: + if subdirectory in name: + basename = os.path.join(subdirectory, basename) + break + + out_file = os.path.join(data_directory, basename) + + with tempfile.TemporaryDirectory() as tdir: + zipped.extract(name, tdir) + shutil.copy(os.path.join(tdir, name), out_file) + + written.append(out_file) + logger.debug("Unzipped: %s", out_file) + + return written - with zipfile.ZipFile(BytesIO(response.content)) as zipped: - # extract each file in the zipped dir to the project - for f in zipped.namelist(): - extension = os.path.splitext(f)[-1] + def fetch_zenodo(self): + """Download and unpack the Zenodo minted data for the current stitches distribution. - # Extract only the csv and nc files - if all([len(extension) > 0, extension in (".csv", ".nc")]): - basename = os.path.basename(f) + :return: The list of files written. + :rtype: list[str] + """ + data_directory = self.target_directory() + + # `os.makedirs(exist_ok=True)` rather than `os.mkdir`, which fails when an + # intermediate parent is missing and races against concurrent callers. + for subdirectory in self.SUBDIRECTORIES: + os.makedirs(os.path.join(data_directory, subdirectory), exist_ok=True) + + data_link = self.resolve_url() + + logger.info( + "Downloading example data for stitches version %s. This may take a few minutes.", + __version__, + ) - # Check to see if tas-data is in the file path - if "tas-data" in f: - basename = os.path.join("tas-data", basename) + # Download to a temporary file so a failed or interrupted transfer cannot + # leave a truncated archive behind to be mistaken for a good one. + with tempfile.TemporaryDirectory() as tdir: + archive_path = os.path.join(tdir, "data.zip") - out_file = os.path.join(data_directory, basename) + self.download(data_link, archive_path) - # extract to a temporary directory to be able to only keep the file out of the dir structure - with tempfile.TemporaryDirectory() as tdir: - # extract file to temporary directory - zipped.extract(f, tdir) + try: + written = self.extract(archive_path, data_directory) + except zipfile.BadZipFile as exc: + raise RuntimeError( + f"The file downloaded from {data_link} is not a valid zip archive. " + "The download may have been truncated, or the URL may no longer " + "point at the data archive." + ) from exc - # construct temporary file full path with name - tfile = os.path.join(tdir, f) + logger.info("Wrote %d files to %s", len(written), data_directory) - print(f"Unzipped: {out_file}") - # transfer only the file sans the parent directory to the data package - shutil.copy(tfile, out_file) + return written def install_package_data(data_dir: str = None): @@ -125,8 +244,9 @@ def install_package_data(data_dir: str = None): Default is the data directory of the package. :type data_dir: str - :return: None + :return: The list of files written. + :rtype: list[str] """ zen = InstallPackageData(data_dir=data_dir) - zen.fetch_zenodo() + return zen.fetch_zenodo() diff --git a/stitches/make_tas_archive.py b/stitches/make_tas_archive.py index 6fade3be..f8da120d 100644 --- a/stitches/make_tas_archive.py +++ b/stitches/make_tas_archive.py @@ -11,6 +11,45 @@ import stitches.fx_util as util +def write_tas_data_by_model(data_frame, output_dir): + """Write one CSV of global tas values per model. + + Split out of `make_tas_archive` so the filename construction can be tested + without running the multi-hour archive build. + + Two defects were fixed here: + + 1. The path was built as ``output_dir + "/" + name + "_tas.csv"``. Because + `importlib.resources.files` returns a ``Traversable`` (a ``PosixPath`` in + practice), ``Traversable + str`` raises ``TypeError``. ``os.path.join`` on + ``str(output_dir)`` is used instead, which also fixes the Windows + separator. + 2. Grouping used ``groupby(["model"])``. With a *list* of keys, pandas 2.0+ + yields a one-element ``tuple`` as the group name, so the filenames would + have become ``('BCC-CSM2-MR',)_tas.csv``. Grouping by the scalar key + ``"model"`` yields the plain string the filename expects. + + :param data_frame: Global tas data containing a ``model`` column. + :type data_frame: pandas.DataFrame + :param output_dir: Directory to write the per-model CSV files into. Created + if it does not already exist. + :return: The list of paths written, ordered by model name. + :rtype: list[str] + """ + util.check_columns(data_frame, {"model"}) + + directory = str(output_dir) + os.makedirs(directory, exist_ok=True) + + files = [] + for name, group in data_frame.groupby("model"): + path = os.path.join(directory, f"{name}_tas.csv") + files.append(path) + group.to_csv(path, index=False) + + return files + + def join_exclude(dat, drop): """Drop some rows from a data frame. @@ -445,15 +484,9 @@ def make_tas_archive(anomaly_startYr=1995, anomaly_endYr=2014): data["zstore"] = new_zstore # Save a copy of the tas values, these are the value that will be used to get the - # tas data chunks. Note that this file has to be compressed so will need to read in - # using pickle_utils.load() - - files = [] + # tas data chunks. tas_data_dir = resources.files("stitches") / "data" / "tas-data" - for name, group in data.groupby(["model"]): - path = tas_data_dir + "/" + name + "_tas.csv" - files.append(path) - group.to_csv(path, index=False) + files = write_tas_data_by_model(data, tas_data_dir) print("Global tas data complete") diff --git a/tests/conftest.py b/tests/conftest.py index fdb3d4a7..109d1abc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,18 +1,169 @@ -"""This module contains pytest fixtures and configurations for testing the stitches package.""" +"""Pytest fixtures and configuration for the stitches test suite. +Design goals: + +* **No implicit network access.** The previous version of this module installed + the full Zenodo-minted package data in a session-scoped ``autouse`` fixture, + which meant every ``pytest`` invocation -- including a run of a single pure + unit test -- downloaded hundreds of megabytes. Package data is now an + explicit, opt-in fixture (``package_data``) that individual tests request. +* **No silent skips.** Tests that need the network or the full data archive are + marked (``network``, ``package_data``, ``slow``) and are deselected with a + visible skip reason rather than passing vacuously. + +Opt-in mechanisms, in precedence order: + +=================== ========================= ================================ +Capability CLI flag Environment variable +=================== ========================= ================================ +Network access ``--network`` ``STITCHES_TEST_NETWORK=1`` +Long-running tests ``--slow`` ``STITCHES_TEST_SLOW=1`` +Full package data ``--package-data`` ``STITCHES_TEST_PACKAGE_DATA=1`` +=================== ========================= ================================ + +``--package-data`` implies ``--network`` on first use, since the archive must be +downloaded before it can be used. +""" + +import os +from importlib import resources +from pathlib import Path + +import pandas as pd import pytest -import stitches +# --------------------------------------------------------------------------- +# Command line options +# --------------------------------------------------------------------------- + +#: Maps a marker name to its ``(cli flag, environment variable)`` pair. +_OPT_IN_MARKERS = { + "network": ("--network", "STITCHES_TEST_NETWORK"), + "slow": ("--slow", "STITCHES_TEST_SLOW"), + "package_data": ("--package-data", "STITCHES_TEST_PACKAGE_DATA"), +} + + +def pytest_addoption(parser): + """Register the opt-in flags for capability-gated tests.""" + group = parser.getgroup("stitches") + group.addoption( + "--network", + action="store_true", + default=False, + help="Run tests marked 'network' that require internet access.", + ) + group.addoption( + "--slow", + action="store_true", + default=False, + help="Run tests marked 'slow' (>30s wall clock).", + ) + group.addoption( + "--package-data", + action="store_true", + default=False, + help=( + "Run tests marked 'package_data', downloading the Zenodo-minted " + "archive if it is not already present. Implies --network." + ), + ) + group.addoption( + "--update-golden", + action="store_true", + default=False, + help=( + "Regenerate golden regression artifacts from the current code " + "instead of asserting against them. Use only when an output change " + "is intentional, and record the justification in CHANGELOG.md." + ), + ) + + +def _enabled(config, marker): + """Return True when the capability behind ``marker`` has been opted into.""" + flag, env_var = _OPT_IN_MARKERS[marker] + if config.getoption(flag.lstrip("-").replace("-", "_")): + return True + if os.environ.get(env_var, "").strip().lower() in {"1", "true", "yes"}: + return True + # Downloading package data necessarily requires the network, so enabling + # package data also enables the network capability. + if marker == "network" and _enabled(config, "package_data"): + return True + return False + + +def pytest_collection_modifyitems(config, items): + """Skip capability-gated tests unless the capability was opted into.""" + for marker in _OPT_IN_MARKERS: + if _enabled(config, marker): + continue + flag, env_var = _OPT_IN_MARKERS[marker] + skip = pytest.mark.skip( + reason=f"needs {marker!r}: pass {flag} or set {env_var}=1 to run" + ) + for item in items: + if marker in item.keywords: + item.add_marker(skip) + +# --------------------------------------------------------------------------- +# Data fixtures +# --------------------------------------------------------------------------- -@pytest.fixture(scope="session", autouse=True) -def setup_package_data(): + +@pytest.fixture(scope="session") +def example_data_dir(): + """Return the directory holding the small committed example CSV fixtures. + + These files ship inside the package and require no download, so they back + the offline tier of the test suite. + """ + return Path(str(resources.files("stitches") / "data" / "example")) + + +@pytest.fixture(scope="session") +def read_example(example_data_dir): + """Return a loader for a committed example CSV by stem name. + + Usage:: + + def test_something(read_example): + target = read_example("test-target_dat") """ - Set up the package data for testing. - This fixture is automatically used in tests that require the package data. - It installs the package data and prepares the testing environment. + def _read(name): + stem = name[:-4] if name.endswith(".csv") else name + path = example_data_dir / f"{stem}.csv" + if not path.is_file(): + raise FileNotFoundError(f"No example fixture named {stem!r} at {path}") + return pd.read_csv(path) + + return _read + + +@pytest.fixture(scope="session") +def package_data(): + """Ensure the full Zenodo-minted package data is installed. + + Tests requesting this fixture must also carry the ``package_data`` marker so + that they are deselected when the capability is not opted into. The download + is performed at most once per session and is skipped when the data already + appears to be present. """ - stitches.install_package_data() + import stitches + + data_dir = Path(str(resources.files("stitches") / "data")) + sentinel = data_dir / "matching_archive.csv" + + if not sentinel.is_file(): + stitches.install_package_data() + + if not sentinel.is_file(): + pytest.fail( + f"package data installation did not produce {sentinel}; " + "cannot run package_data-marked tests" + ) - return None + return data_dir diff --git a/tests/regression/__init__.py b/tests/regression/__init__.py new file mode 100644 index 00000000..3e84fdcf --- /dev/null +++ b/tests/regression/__init__.py @@ -0,0 +1,27 @@ +"""Golden-output regression tests for the stitches package. + +The purpose of this package is to guarantee **output invariance**: refactors, +dependency upgrades, and performance work must not change the numbers that +stitches produces, except where the current output is provably wrong. + +How it works +------------ + +Each test calls a function and compares the result against a *golden* artifact +recorded from a known-good baseline. Artifacts live in ``golden/`` as Parquet +files (stable, typed, compact -- unlike CSV, which invites float-formatting +drift that can both mask real diffs and manufacture fake ones). + +Regenerating goldens +-------------------- + +Golden files are regenerated only deliberately:: + + pytest tests/regression --update-golden + +Policy: any PR that changes a file under ``tests/regression/golden/`` **must** +include a ``CHANGELOG.md`` entry under ``### Changed -- outputs`` explaining +which defect the new output corrects, and must be reviewed by a domain +maintainer rather than only a code reviewer. An unexplained golden update +defeats the entire purpose of this suite. +""" diff --git a/tests/regression/conftest.py b/tests/regression/conftest.py new file mode 100644 index 00000000..d2cba91c --- /dev/null +++ b/tests/regression/conftest.py @@ -0,0 +1,267 @@ +"""Comparison helpers and fixtures for the golden-output regression suite. + +The central helper is :func:`assert_matches_golden`, which compares a DataFrame +against a recorded Parquet artifact. + +Canonicalization +---------------- + +Before comparison both frames are passed through :func:`canonicalize`, which +sorts columns by name and rows by their full contents. This deliberately makes +the comparison insensitive to row and column *ordering* while remaining fully +sensitive to row and column *content*. + +The rationale: ``groupby``/``concat``-heavy code legitimately changes iteration +order across pandas versions and across refactors that are otherwise pure. If +ordering were significant, the suite would emit false failures on every such +change and would quickly be ignored. Where ordering is genuinely part of the +contract -- for example, `make_recipe` sorts by ``stitching_id`` and +``target_start_yr`` before returning -- it is asserted explicitly by a dedicated +test rather than implicitly by the golden comparison. + +Tolerances +---------- + +Integral and categorical data must match exactly. Floating point results are +compared with a tight but nonzero tolerance, because NumPy and pandas are free +to reassociate reductions between versions; demanding bit-equality there would +produce failures that carry no scientific meaning. See ``TOLERANCES``. +""" + +import hashlib +import json +from pathlib import Path + +import pandas as pd +import pytest +from pandas.testing import assert_frame_equal + +#: Directory holding recorded golden artifacts. +GOLDEN_DIR = Path(__file__).parent / "golden" + +#: Named tolerance presets, keyed by the kind of quantity being compared. +#: +#: ``exact`` - indices, years, labels, filenames, counts. Any change is a bug. +#: ``distance`` - the L2/dx/fx match distances; a few ULP of drift is acceptable. +#: ``value`` - temperature values derived from means over the archive. +#: ``field`` - gridded NetCDF field values, which accumulate more error. +TOLERANCES = { + "exact": {"rtol": 0.0, "atol": 0.0}, + "distance": {"rtol": 1e-12, "atol": 0.0}, + "value": {"rtol": 1e-10, "atol": 0.0}, + "field": {"rtol": 1e-6, "atol": 1e-9}, +} + + +def canonicalize(df): + """Return ``df`` in a deterministic row and column order. + + Columns are sorted by name and rows are sorted by their full contents using + a stable sort, so the result depends only on the *set* of rows and columns + present, not on the order the producing code happened to emit them in. + + :param df: The DataFrame to canonicalize. + :type df: pandas.DataFrame + :return: A new DataFrame with sorted columns, sorted rows, and a reset index. + :rtype: pandas.DataFrame + """ + out = df.reindex(sorted(df.columns), axis=1) + if len(out) > 0: + out = out.sort_values(list(out.columns), kind="mergesort") + return out.reset_index(drop=True) + + +def frame_digest(df): + """Return a short, stable SHA-256 digest of a canonicalized DataFrame. + + Used for artifacts that are too large to vendor, where only a checksum is + stored. Note that this is exact by construction and therefore cannot express + a floating-point tolerance; reserve it for frames whose values are integral + or categorical, or where any change at all is worth investigating. + + :param df: The DataFrame to digest. + :type df: pandas.DataFrame + :return: The first 32 hex characters of the digest. + :rtype: str + """ + canonical = canonicalize(df) + hashed = pd.util.hash_pandas_object(canonical, index=False) + return hashlib.sha256(hashed.values.tobytes()).hexdigest()[:32] + + +class GoldenComparer: + """Compares frames against golden artifacts, or rewrites them on request. + + Instantiated by the :func:`golden` fixture. When ``update`` is True the + comparison methods write the incoming data to disk and pass, which is how + ``--update-golden`` regenerates the suite. + """ + + def __init__(self, update=False, golden_dir=GOLDEN_DIR): + self.update = update + self.golden_dir = Path(golden_dir) + #: Relative paths written during this session, for reporting. + self.written = [] + + # -- frames ---------------------------------------------------------- + + def assert_frame(self, actual, name, tolerance="distance"): + """Assert ``actual`` matches the golden Parquet artifact ``name``. + + :param actual: The frame produced by the code under test. + :type actual: pandas.DataFrame + :param name: Artifact path relative to the golden directory, without + the ``.parquet`` suffix, e.g. ``"match/tol0"``. + :type name: str + :param tolerance: Key into :data:`TOLERANCES`. + :type tolerance: str + """ + if tolerance not in TOLERANCES: + raise ValueError( + f"unknown tolerance {tolerance!r}; expected one of {sorted(TOLERANCES)}" + ) + + path = self.golden_dir / f"{name}.parquet" + + if self.update: + path.parent.mkdir(parents=True, exist_ok=True) + canonicalize(actual).to_parquet(path, index=False) + self.written.append(str(path.relative_to(self.golden_dir))) + return + + if not path.is_file(): + pytest.fail( + f"missing golden artifact {path}.\n" + f"Generate it with: pytest tests/regression --update-golden" + ) + + expected = pd.read_parquet(path) + + actual_c = canonicalize(actual) + expected_c = canonicalize(expected) + + # Report a shape or column mismatch directly; assert_frame_equal's + # message for these cases is much harder to read. + if list(actual_c.columns) != list(expected_c.columns): + raise AssertionError( + f"{name}: column mismatch\n" + f" unexpected: {sorted(set(actual_c.columns) - set(expected_c.columns))}\n" + f" missing: {sorted(set(expected_c.columns) - set(actual_c.columns))}" + ) + if len(actual_c) != len(expected_c): + raise AssertionError( + f"{name}: row count changed: expected {len(expected_c)}, got {len(actual_c)}" + ) + + assert_frame_equal( + actual_c, + expected_c, + # Integer width and categorical-vs-object differences vary across + # platforms and pandas versions without any change in meaning. + check_dtype=False, + check_categorical=False, + obj=f"golden[{name}]", + **TOLERANCES[tolerance], + ) + + # -- digests --------------------------------------------------------- + + def assert_digest(self, actual, name): + """Assert the digest of ``actual`` matches the recorded JSON digest. + + For artifacts too large to vendor as Parquet. + + :param actual: The frame produced by the code under test. + :type actual: pandas.DataFrame + :param name: Artifact path relative to the golden directory, without + the ``.json`` suffix. + :type name: str + """ + path = self.golden_dir / f"{name}.json" + digest = frame_digest(actual) + record = {"digest": digest, "rows": int(len(actual)), "columns": sorted(actual.columns)} + + if self.update: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(record, indent=2, sort_keys=True) + "\n") + self.written.append(str(path.relative_to(self.golden_dir))) + return + + if not path.is_file(): + pytest.fail( + f"missing golden digest {path}.\n" + f"Generate it with: pytest tests/regression --update-golden" + ) + + expected = json.loads(path.read_text()) + + assert record["columns"] == expected["columns"], f"{name}: columns changed" + assert record["rows"] == expected["rows"], ( + f"{name}: row count changed: expected {expected['rows']}, got {record['rows']}" + ) + assert digest == expected["digest"], ( + f"{name}: content digest changed.\n" + f" expected {expected['digest']}\n" + f" actual {digest}\n" + f"If this change is intentional and corrects a defect, regenerate with " + f"--update-golden and document it in CHANGELOG.md." + ) + + # -- scalars --------------------------------------------------------- + + def assert_values(self, actual, name): + """Assert a JSON-serializable mapping matches the recorded values. + + Useful for pinning things that are not frames, such as generated + filenames or summary statistics. + + :param actual: A JSON-serializable mapping. + :type actual: dict + :param name: Artifact path relative to the golden directory, without + the ``.json`` suffix. + :type name: str + """ + path = self.golden_dir / f"{name}.json" + + if self.update: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(actual, indent=2, sort_keys=True) + "\n") + self.written.append(str(path.relative_to(self.golden_dir))) + return + + if not path.is_file(): + pytest.fail( + f"missing golden values {path}.\n" + f"Generate it with: pytest tests/regression --update-golden" + ) + + expected = json.loads(path.read_text()) + assert actual == expected, f"{name}: recorded values changed" + + +@pytest.fixture(scope="session") +def golden(request): + """Return a :class:`GoldenComparer` honoring the ``--update-golden`` flag.""" + update = request.config.getoption("update_golden") + comparer = GoldenComparer(update=update) + + if update: + comparer.golden_dir.mkdir(parents=True, exist_ok=True) + + yield comparer + + if update and comparer.written: + print( + f"\n--update-golden rewrote {len(comparer.written)} artifact(s) under " + f"{comparer.golden_dir}:\n " + "\n ".join(sorted(comparer.written)) + + "\n\nRemember to document intentional output changes in CHANGELOG.md." + ) + + +# Note: the `regression` marker is applied declaratively via a module-level +# `pytestmark` in each test module rather than from a hook or autouse fixture +# here. `-m` filtering is applied by pytest's own collection hook, and a marker +# added from another `pytest_collection_modifyitems` is not reliably visible to +# it, which made `pytest -m regression` deselect the whole suite. Since the CI +# job selects on this marker, a silent full-suite skip is the worst possible +# failure mode, so the marker is declared where it cannot be missed. diff --git a/tests/regression/golden/data/global_mean.parquet b/tests/regression/golden/data/global_mean.parquet new file mode 100644 index 00000000..f7483e7e Binary files /dev/null and b/tests/regression/golden/data/global_mean.parquet differ diff --git a/tests/regression/golden/match/drop_hist_false_duplicates.parquet b/tests/regression/golden/match/drop_hist_false_duplicates.parquet new file mode 100644 index 00000000..1d6b8822 Binary files /dev/null and b/tests/regression/golden/match/drop_hist_false_duplicates.parquet differ diff --git a/tests/regression/golden/match/internal_dist.parquet b/tests/regression/golden/match/internal_dist.parquet new file mode 100644 index 00000000..7ea5f446 Binary files /dev/null and b/tests/regression/golden/match/internal_dist.parquet differ diff --git a/tests/regression/golden/match/internal_dist_tol0.1.parquet b/tests/regression/golden/match/internal_dist_tol0.1.parquet new file mode 100644 index 00000000..7ea5f446 Binary files /dev/null and b/tests/regression/golden/match/internal_dist_tol0.1.parquet differ diff --git a/tests/regression/golden/match/neighborhood_row_counts.json b/tests/regression/golden/match/neighborhood_row_counts.json new file mode 100644 index 00000000..41a94777 --- /dev/null +++ b/tests/regression/golden/match/neighborhood_row_counts.json @@ -0,0 +1,7 @@ +{ + "0.0": 56, + "0.05": 66, + "0.1": 66, + "0.2": 66, + "0.5": 66 +} diff --git a/tests/regression/golden/match/neighborhood_self.parquet b/tests/regression/golden/match/neighborhood_self.parquet new file mode 100644 index 00000000..942a47e2 Binary files /dev/null and b/tests/regression/golden/match/neighborhood_self.parquet differ diff --git a/tests/regression/golden/match/neighborhood_tol0.1_dedup.parquet b/tests/regression/golden/match/neighborhood_tol0.1_dedup.parquet new file mode 100644 index 00000000..3bbe69fa Binary files /dev/null and b/tests/regression/golden/match/neighborhood_tol0.1_dedup.parquet differ diff --git a/tests/regression/golden/match/neighborhood_tol0.1_nodedup.parquet b/tests/regression/golden/match/neighborhood_tol0.1_nodedup.parquet new file mode 100644 index 00000000..b94a7156 Binary files /dev/null and b/tests/regression/golden/match/neighborhood_tol0.1_nodedup.parquet differ diff --git a/tests/regression/golden/match/neighborhood_tol0.5_dedup.parquet b/tests/regression/golden/match/neighborhood_tol0.5_dedup.parquet new file mode 100644 index 00000000..3bbe69fa Binary files /dev/null and b/tests/regression/golden/match/neighborhood_tol0.5_dedup.parquet differ diff --git a/tests/regression/golden/match/neighborhood_tol0_dedup.parquet b/tests/regression/golden/match/neighborhood_tol0_dedup.parquet new file mode 100644 index 00000000..2077ba63 Binary files /dev/null and b/tests/regression/golden/match/neighborhood_tol0_dedup.parquet differ diff --git a/tests/regression/golden/match/neighborhood_tol0_nodedup.parquet b/tests/regression/golden/match/neighborhood_tol0_nodedup.parquet new file mode 100644 index 00000000..2077ba63 Binary files /dev/null and b/tests/regression/golden/match/neighborhood_tol0_nodedup.parquet differ diff --git a/tests/regression/golden/match/shuffle_seed20240101.parquet b/tests/regression/golden/match/shuffle_seed20240101.parquet new file mode 100644 index 00000000..5c41a3f3 Binary files /dev/null and b/tests/regression/golden/match/shuffle_seed20240101.parquet differ diff --git a/tests/regression/golden/processing/chunk_info_n5.parquet b/tests/regression/golden/processing/chunk_info_n5.parquet new file mode 100644 index 00000000..029344d3 Binary files /dev/null and b/tests/regression/golden/processing/chunk_info_n5.parquet differ diff --git a/tests/regression/golden/processing/chunk_info_n9.parquet b/tests/regression/golden/processing/chunk_info_n9.parquet new file mode 100644 index 00000000..3a393287 Binary files /dev/null and b/tests/regression/golden/processing/chunk_info_n9.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n20.parquet b/tests/regression/golden/processing/chunk_ts_n20.parquet new file mode 100644 index 00000000..9b1cf48a Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n20.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n5.parquet b/tests/regression/golden/processing/chunk_ts_n5.parquet new file mode 100644 index 00000000..52108106 Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n5.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n9.parquet b/tests/regression/golden/processing/chunk_ts_n9.parquet new file mode 100644 index 00000000..1f60a6fc Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n9.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n9_base0.parquet b/tests/regression/golden/processing/chunk_ts_n9_base0.parquet new file mode 100644 index 00000000..1f60a6fc Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n9_base0.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n9_base1.parquet b/tests/regression/golden/processing/chunk_ts_n9_base1.parquet new file mode 100644 index 00000000..1b5565f6 Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n9_base1.parquet differ diff --git a/tests/regression/golden/processing/chunk_ts_n9_base4.parquet b/tests/regression/golden/processing/chunk_ts_n9_base4.parquet new file mode 100644 index 00000000..9ba4d347 Binary files /dev/null and b/tests/regression/golden/processing/chunk_ts_n9_base4.parquet differ diff --git a/tests/regression/golden/processing/rolling_mean_w11.parquet b/tests/regression/golden/processing/rolling_mean_w11.parquet new file mode 100644 index 00000000..fb2fb8fd Binary files /dev/null and b/tests/regression/golden/processing/rolling_mean_w11.parquet differ diff --git a/tests/regression/golden/processing/rolling_mean_w3.parquet b/tests/regression/golden/processing/rolling_mean_w3.parquet new file mode 100644 index 00000000..40fe0c7e Binary files /dev/null and b/tests/regression/golden/processing/rolling_mean_w3.parquet differ diff --git a/tests/regression/golden/processing/rolling_mean_w9.parquet b/tests/regression/golden/processing/rolling_mean_w9.parquet new file mode 100644 index 00000000..9b96d2a1 Binary files /dev/null and b/tests/regression/golden/processing/rolling_mean_w9.parquet differ diff --git a/tests/regression/golden/processing/subset_archive.parquet b/tests/regression/golden/processing/subset_archive.parquet new file mode 100644 index 00000000..bd61059c Binary files /dev/null and b/tests/regression/golden/processing/subset_archive.parquet differ diff --git a/tests/regression/golden/recipe/handle_final_period.parquet b/tests/regression/golden/recipe/handle_final_period.parquet new file mode 100644 index 00000000..fb9053c2 Binary files /dev/null and b/tests/regression/golden/recipe/handle_final_period.parquet differ diff --git a/tests/regression/golden/recipe/handle_transition_periods.parquet b/tests/regression/golden/recipe/handle_transition_periods.parquet new file mode 100644 index 00000000..74785ef9 Binary files /dev/null and b/tests/regression/golden/recipe/handle_transition_periods.parquet differ diff --git a/tests/regression/golden/recipe/num_perms_guide.parquet b/tests/regression/golden/recipe/num_perms_guide.parquet new file mode 100644 index 00000000..a9d86f55 Binary files /dev/null and b/tests/regression/golden/recipe/num_perms_guide.parquet differ diff --git a/tests/regression/golden/recipe/num_perms_targets.parquet b/tests/regression/golden/recipe/num_perms_targets.parquet new file mode 100644 index 00000000..9ce5dc41 Binary files /dev/null and b/tests/regression/golden/recipe/num_perms_targets.parquet differ diff --git a/tests/regression/golden/recipe/permute_N1_seed1.parquet b/tests/regression/golden/recipe/permute_N1_seed1.parquet new file mode 100644 index 00000000..8a12b574 Binary files /dev/null and b/tests/regression/golden/recipe/permute_N1_seed1.parquet differ diff --git a/tests/regression/golden/recipe/permute_N2_seed1.parquet b/tests/regression/golden/recipe/permute_N2_seed1.parquet new file mode 100644 index 00000000..8a12b574 Binary files /dev/null and b/tests/regression/golden/recipe/permute_N2_seed1.parquet differ diff --git a/tests/regression/golden/recipe/permute_N2_tol0.2_seed1.parquet b/tests/regression/golden/recipe/permute_N2_tol0.2_seed1.parquet new file mode 100644 index 00000000..39e99693 Binary files /dev/null and b/tests/regression/golden/recipe/permute_N2_tol0.2_seed1.parquet differ diff --git a/tests/regression/golden/recipe/remove_duplicates.parquet b/tests/regression/golden/recipe/remove_duplicates.parquet new file mode 100644 index 00000000..5634dd0e Binary files /dev/null and b/tests/regression/golden/recipe/remove_duplicates.parquet differ diff --git a/tests/regression/golden/util/anti_join.parquet b/tests/regression/golden/util/anti_join.parquet new file mode 100644 index 00000000..603d4247 Binary files /dev/null and b/tests/regression/golden/util/anti_join.parquet differ diff --git a/tests/regression/golden/util/combine_df.parquet b/tests/regression/golden/util/combine_df.parquet new file mode 100644 index 00000000..854332ca Binary files /dev/null and b/tests/regression/golden/util/combine_df.parquet differ diff --git a/tests/regression/test_invariance_match.py b/tests/regression/test_invariance_match.py new file mode 100644 index 00000000..7c06d8a2 --- /dev/null +++ b/tests/regression/test_invariance_match.py @@ -0,0 +1,166 @@ +"""Golden-output regression tests for the matching functions. + +Covers :mod:`stitches.fx_match`. All fixtures are the small committed CSVs under +``stitches/data/example``, so these tests run entirely offline and are cheap +enough to gate every pull request. +""" + +import pandas as pd +import pytest + +from stitches.fx_match import ( + drop_hist_false_duplicates, + internal_dist, + match_neighborhood, + shuffle_function, +) + +# Every test in this module is an output-invariance check; the CI regression job +# selects on this marker. +pytestmark = pytest.mark.regression + + +@pytest.fixture(scope="module") +def target(read_example): + """Return the example target data.""" + return read_example("test-target_dat") + + +@pytest.fixture(scope="module") +def archive(read_example): + """Return the example archive data.""" + return read_example("test-archive_dat") + + +# --------------------------------------------------------------------------- +# match_neighborhood +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "tol,dedup,name", + [ + (0.0, True, "match/neighborhood_tol0_dedup"), + (0.0, False, "match/neighborhood_tol0_nodedup"), + (0.1, True, "match/neighborhood_tol0.1_dedup"), + (0.1, False, "match/neighborhood_tol0.1_nodedup"), + (0.5, True, "match/neighborhood_tol0.5_dedup"), + ], +) +def test_match_neighborhood_invariant(golden, target, archive, tol, dedup, name): + """`match_neighborhood` output is unchanged across the tolerance sweep. + + The tolerance sweep matters because ``tol`` controls how many archive points + fall inside each target neighborhood, exercising both the single-match and + many-match branches of the distance filtering. + """ + out = match_neighborhood(target, archive, tol=tol, drop_hist_duplicates=dedup) + golden.assert_frame(out, name, tolerance="distance") + + +def test_match_neighborhood_self_match_invariant(golden, target): + """Matching the target against itself is unchanged. + + A self-match is the degenerate case where every distance should be zero, so + it isolates the distance computation from the neighborhood selection. + """ + out = match_neighborhood(target, target, tol=0) + golden.assert_frame(out, "match/neighborhood_self", tolerance="distance") + + +def test_match_neighborhood_self_match_distances_are_zero(target): + """Every distance in a self-match is exactly zero. + + This is an absolute property rather than a recorded value, so it is asserted + directly instead of against a golden file. + """ + out = match_neighborhood(target, target, tol=0) + + for column in ("dist_dx", "dist_fx", "dist_l2"): + assert (out[column] == 0).all(), f"{column} is not identically zero" + + +def test_match_neighborhood_row_count_is_stable(golden, target, archive): + """Row counts per tolerance are unchanged. + + Recorded separately from the frames so that a change in *how many* matches + are produced is reported as its own clearly-named failure. + """ + counts = { + str(tol): int(len(match_neighborhood(target, archive, tol=tol))) + for tol in (0.0, 0.05, 0.1, 0.2, 0.5) + } + golden.assert_values(counts, "match/neighborhood_row_counts") + + +# --------------------------------------------------------------------------- +# internal_dist +# --------------------------------------------------------------------------- + + +def test_internal_dist_invariant(golden, target): + """`internal_dist` output for a fixed probe point is unchanged.""" + out = internal_dist(target.fx[0], target.dx[0], target) + golden.assert_frame(out, "match/internal_dist", tolerance="distance") + + +def test_internal_dist_with_tolerance_invariant(golden, target): + """`internal_dist` with a nonzero tolerance is unchanged.""" + out = internal_dist(target.fx[0], target.dx[0], target, tol=0.1) + golden.assert_frame(out, "match/internal_dist_tol0.1", tolerance="distance") + + +# --------------------------------------------------------------------------- +# drop_hist_false_duplicates +# --------------------------------------------------------------------------- + + +def test_drop_hist_false_duplicates_invariant(golden, read_example): + """`drop_hist_false_duplicates` output is unchanged. + + This function resolves ties by keeping the row with the minimum ``idvalue``, + which is order-sensitive, so pinning it guards against an incidental change + in groupby iteration order silently changing which duplicate survives. + """ + match_data = read_example("test-match_w_dup") + out = drop_hist_false_duplicates(match_data) + golden.assert_frame(out, "match/drop_hist_false_duplicates", tolerance="distance") + + +def test_drop_hist_false_duplicates_leaves_one_historical_experiment(read_example): + """Only one historical target experiment survives deduplication. + + An invariant of the function's purpose: identical historical data pasted into + every future experiment must collapse to a single representative. + """ + match_data = read_example("test-match_w_dup") + cleaned = drop_hist_false_duplicates(match_data) + + hist = cleaned[cleaned["target_start_yr"] <= 2020]["target_experiment"].unique() + assert len(hist) == 1 + + +# --------------------------------------------------------------------------- +# shuffle_function +# --------------------------------------------------------------------------- + + +def test_shuffle_function_seeded_invariant(golden, target): + """A seeded shuffle produces an unchanged permutation. + + Pins the permutation so that a future change to how the seed is threaded + through (for example, moving to a single shared generator) is caught rather + than silently altering user-visible reproducible output. + """ + out = shuffle_function(target, seed=20240101) + golden.assert_frame(out, "match/shuffle_seed20240101", tolerance="value") + + +def test_shuffle_function_preserves_contents(target): + """Shuffling is a pure permutation: no rows are added, dropped, or altered.""" + out = shuffle_function(target, seed=7) + + pd.testing.assert_frame_equal( + out.sort_values(list(out.columns)).reset_index(drop=True), + target.sort_values(list(target.columns)).reset_index(drop=True), + ) diff --git a/tests/regression/test_invariance_processing.py b/tests/regression/test_invariance_processing.py new file mode 100644 index 00000000..aba6d30f --- /dev/null +++ b/tests/regression/test_invariance_processing.py @@ -0,0 +1,233 @@ +"""Golden-output regression tests for the time-series processing functions. + +Covers :mod:`stitches.fx_processing`, which performs the smoothing and chunking +that produce the ``fx`` (level) and ``dx`` (rate of change) values the matching +algorithm depends on. An error here silently propagates into every recipe, so +these are among the most valuable invariants in the suite. + +The fixtures are synthetic but deterministic: a smooth warming trend plus a fixed +oscillation, built from a seeded generator so the series is byte-identical on +every platform. Synthetic data is preferable here because it lets the window +arithmetic be exercised at exact, known lengths. +""" + +import numpy as np +import pandas as pd +import pytest + +from stitches.fx_processing import ( + calculate_rolling_mean, + chunk_ts, + get_chunk_info, + subset_archive, +) + +# Every test in this module is an output-invariance check; the CI regression job +# selects on this marker. +pytestmark = pytest.mark.regression + + +def _series(start=1850, end=2100, model="test_model", experiment="ssp245", ensemble="r1i1p1f1"): + """Build a deterministic synthetic annual temperature series. + + The signal combines a quadratic warming trend, a decadal oscillation, and a + small seeded pseudo-random component. The seeded component matters: it + prevents the rolling mean and the linear fit from operating on a perfectly + smooth curve, where a bug in window centering could go unnoticed. + + :return: A DataFrame with the columns the processing functions require. + :rtype: pandas.DataFrame + """ + years = np.arange(start, end + 1) + n = len(years) + + # Fixed generator seed keeps this reproducible across platforms and versions. + rng = np.random.default_rng(20240101) + trend = 0.00012 * (years - start) ** 2 + oscillation = 0.35 * np.sin(np.arange(n) / 11.0) + noise = rng.normal(0.0, 0.05, size=n) + + return pd.DataFrame( + { + "year": years, + "value": trend + oscillation + noise, + "variable": "tas", + "model": model, + "experiment": experiment, + "ensemble": ensemble, + "unit": "K", + } + ) + + +@pytest.fixture(scope="module") +def series(): + """Return the synthetic single-realization series.""" + return _series() + + +@pytest.fixture(scope="module") +def multi_series(): + """Return a multi-realization, multi-experiment series. + + Needed because `calculate_rolling_mean` groups by + model/experiment/ensemble/variable; a single group would not exercise the + grouping at all. + """ + frames = [ + _series(experiment=exp, ensemble=ens) + for exp in ("historical", "ssp245", "ssp585") + for ens in ("r1i1p1f1", "r2i1p1f1") + ] + return pd.concat(frames).reset_index(drop=True) + + +# --------------------------------------------------------------------------- +# calculate_rolling_mean +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("size", [3, 9, 11]) +def test_rolling_mean_invariant(golden, multi_series, size): + """`calculate_rolling_mean` output is unchanged across window sizes. + + Window size 9 is the package default; 3 and 11 bracket it so an off-by-one in + the window arithmetic shows up as a shape or edge-value change. + """ + out = calculate_rolling_mean(multi_series.copy(), size) + golden.assert_frame(out, f"processing/rolling_mean_w{size}", tolerance="value") + + +def test_rolling_mean_preserves_row_count(multi_series): + """Smoothing returns one row per input row. + + The implementation uses ``min_periods=1`` specifically so that the first and + last ``(size-1)/2`` years are retained rather than becoming NaN. This asserts + that intent directly. + """ + out = calculate_rolling_mean(multi_series.copy(), 9) + + assert len(out) == len(multi_series) + assert out["value"].notna().all() + + +def test_rolling_mean_edges_are_not_nan(multi_series): + """The window edges carry real values, not NaN.""" + out = calculate_rolling_mean(multi_series.copy(), 9) + column = "rollingAvg" if "rollingAvg" in out.columns else "value" + + assert out[column].notna().all(), f"{column} contains NaN at the series edges" + + +# --------------------------------------------------------------------------- +# chunk_ts +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("n", [5, 9, 20]) +def test_chunk_ts_invariant(golden, series, n): + """`chunk_ts` chunk assignment is unchanged across window sizes.""" + out = chunk_ts(series.copy(), n) + golden.assert_frame(out, f"processing/chunk_ts_n{n}", tolerance="value") + + +@pytest.mark.parametrize("base_chunk", [0, 1, 4]) +def test_chunk_ts_staggered_invariant(golden, series, base_chunk): + """`chunk_ts` staggered offsets are unchanged. + + ``base_chunk`` drops leading years to build staggered archives. Pinning + several offsets guards the slicing arithmetic. + """ + out = chunk_ts(series.copy(), 9, base_chunk=base_chunk) + golden.assert_frame(out, f"processing/chunk_ts_n9_base{base_chunk}", tolerance="value") + + +def test_chunk_ts_chunk_sizes(series): + """Every chunk holds ``n`` years except a possibly short final chunk.""" + n = 9 + out = chunk_ts(series.copy(), n) + sizes = out.groupby("chunk").size() + + assert (sizes.iloc[:-1] == n).all(), "an interior chunk is not n years long" + assert 0 < sizes.iloc[-1] <= n, "final chunk size out of range" + + +def test_chunk_ts_rejects_base_chunk_larger_than_n(series): + """``base_chunk > n`` is rejected.""" + with pytest.raises(TypeError): + chunk_ts(series.copy(), 5, base_chunk=6) + + +def test_chunk_ts_rejects_multiple_variables(series): + """A frame holding more than one variable is rejected.""" + mixed = series.copy() + mixed.loc[mixed.index[:10], "variable"] = "pr" + + with pytest.raises(TypeError): + chunk_ts(mixed, 9) + + +# --------------------------------------------------------------------------- +# get_chunk_info +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("n", [5, 9]) +def test_get_chunk_info_invariant(golden, series, n): + """`get_chunk_info` fx and dx values are unchanged. + + ``dx`` comes from a scikit-learn ``LinearRegression`` fit per chunk, so this + test also pins the package against a change in that dependency's numerics. + """ + chunked = chunk_ts(series.copy(), n) + out = get_chunk_info(chunked) + golden.assert_frame(out, f"processing/chunk_info_n{n}", tolerance="value") + + +def test_get_chunk_info_one_row_per_chunk(series): + """`get_chunk_info` summarizes each chunk to exactly one row.""" + chunked = chunk_ts(series.copy(), 9) + out = get_chunk_info(chunked) + + assert len(out) == chunked["chunk"].nunique() + + +def test_get_chunk_info_years_are_integers(series): + """Year columns are returned as integers, not floats or categoricals.""" + chunked = chunk_ts(series.copy(), 9) + out = get_chunk_info(chunked) + + for column in ("start_yr", "end_yr", "year"): + assert pd.api.types.is_integer_dtype(out[column]), f"{column} is not integral" + + +def test_get_chunk_info_year_bounds_are_consistent(series): + """Each chunk's representative year lies within its own start/end bounds.""" + chunked = chunk_ts(series.copy(), 9) + out = get_chunk_info(chunked) + + assert (out["start_yr"] <= out["year"]).all() + assert (out["year"] <= out["end_yr"]).all() + + +# --------------------------------------------------------------------------- +# subset_archive +# --------------------------------------------------------------------------- + + +def test_subset_archive_invariant(golden, series): + """`subset_archive` selection is unchanged.""" + staggered = pd.concat( + [chunk_ts(series.copy(), 9, base_chunk=offset) for offset in range(0, 3)] + ).reset_index(drop=True) + info = get_chunk_info( + chunk_ts(series.copy(), 9) + ) + end_years = sorted(info["end_yr"].unique())[:5] + + archive = info.copy() + out = subset_archive(archive, end_years) + golden.assert_frame(out, "processing/subset_archive", tolerance="value") + + assert set(out["end_yr"]).issubset(set(end_years)) + assert len(staggered) > 0 # staggered construction must not silently produce nothing diff --git a/tests/regression/test_invariance_recipe.py b/tests/regression/test_invariance_recipe.py new file mode 100644 index 00000000..23757d3a --- /dev/null +++ b/tests/regression/test_invariance_recipe.py @@ -0,0 +1,276 @@ +"""Golden-output regression tests for the recipe construction functions. + +Covers :mod:`stitches.fx_recipe`, the most algorithmically involved part of the +package. `permute_stitching_recipes` samples archive points per target window +while enforcing several non-local constraints (no duplicate archive points, no +envelope collapse across generated realizations), and the transition handlers +then rewrite window boundaries. Small changes here are easy to make by accident +and hard to notice, which is exactly what these goldens are for. + +All randomized calls pass an explicit ``seed`` so the recorded artifacts are +reproducible. Tests that must not depend on a particular draw assert structural +properties instead of recorded values. +""" + +import pandas as pd +import pytest + +from stitches.fx_match import match_neighborhood +from stitches.fx_recipe import ( + get_num_perms, + handle_final_period, + handle_transition_periods, + permute_stitching_recipes, + remove_duplicates, +) + +# The synthetic target/archive pair used by the existing unit tests. Reused here +# so the regression suite and the unit suite describe the same scenario. +from tests.test_fx_recipe import TestRecipe + +# Every test in this module is an output-invariance check; the CI regression job +# selects on this marker. +pytestmark = pytest.mark.regression + +TARGET_DATA = TestRecipe.TARGET_DATA +ARCHIVE_DATA = TestRecipe.ARCHIVE_DATA + + +@pytest.fixture(scope="module") +def matched(): + """Return matched data at the tolerance used by the existing unit tests.""" + return match_neighborhood(TARGET_DATA, ARCHIVE_DATA, tol=0.07) + + +@pytest.fixture(scope="module") +def matched_wide(): + """Return matched data at a wider tolerance, giving more candidates per window.""" + return match_neighborhood(TARGET_DATA, ARCHIVE_DATA, tol=0.2) + + +@pytest.fixture(scope="module") +def matched_nn(): + """Return nearest-neighbor matches: exactly one archive point per target year. + + ``remove_duplicates`` requires singular matches and raises `TypeError` + otherwise, so it must be fed ``tol=0`` output rather than a permuted recipe. + """ + return match_neighborhood(TARGET_DATA, ARCHIVE_DATA, tol=0.0) + + +# --------------------------------------------------------------------------- +# get_num_perms +# --------------------------------------------------------------------------- + + +def test_get_num_perms_targets_invariant(golden, matched): + """The per-target permutation summary is unchanged. + + ``get_num_perms`` decides how many collapse-free realizations each target can + support, which in turn drives the order targets are processed in. A change + here reorders the whole construction. + """ + targets, _ = get_num_perms(matched) + golden.assert_frame(targets, "recipe/num_perms_targets", tolerance="exact") + + +def test_get_num_perms_guide_invariant(golden, matched): + """The per-window permutation guide is unchanged.""" + _, guide = get_num_perms(matched) + golden.assert_frame(guide, "recipe/num_perms_guide", tolerance="exact") + + +def test_get_num_perms_counts_are_positive(matched): + """Every target window has at least one candidate match.""" + _, guide = get_num_perms(matched) + count_column = "n_matches" if "n_matches" in guide.columns else guide.columns[-1] + + assert (guide[count_column] > 0).all() + + +# --------------------------------------------------------------------------- +# remove_duplicates +# --------------------------------------------------------------------------- + + +def test_remove_duplicates_invariant(golden, matched_nn): + """`remove_duplicates` output is unchanged. + + Where two target years claim the same archive point, the closer one keeps it + and the other is re-matched against an archive with the taken points removed. + This pins which target year wins and what the loser is re-matched to. + """ + out = remove_duplicates(md=matched_nn.copy(), archive=ARCHIVE_DATA) + golden.assert_frame(out, "recipe/remove_duplicates", tolerance="distance") + + +def test_remove_duplicates_leaves_no_repeated_archive_point(matched_nn): + """No archive point is used twice after deduplication.""" + out = remove_duplicates(md=matched_nn.copy(), archive=ARCHIVE_DATA) + archive_cols = [c for c in out.columns if c.startswith("archive_")] + + assert not out[archive_cols].duplicated().any() + + +def test_remove_duplicates_preserves_target_coverage(matched_nn): + """Re-matching never drops a target year.""" + out = remove_duplicates(md=matched_nn.copy(), archive=ARCHIVE_DATA) + + assert set(out["target_year"]) == set(matched_nn["target_year"]) + + +def test_remove_duplicates_rejects_multiple_matches_per_year(matched): + """Feeding non-singular matches raises `TypeError`. + + Documents a real precondition of the function: it operates on one recipe at a + time, not on the full many-to-many match table. + """ + with pytest.raises(TypeError): + remove_duplicates(md=matched.copy(), archive=ARCHIVE_DATA) + + +# --------------------------------------------------------------------------- +# permute_stitching_recipes +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("n_matches", [1, 2]) +def test_permute_stitching_recipes_invariant(golden, matched, n_matches): + """Seeded recipe permutations are unchanged.""" + out = permute_stitching_recipes( + N_matches=n_matches, matched_data=matched, archive=ARCHIVE_DATA, seed=1 + ) + golden.assert_frame(out, f"recipe/permute_N{n_matches}_seed1", tolerance="distance") + + +def test_permute_stitching_recipes_wide_tolerance_invariant(golden, matched_wide): + """Seeded permutations at a wider tolerance are unchanged. + + A wider tolerance means more candidates per window, exercising the sampling + and duplicate-rejection loop far more heavily than the narrow case. + """ + out = permute_stitching_recipes( + N_matches=2, matched_data=matched_wide, archive=ARCHIVE_DATA, seed=1 + ) + golden.assert_frame(out, "recipe/permute_N2_tol0.2_seed1", tolerance="distance") + + +def test_permute_covers_every_target_window(matched): + """A generated recipe spans exactly the target's time windows.""" + out = permute_stitching_recipes( + N_matches=1, matched_data=matched, archive=ARCHIVE_DATA, seed=1 + ) + + assert set(out["target_year"]) == set(TARGET_DATA["year"]) + + +def test_permute_draws_only_from_the_archive(matched): + """Every selected window really exists in the archive.""" + out = permute_stitching_recipes( + N_matches=2, matched_data=matched, archive=ARCHIVE_DATA, seed=1 + ) + + archive_cols = [c for c in out.columns if c.startswith("archive_")] + check = out[archive_cols].copy() + check.columns = [c.replace("archive_", "") for c in check.columns] + + assert len(check.merge(ARCHIVE_DATA)) == len(check), ( + "recipe contains a window that is not present in the archive" + ) + + +def test_permute_uses_each_archive_point_once(matched): + """No archive point is reused within a generated realization.""" + out = permute_stitching_recipes( + N_matches=2, matched_data=matched, archive=ARCHIVE_DATA, seed=1 + ) + + archive_cols = [c for c in out.columns if c.startswith("archive_")] + for _, group in out.groupby("stitching_id"): + assert not group[archive_cols].duplicated().any(), ( + "an archive point is reused within one stitching_id" + ) + + +def test_permute_has_no_envelope_collapse(matched_wide): + """Distinct target realizations do not share an archive point in the same year. + + "Envelope collapse" is the failure mode where independently generated + realizations converge onto identical archive data, understating the spread of + the emulated ensemble. Guarding it is a core correctness property. + """ + target2 = pd.concat([ARCHIVE_DATA, TARGET_DATA]).reset_index(drop=True).copy() + archive2 = target2.copy() + + out = permute_stitching_recipes( + N_matches=2, + matched_data=match_neighborhood(target2, archive2, tol=0.2), + archive=archive2, + seed=1, + ) + + archive_cols = [c for c in out.columns if c.startswith("archive_")] + for year in out["target_year"].unique(): + window = out[out["target_year"] == year] + assert len(window) == len(window[archive_cols].drop_duplicates()), ( + f"envelope collapse at target_year={year}" + ) + + +def test_permute_rejects_multiple_target_experiments(matched): + """Mixing target experiments in one call is rejected.""" + mixed = matched.copy() + mixed.loc[mixed.index[: len(mixed) // 2], "target_experiment"] = "ssp585" + + with pytest.raises(TypeError): + permute_stitching_recipes( + N_matches=1, matched_data=mixed, archive=ARCHIVE_DATA, seed=1 + ) + + +# --------------------------------------------------------------------------- +# transition handling +# --------------------------------------------------------------------------- + + +@pytest.fixture(scope="module") +def messy_recipe(matched): + """Return a seeded unformatted recipe for the transition handlers.""" + return permute_stitching_recipes( + N_matches=1, matched_data=matched, archive=ARCHIVE_DATA, seed=1 + ) + + +def test_handle_transition_periods_invariant(golden, messy_recipe): + """`handle_transition_periods` output is unchanged. + + This function splits windows that straddle the historical/future boundary, so + an error produces recipes that request data from the wrong experiment. + """ + out = handle_transition_periods(messy_recipe.copy()) + golden.assert_frame(out, "recipe/handle_transition_periods", tolerance="distance") + + +def test_handle_final_period_invariant(golden, messy_recipe): + """`handle_final_period` output is unchanged.""" + out = handle_final_period(handle_transition_periods(messy_recipe.copy())) + golden.assert_frame(out, "recipe/handle_final_period", tolerance="distance") + + +def test_transition_handling_preserves_target_coverage(messy_recipe): + """Transition handling never leaves a gap in the target timeline. + + The union of target windows must still cover the original span contiguously; + a gap would silently produce a stitched series with missing years. + """ + out = handle_final_period(handle_transition_periods(messy_recipe.copy())) + + for _, group in out.groupby("stitching_id"): + ordered = group.sort_values("target_start_yr") + starts = ordered["target_start_yr"].tolist() + ends = ordered["target_end_yr"].tolist() + + for previous_end, next_start in zip(ends, starts[1:]): + assert next_start == previous_end + 1, ( + f"gap or overlap in target coverage: {previous_end} -> {next_start}" + ) diff --git a/tests/regression/test_invariance_util.py b/tests/regression/test_invariance_util.py new file mode 100644 index 00000000..3971bd07 --- /dev/null +++ b/tests/regression/test_invariance_util.py @@ -0,0 +1,191 @@ +"""Golden-output regression tests for the utility and data helpers. + +Covers :mod:`stitches.fx_util` and :mod:`stitches.fx_data`. These functions are +small and cheap, but `global_mean` in particular carries real scientific weight: +it applies the cosine-of-latitude area weighting that converts a gridded field +into a global mean temperature. An error there would bias every emulated series +while still producing entirely plausible-looking numbers, so it is verified +against an analytically known result rather than only against a recorded one. +""" + +import numpy as np +import pandas as pd +import pytest +import xarray as xr + +from stitches.fx_data import get_lat_name, global_mean +from stitches.fx_util import anti_join, combine_df, nrow, selstr + +# Every test in this module is an output-invariance check; the CI regression job +# selects on this marker. +pytestmark = pytest.mark.regression + + +# --------------------------------------------------------------------------- +# fx_util +# --------------------------------------------------------------------------- + + +def test_selstr_extracts_substrings(): + """`selstr` slices a fixed character range out of a string.""" + assert selstr("abcdef", 0, 3) == "abc" + assert selstr("abcdef", 2, 4) == "cd" + assert selstr("abcdef", 0, 6) == "abcdef" + + +def test_nrow_counts_rows(): + """`nrow` reports the row count for frames and the length for arrays.""" + assert nrow(pd.DataFrame({"a": [1, 2, 3]})) == 3 + assert nrow(pd.DataFrame({"a": []})) == 0 + assert nrow(np.array([1, 2, 3, 4])) == 4 + + +def test_combine_df_invariant(golden): + """`combine_df` cross-join output is unchanged.""" + left = pd.DataFrame({"a": [1, 2], "b": ["x", "y"]}) + right = pd.DataFrame({"c": [10, 20, 30]}) + + out = combine_df(left, right) + golden.assert_frame(out, "util/combine_df", tolerance="exact") + + +def test_combine_df_produces_cartesian_product(): + """`combine_df` yields one row per input pair.""" + left = pd.DataFrame({"a": [1, 2], "b": ["x", "y"]}) + right = pd.DataFrame({"c": [10, 20, 30]}) + + out = combine_df(left, right) + + assert len(out) == len(left) * len(right) + + +def test_combine_df_rejects_shared_columns(): + """Overlapping column names are rejected rather than silently suffixed.""" + frame = pd.DataFrame({"a": [1, 2]}) + + with pytest.raises(Exception): + combine_df(frame, frame) + + +def test_anti_join_invariant(golden): + """`anti_join` output is unchanged.""" + left = pd.DataFrame({"k": [1, 2, 3, 4], "v": ["a", "b", "c", "d"]}) + right = pd.DataFrame({"k": [2, 4], "v": ["b", "d"]}) + + out = anti_join(left, right, bycols=["k"]) + golden.assert_frame(out, "util/anti_join", tolerance="exact") + + +def test_anti_join_keeps_only_unmatched_rows(): + """`anti_join` retains exactly the left rows with no match on the right.""" + left = pd.DataFrame({"k": [1, 2, 3, 4], "v": ["a", "b", "c", "d"]}) + right = pd.DataFrame({"k": [2, 4], "v": ["b", "d"]}) + + out = anti_join(left, right, bycols=["k"]) + + assert sorted(out["k"].tolist()) == [1, 3] + + +def test_anti_join_with_no_overlap_returns_left(): + """With no shared keys, every left row survives.""" + left = pd.DataFrame({"k": [1, 2], "v": ["a", "b"]}) + right = pd.DataFrame({"k": [98, 99], "v": ["y", "z"]}) + + out = anti_join(left, right, bycols=["k"]) + + assert len(out) == len(left) + + +# --------------------------------------------------------------------------- +# fx_data +# --------------------------------------------------------------------------- + + +def _grid(lat_name="lat", nlat=19, nlon=36, ntime=3): + """Build a small synthetic gridded dataset with a regular lat/lon grid. + + :param lat_name: Name to give the latitude coordinate, so the ``lat`` and + ``latitude`` spellings can both be exercised. + :return: A Dataset with a single ``tas`` variable. + :rtype: xarray.Dataset + """ + lats = np.linspace(-90, 90, nlat) + lons = np.linspace(0, 350, nlon) + times = pd.date_range("2000-01-01", periods=ntime, freq="MS") + + # A latitude-dependent field: warm at the equator, cold at the poles. This + # makes the area weighting matter -- an unweighted mean would over-count the + # poles, where grid cells are much smaller. + field = np.empty((ntime, nlat, nlon)) + for t in range(ntime): + field[t] = np.cos(np.deg2rad(lats))[:, None] * np.ones((1, nlon)) + t + + return xr.Dataset( + {"tas": ((("time", lat_name, "lon")), field)}, + coords={"time": times, lat_name: lats, "lon": lons}, + ) + + +@pytest.mark.parametrize("lat_name", ["lat", "latitude"]) +def test_get_lat_name_accepts_both_spellings(lat_name): + """`get_lat_name` finds the latitude coordinate under either name.""" + ds = _grid(lat_name=lat_name) + + assert get_lat_name(ds) == lat_name + + +def test_get_lat_name_raises_without_latitude(): + """A dataset with no recognizable latitude coordinate raises `RuntimeError`.""" + ds = xr.Dataset({"tas": (("time",), [1.0, 2.0])}, coords={"time": [0, 1]}) + + with pytest.raises(RuntimeError): + get_lat_name(ds) + + +def test_global_mean_invariant(golden): + """`global_mean` output is unchanged.""" + ds = _grid() + out = global_mean(ds) + + frame = out["tas"].to_dataframe().reset_index() + frame["time"] = frame["time"].astype(str) + golden.assert_frame(frame, "data/global_mean", tolerance="value") + + +def test_global_mean_reduces_to_time_only(): + """`global_mean` collapses every dimension except time.""" + ds = _grid() + out = global_mean(ds) + + assert set(out["tas"].dims) == {"time"} + + +def test_global_mean_of_constant_field_is_that_constant(): + """Averaging a spatially uniform field returns the constant. + + An analytic check on the weighting: whatever the weights are, if they are + correctly normalized then a constant field must average to itself. A bug in + the normalization shows up here immediately. + """ + ds = _grid() + ds["tas"] = xr.full_like(ds["tas"], 42.0) + + out = global_mean(ds) + + np.testing.assert_allclose(out["tas"].values, 42.0, rtol=1e-12) + + +def test_global_mean_weights_by_latitude(): + """The weighted mean of cos(latitude) exceeds the unweighted mean. + + With a cos(lat) field, area weighting emphasizes the wide equatorial cells + where the field is largest, so the weighted result must be strictly greater + than a naive arithmetic mean over grid cells. This confirms weighting is + actually applied rather than silently skipped. + """ + ds = _grid() + + weighted = float(global_mean(ds)["tas"].isel(time=0)) + unweighted = float(ds["tas"].isel(time=0).mean()) + + assert weighted > unweighted diff --git a/tests/test_install_data.py b/tests/test_install_data.py index 36ecacf5..6d3458fd 100644 --- a/tests/test_install_data.py +++ b/tests/test_install_data.py @@ -1,22 +1,307 @@ -import unittest +"""Tests for the Zenodo package-data installer. + +The download itself needs the network, but the parts most likely to break -- +URL resolution, directory creation, archive extraction, and error handling on a +bad response -- are exercised offline against a locally built zip file. +""" + +import io +import os +import zipfile + +import pytest +import requests import stitches import stitches.install_pkgdata as sd -class TestInstallRawData(unittest.TestCase): - """Tests for verifying the installation of raw data.""" +@pytest.fixture +def fake_archive(tmp_path): + """Build a zip file shaped like the real Zenodo data archive. + + Includes nested ``tas-data`` and ``temp-data`` members plus files that must be + filtered out, so extraction behavior can be checked in full. + """ + path = tmp_path / "data.zip" + + with zipfile.ZipFile(path, "w") as zipped: + zipped.writestr("data/matching_archive.csv", "a,b\n1,2\n") + zipped.writestr("data/pangeo_table.csv", "a,b\n3,4\n") + zipped.writestr("data/tas-data/CanESM5_tas.csv", "year,value\n1850,1.0\n") + zipped.writestr("data/tas-data/BCC-CSM2-MR_tas.csv", "year,value\n1850,2.0\n") + zipped.writestr("data/temp-data/scratch.csv", "x\n1\n") + zipped.writestr("data/example.nc", b"\x89HDF\r\n\x1a\n") + # Members that must be ignored. + zipped.writestr("data/README.md", "# not extracted\n") + zipped.writestr("data/notes.txt", "not extracted\n") + + return path + + +# --------------------------------------------------------------------------- +# URL resolution +# --------------------------------------------------------------------------- + + +def test_current_version_has_registered_data_url(): + """The installed version has an entry in the URL registry. + + Guards against tagging a release without registering its dataset, which would + silently downgrade users to the fallback archive. + """ + assert stitches.__version__ in sd.InstallPackageData.DATA_VERSION_URLS + + +def test_resolve_url_returns_registered_url(): + """A known version resolves to its registered URL.""" + installer = sd.InstallPackageData() + + resolved = installer.resolve_url("0.13") + + assert resolved == sd.InstallPackageData.DATA_VERSION_URLS["0.13"] + + +def test_resolve_url_warns_for_unknown_version(caplog): + """An unregistered version falls back but logs a warning. + + The original code printed a message built into a local variable that was + discarded on some paths, so the mismatch could pass unnoticed. + """ + installer = sd.InstallPackageData() + + with caplog.at_level("WARNING"): + resolved = installer.resolve_url("99.99.99-does-not-exist") + + assert resolved == sd.InstallPackageData.DEFAULT_VERSION + assert any("no data archive is registered" in r.message.lower() for r in caplog.records) + + +def test_default_version_is_a_string(): + """``DEFAULT_VERSION`` is a URL string.""" + assert isinstance(sd.InstallPackageData.DEFAULT_VERSION, str) + assert sd.InstallPackageData.DEFAULT_VERSION.startswith("https://") + + +# --------------------------------------------------------------------------- +# Target directory +# --------------------------------------------------------------------------- + + +def test_target_directory_honors_explicit_dir(tmp_path): + """An explicit ``data_dir`` is used verbatim.""" + installer = sd.InstallPackageData(data_dir=str(tmp_path)) + + assert installer.target_directory() == str(tmp_path) + + +def test_target_directory_defaults_into_package(): + """With no ``data_dir``, the package's own data directory is used.""" + installer = sd.InstallPackageData() + + assert installer.target_directory().endswith(os.path.join("stitches", "data")) + + +# --------------------------------------------------------------------------- +# Extraction +# --------------------------------------------------------------------------- + + +def test_extract_keeps_only_csv_and_nc(tmp_path, fake_archive): + """Only ``.csv`` and ``.nc`` members are extracted.""" + target = tmp_path / "out" + + installer = sd.InstallPackageData(data_dir=str(target)) + written = installer.extract(str(fake_archive), str(target)) + + extensions = {os.path.splitext(f)[-1] for f in written} + assert extensions <= {".csv", ".nc"} + assert not (target / "README.md").exists() + assert not (target / "notes.txt").exists() + + +def test_extract_preserves_tas_data_subdirectory(tmp_path, fake_archive): + """``tas-data`` members land in the ``tas-data`` subdirectory.""" + target = tmp_path / "out" + + installer = sd.InstallPackageData(data_dir=str(target)) + installer.extract(str(fake_archive), str(target)) + + assert (target / "tas-data" / "CanESM5_tas.csv").is_file() + assert (target / "tas-data" / "BCC-CSM2-MR_tas.csv").is_file() + + +def test_extract_preserves_temp_data_subdirectory(tmp_path, fake_archive): + """``temp-data`` members land in the ``temp-data`` subdirectory. + + Regression test: the original code created ``temp-data`` but only re-nested + members whose path contained ``tas-data``, so ``temp-data`` files were + flattened into the top level and the directory stayed empty. + """ + target = tmp_path / "out" + + installer = sd.InstallPackageData(data_dir=str(target)) + installer.extract(str(fake_archive), str(target)) + + assert (target / "temp-data" / "scratch.csv").is_file() + assert not (target / "scratch.csv").exists() + + +def test_extract_flattens_top_level_members(tmp_path, fake_archive): + """Members outside a known subdirectory are written to the top level.""" + target = tmp_path / "out" + + installer = sd.InstallPackageData(data_dir=str(target)) + installer.extract(str(fake_archive), str(target)) + + assert (target / "matching_archive.csv").is_file() + assert (target / "pangeo_table.csv").is_file() + assert (target / "example.nc").is_file() + + +def test_extract_content_is_intact(tmp_path, fake_archive): + """Extracted file contents match what was archived.""" + target = tmp_path / "out" + + installer = sd.InstallPackageData(data_dir=str(target)) + installer.extract(str(fake_archive), str(target)) + + assert (target / "matching_archive.csv").read_text() == "a,b\n1,2\n" + + +# --------------------------------------------------------------------------- +# Download error handling +# --------------------------------------------------------------------------- + + +class _FakeResponse: + """Minimal stand-in for a streaming ``requests`` response.""" + + def __init__(self, payload=b"", status=200, headers=None): + self._payload = payload + self.status_code = status + self.headers = headers or {"content-length": str(len(payload))} + + def __enter__(self): + return self + + def __exit__(self, *exc_info): + return False + + def raise_for_status(self): + if self.status_code >= 400: + raise requests.HTTPError(f"{self.status_code} error") + + def iter_content(self, chunk_size=1): + stream = io.BytesIO(self._payload) + while True: + chunk = stream.read(chunk_size) + if not chunk: + return + yield chunk + + +def test_download_raises_on_http_error(tmp_path, monkeypatch): + """An error status raises instead of writing an error page to disk. + + The original code never called ``raise_for_status``, so a 404 HTML body was + written out and only failed later with a confusing "not a zip file" error. + """ + monkeypatch.setattr( + sd.requests, "get", lambda *a, **k: _FakeResponse(b"404", status=404) + ) + + installer = sd.InstallPackageData(data_dir=str(tmp_path)) + + with pytest.raises(requests.HTTPError): + installer.download("https://example.invalid/data.zip", str(tmp_path / "d.zip")) + + +def test_download_streams_payload_to_disk(tmp_path, monkeypatch): + """The response body is written to the destination path.""" + payload = b"x" * 4096 + monkeypatch.setattr(sd.requests, "get", lambda *a, **k: _FakeResponse(payload)) + + destination = tmp_path / "d.zip" + installer = sd.InstallPackageData(data_dir=str(tmp_path)) + installer.download("https://example.invalid/data.zip", str(destination)) + + assert destination.read_bytes() == payload + + +def test_download_passes_timeout(tmp_path, monkeypatch): + """A timeout is supplied, so a hung server cannot block forever. + + The original call had no timeout at all. + """ + seen = {} + + def _capture(url, **kwargs): + seen.update(kwargs) + return _FakeResponse(b"data") + + monkeypatch.setattr(sd.requests, "get", _capture) + + installer = sd.InstallPackageData(data_dir=str(tmp_path)) + installer.download("https://example.invalid/data.zip", str(tmp_path / "d.zip")) + + assert seen.get("timeout") is not None + assert seen.get("stream") is True + + +def test_fetch_zenodo_reports_bad_archive(tmp_path, monkeypatch): + """A non-zip download produces an actionable error message.""" + monkeypatch.setattr( + sd.requests, "get", lambda *a, **k: _FakeResponse(b"not a zip file at all") + ) + + installer = sd.InstallPackageData(data_dir=str(tmp_path)) + + with pytest.raises(RuntimeError, match="not a valid zip archive"): + installer.fetch_zenodo() + + +def test_fetch_zenodo_creates_subdirectories(tmp_path, monkeypatch, fake_archive): + """Required subdirectories are created, including missing parents. + + The original code used ``os.mkdir``, which raises if a parent is absent. + """ + payload = fake_archive.read_bytes() + monkeypatch.setattr(sd.requests, "get", lambda *a, **k: _FakeResponse(payload)) + + target = tmp_path / "missing" / "parents" / "data" + installer = sd.InstallPackageData(data_dir=str(target)) + + installer.fetch_zenodo() + + assert (target / "tas-data").is_dir() + assert (target / "temp-data").is_dir() + assert (target / "tas-data" / "CanESM5_tas.csv").is_file() + + +def test_fetch_zenodo_is_idempotent(tmp_path, monkeypatch, fake_archive): + """Running twice succeeds; existing directories are not an error.""" + payload = fake_archive.read_bytes() + monkeypatch.setattr(sd.requests, "get", lambda *a, **k: _FakeResponse(payload)) + + target = tmp_path / "data" + installer = sd.InstallPackageData(data_dir=str(target)) + + installer.fetch_zenodo() + written = installer.fetch_zenodo() + + assert len(written) > 0 - def test_instantiate(self): - """Test instantiation of InstallPackageData with a fake data directory.""" - zen = sd.InstallPackageData(data_dir="fake") - # Ensure default version is set - self.assertEqual(str, type(zen.DEFAULT_VERSION)) +def test_install_package_data_returns_written_files(tmp_path, monkeypatch, fake_archive): + """The module-level helper returns the list of files written. - # Ensure URLs are present for current version - self.assertTrue(stitches.__version__ in zen.DATA_VERSION_URLS) + Previously it returned ``None``, giving callers no way to confirm what landed. + """ + payload = fake_archive.read_bytes() + monkeypatch.setattr(sd.requests, "get", lambda *a, **k: _FakeResponse(payload)) + written = sd.install_package_data(data_dir=str(tmp_path / "data")) -if __name__ == "__main__": - unittest.main() + assert isinstance(written, list) + assert len(written) == 6 diff --git a/tests/test_make_tas_archive.py b/tests/test_make_tas_archive.py new file mode 100644 index 00000000..b15faabd --- /dev/null +++ b/tests/test_make_tas_archive.py @@ -0,0 +1,217 @@ +"""Tests for the tas archive construction helpers. + +Focused on `write_tas_data_by_model`, which was extracted from `make_tas_archive` +specifically so its filename construction could be tested. The full +`make_tas_archive` run downloads the entire CMIP6 tas archive from Pangeo and +takes hours, so the bug these tests cover was previously unreachable in CI -- +which is why it shipped. +""" + +import os + +import pandas as pd +import pytest + +from stitches.make_tas_archive import ( + calculate_anomaly, + join_exclude, + rbind, + write_tas_data_by_model, +) + + +@pytest.fixture +def tas_frame(): + """Return a small multi-model global tas frame.""" + rows = [] + for model in ("BCC-CSM2-MR", "CanESM5", "UKESM1-0-LL"): + for year in range(2000, 2005): + rows.append( + { + "model": model, + "experiment": "historical", + "ensemble": "r1i1p1f1", + "variable": "tas", + "year": year, + "value": 287.0 + year / 1000, + "unit": "K", + } + ) + return pd.DataFrame(rows) + + +# --------------------------------------------------------------------------- +# write_tas_data_by_model +# --------------------------------------------------------------------------- + + +def test_write_tas_data_by_model_filenames(tmp_path, tas_frame): + """Filenames are ``_tas.csv`` with no tuple punctuation. + + Regression test for two defects present in the original implementation: + + * ``groupby(["model"])`` with a single-element *list* yields a one-element + tuple as the group name under pandas 2.0+, which would have produced + ``('BCC-CSM2-MR',)_tas.csv``. + * the path was built with ``Traversable + str``, which raises ``TypeError``. + """ + files = write_tas_data_by_model(tas_frame, tmp_path) + + names = sorted(os.path.basename(f) for f in files) + assert names == [ + "BCC-CSM2-MR_tas.csv", + "CanESM5_tas.csv", + "UKESM1-0-LL_tas.csv", + ] + + for name in names: + assert "(" not in name and ")" not in name and "," not in name + + +def test_write_tas_data_by_model_accepts_path_like(tas_frame, tmp_path): + """A path-like output directory is accepted, not just a ``str``. + + ``make_tas_archive`` passes ``resources.files("stitches") / "data" / + "tas-data"``, which is a ``Traversable`` (a ``PosixPath`` in practice). The + original code concatenated it with ``+``, which raises ``TypeError`` for path + objects. Passing a ``Path`` here reproduces that call shape. + """ + files = write_tas_data_by_model(tas_frame, tmp_path) + + assert len(files) == 3 + assert all(isinstance(f, str) for f in files), "paths should be returned as str" + + +def test_write_tas_data_by_model_matches_production_directory_type(tas_frame, tmp_path): + """The type produced by ``importlib.resources`` is handled. + + Guards the specific failure mode directly: the object type that + ``make_tas_archive`` supplies must not raise when joined. + """ + from importlib import resources + + production_type = type(resources.files("stitches") / "data" / "tas-data") + + # Confirm the assumption behind this test still holds, then exercise the + # helper with an instance of that same type pointing at a disposable dir. + assert issubclass(production_type, os.PathLike) + + files = write_tas_data_by_model(tas_frame, production_type(tmp_path)) + + assert len(files) == 3 + + +def test_write_tas_data_by_model_files_exist_and_roundtrip(tmp_path, tas_frame): + """Each written file is readable and holds only its own model's rows.""" + files = write_tas_data_by_model(tas_frame, tmp_path) + + total = 0 + for path in files: + assert os.path.isfile(path), f"{path} was not written" + + frame = pd.read_csv(path) + assert frame["model"].nunique() == 1 + + expected_model = os.path.basename(path).replace("_tas.csv", "") + assert frame["model"].unique()[0] == expected_model + + total += len(frame) + + assert total == len(tas_frame), "rows were lost or duplicated across files" + + +def test_write_tas_data_by_model_creates_missing_directory(tmp_path, tas_frame): + """A nonexistent output directory is created rather than raising. + + The original code called ``os.mkdir``, which fails if an intermediate parent + is missing; ``os.makedirs(exist_ok=True)`` is used instead. + """ + target = tmp_path / "deeply" / "nested" / "tas-data" + + files = write_tas_data_by_model(tas_frame, target) + + assert target.is_dir() + assert len(files) == 3 + + +def test_write_tas_data_by_model_is_idempotent(tmp_path, tas_frame): + """Writing twice into the same directory succeeds and does not duplicate rows.""" + write_tas_data_by_model(tas_frame, tmp_path) + files = write_tas_data_by_model(tas_frame, tmp_path) + + for path in files: + frame = pd.read_csv(path) + assert len(frame) == 5 + + +def test_write_tas_data_by_model_requires_model_column(tmp_path): + """A frame without a ``model`` column is rejected.""" + with pytest.raises(Exception): + write_tas_data_by_model(pd.DataFrame({"value": [1, 2]}), tmp_path) + + +def test_write_tas_data_by_model_handles_model_names_with_dots(tmp_path): + """Model identifiers containing dots and dashes survive intact. + + Real CMIP6 source IDs include names such as ``EC-Earth3-Veg-LR`` and + ``FGOALS-f3-L``; the filename must not be truncated at a separator. + """ + frame = pd.DataFrame( + {"model": ["EC-Earth3-Veg-LR", "FGOALS-f3-L"], "value": [1.0, 2.0]} + ) + + files = write_tas_data_by_model(frame, tmp_path) + names = sorted(os.path.basename(f) for f in files) + + assert names == ["EC-Earth3-Veg-LR_tas.csv", "FGOALS-f3-L_tas.csv"] + + +# --------------------------------------------------------------------------- +# Supporting helpers +# --------------------------------------------------------------------------- + + +def test_rbind_with_empty_frame(): + """`rbind` combines frames even when one side is empty.""" + populated = pd.DataFrame({"a": [1, 2]}) + empty = pd.DataFrame({"a": []}) + + assert len(rbind(populated, empty)) == 2 + assert len(rbind(empty, populated)) == 2 + + +def test_join_exclude_drops_matching_rows(): + """`join_exclude` removes the rows described by the drop frame.""" + data = pd.DataFrame({"model": ["a", "b", "c"], "value": [1, 2, 3]}) + drop = pd.DataFrame({"model": ["b"]}) + + out = join_exclude(data, drop) + + assert sorted(out["model"]) == ["a", "c"] + + +def test_calculate_anomaly_centers_reference_period(): + """`calculate_anomaly` subtracts the mean of the reference window. + + Constructed so the reference-period mean is exactly known: with a constant + value across the reference years, every anomaly in that window must be zero. + """ + rows = [] + for year in range(1995, 2021): + rows.append( + { + "model": "test_model", + "experiment": "historical" if year <= 2014 else "ssp245", + "ensemble": "r1i1p1f1", + "variable": "tas", + "year": year, + "value": 287.0, + "unit": "K", + } + ) + data = pd.DataFrame(rows) + + out = calculate_anomaly(data, startYr=1995, endYr=2014) + + reference = out[(out["year"] >= 1995) & (out["year"] <= 2014)] + assert reference["value"].abs().max() == pytest.approx(0.0, abs=1e-12) diff --git a/tests/test_pangeo.py b/tests/test_pangeo.py index a20f7932..6d936882 100644 --- a/tests/test_pangeo.py +++ b/tests/test_pangeo.py @@ -1,45 +1,43 @@ -import unittest +"""Tests for the Pangeo CMIP6 catalog interface. + +These tests reach out to the live Pangeo catalog on Google Cloud Storage and are +therefore marked ``network``. Previously they were gated behind a ``RUN = "ci"`` +class attribute whose false branch asserted ``0 == 0``, meaning they reported as +*passing* in CI while exercising nothing. They now skip visibly instead; run +them with ``pytest --network``. +""" import pandas as pd +import pytest import xarray as xr from stitches.fx_pangeo import fetch_nc, fetch_pangeo_table -class TestPangeo(unittest.TestCase): - """ - A test case for Pangeo-related functions. - - This test class is used to run tests for functions that interact with the Pangeo data archive. - It includes a flag to run tests for continuous integration or for all cases. - """ - - RUN = "ci" - - def test_pangeo_fn(self): - """Test Pangeo-related functions for continuous integration or full cases.""" - if TestPangeo.RUN == "ci": - self.assertEqual(0, 0) - else: - # Get the table of all pangeo contents as a data frame - ptable = fetch_pangeo_table() - self.assertEqual( - type(ptable), - pd.core.frame.DataFrame, - "fetch_pangeo_table did not return a data frame", - ) - - # Select a single files from the pangeo cataog to import - import_this = ptable.loc[ - (ptable["table_id"] == "Amon") - & (ptable["activity_id"] == "ScenarioMIP") - ].reset_index(drop=True) - path = import_this["zstore"][0] - out = fetch_nc(path) - self.assertEqual( - type(out), xr.core.dataset.Dataset, "problem with fetch_nc" - ) - - -if __name__ == "__main__": - unittest.main() +@pytest.mark.network +def test_fetch_pangeo_table_returns_dataframe(): + """`fetch_pangeo_table` returns a non-empty DataFrame with expected columns.""" + ptable = fetch_pangeo_table() + + assert isinstance(ptable, pd.DataFrame), "fetch_pangeo_table did not return a DataFrame" + assert len(ptable) > 0, "Pangeo table is empty" + + for column in ("zstore", "table_id", "activity_id", "source_id", "variable_id"): + assert column in ptable.columns, f"Pangeo table missing column {column!r}" + + +@pytest.mark.network +@pytest.mark.slow +def test_fetch_nc_returns_dataset(): + """`fetch_nc` opens a single zarr store from the catalog as an xarray Dataset.""" + ptable = fetch_pangeo_table() + + import_this = ptable.loc[ + (ptable["table_id"] == "Amon") & (ptable["activity_id"] == "ScenarioMIP") + ].reset_index(drop=True) + assert len(import_this) > 0, "no Amon/ScenarioMIP entries found in the Pangeo table" + + out = fetch_nc(import_this["zstore"][0]) + + assert isinstance(out, xr.Dataset), "problem with fetch_nc" + assert "time" in out.dims diff --git a/tests/test_seed.py b/tests/test_seed.py new file mode 100644 index 00000000..e075c952 --- /dev/null +++ b/tests/test_seed.py @@ -0,0 +1,140 @@ +"""Tests for the ``seed`` arguments on the randomized entry points. + +`shuffle_function` and `permute_stitching_recipes` both draw random samples. +Before the ``seed`` parameter existed, the only way to obtain a reproducible +recipe was ``permute_stitching_recipes(testing=True)``, which hardcoded +``random_state=1``. Overloading a "testing" flag to mean "be deterministic" is +awkward for users who legitimately need reproducible output, so an explicit +``seed`` was added. + +These tests pin the backward-compatibility contract: + +* omitting ``seed`` reproduces the historical nondeterministic behavior; +* ``testing=True`` remains exactly equivalent to ``seed=1``; +* distinct seeds produce distinct draws, i.e. the seed is actually plumbed + through rather than silently ignored. +""" + +import pandas as pd +import pytest + +from stitches.fx_match import match_neighborhood, shuffle_function +from stitches.fx_recipe import permute_stitching_recipes + +from test_fx_recipe import TestRecipe + +TARGET_DATA = TestRecipe.TARGET_DATA +ARCHIVE_DATA = TestRecipe.ARCHIVE_DATA + + +@pytest.fixture(scope="module") +def matched(): + """Return matched data shared by the permutation tests.""" + return match_neighborhood(TARGET_DATA, ARCHIVE_DATA, tol=0.07) + + +def _permute(matched_data, **kwargs): + """Call `permute_stitching_recipes` with the shared test arguments.""" + return permute_stitching_recipes( + N_matches=2, matched_data=matched_data, archive=ARCHIVE_DATA, **kwargs + ) + + +# --------------------------------------------------------------------------- +# shuffle_function +# --------------------------------------------------------------------------- + + +def test_shuffle_function_preserves_shape(): + """Shuffling reorders rows without adding or dropping any.""" + subset = TARGET_DATA.head(10) + out = shuffle_function(subset) + + assert subset.shape == out.shape + assert sorted(out["year"].tolist()) == sorted(subset["year"].tolist()) + + +def test_shuffle_function_seed_is_reproducible(): + """The same seed yields the same permutation.""" + first = shuffle_function(TARGET_DATA, seed=42) + second = shuffle_function(TARGET_DATA, seed=42) + + pd.testing.assert_frame_equal(first, second) + + +def test_shuffle_function_distinct_seeds_differ(): + """Different seeds yield different permutations.""" + first = shuffle_function(TARGET_DATA, seed=42) + second = shuffle_function(TARGET_DATA, seed=43) + + assert not first.equals(second) + + +def test_shuffle_function_without_seed_is_random(): + """Omitting the seed preserves the historical nondeterministic behavior. + + Repeated draws are compared rather than a single pair to keep the odds of a + coincidental match negligible for a 28-row frame. + """ + draws = [shuffle_function(TARGET_DATA) for _ in range(5)] + + assert any(not draws[0].equals(other) for other in draws[1:]) + + +# --------------------------------------------------------------------------- +# permute_stitching_recipes +# --------------------------------------------------------------------------- + + +def test_permute_seed_matches_legacy_testing_flag(matched): + """``seed=1`` must reproduce ``testing=True`` exactly. + + This is the backward-compatibility guarantee: published results generated + with ``testing=True`` remain reproducible via the new parameter, so the seed + refactor cannot have altered any existing output. + """ + legacy = _permute(matched, testing=True) + seeded = _permute(matched, seed=1) + + pd.testing.assert_frame_equal(legacy, seeded) + + +def test_permute_testing_flag_is_still_reproducible(matched): + """``testing=True`` remains deterministic across calls.""" + first = _permute(matched, testing=True) + second = _permute(matched, testing=True) + + pd.testing.assert_frame_equal(first, second) + + +def test_permute_seed_is_reproducible(matched): + """A given seed yields the same recipe across calls.""" + first = _permute(matched, seed=1234) + second = _permute(matched, seed=1234) + + pd.testing.assert_frame_equal(first, second) + + +def test_permute_distinct_seeds_differ(matched): + """Different seeds select different archive points, proving the seed is used.""" + first = _permute(matched, seed=1) + second = _permute(matched, seed=99) + + assert not first.equals(second) + + +def test_permute_without_seed_is_random(matched): + """Omitting both ``seed`` and ``testing`` preserves nondeterminism.""" + draws = [_permute(matched) for _ in range(5)] + + assert any(not draws[0].equals(other) for other in draws[1:]) + + +def test_permute_seed_overrides_testing_flag(matched): + """An explicit ``seed`` takes precedence over ``testing=True``.""" + with_seed = _permute(matched, testing=True, seed=99) + seed_only = _permute(matched, seed=99) + legacy = _permute(matched, testing=True) + + pd.testing.assert_frame_equal(with_seed, seed_only) + assert not with_seed.equals(legacy) diff --git a/tests/test_stitch.py b/tests/test_stitch.py index 2f38bb77..29d010a2 100644 --- a/tests/test_stitch.py +++ b/tests/test_stitch.py @@ -1,8 +1,22 @@ -import os -import unittest +"""Tests for the stitching functions. + +Capability notes: + +* ``find_var_cols`` / ``find_zfiles`` are pure DataFrame helpers and run offline. +* ``gmat_stitching`` reads the per-model ``tas-data`` CSVs from the installed + package data, so it is marked ``package_data``. +* ``gridded_stitching`` and ``internal_stitch`` pull zarr stores from Pangeo and + write NetCDF, so they are marked ``network`` and ``slow``. + +The previous version of this module gated the gridded tests behind a +``RUN = "ci"`` class attribute whose false branch asserted ``0 == 0``. That made +them report as passing in CI while exercising nothing. They now skip visibly; +run them with ``pytest --network --slow``. +""" import numpy as np import pandas as pd +import pytest import xarray as xr from stitches.fx_pangeo import fetch_nc @@ -15,172 +29,191 @@ ) from stitches.fx_util import nrow +# An example recipe used across the stitching tests. +MY_RP = pd.DataFrame( + data={ + "target_start_yr": [1850, 1859], + "target_end_yr": [1858, 1867], + "archive_experiment": ["historical", "historical"], + "archive_variable": ["tas", "tas"], + "archive_model": ["BCC-CSM2-MR", "BCC-CSM2-MR"], + "archive_ensemble": ["r1i1p1f1", "r1i1p1f1"], + "stitching_id": ["ssp245~r1i1p1f1~1", "ssp245~r1i1p1f1~1"], + "archive_start_yr": [1859, 1886], + "archive_end_yr": [1867, 1894], + "tas_file": [ + "gs://cmip6/CMIP6/CMIP/BCC/BCC-CSM2-MR/historical/r1i1p1f1/Amon/tas/gn/v20181126/", + "gs://cmip6/CMIP6/CMIP/BCC/BCC-CSM2-MR/historical/r1i1p1f1/Amon/tas/gn/v20181126/", + ], + } +) + + +@pytest.fixture +def recipe(): + """Return a fresh copy of the example recipe so tests cannot mutate shared state.""" + return MY_RP.copy() + + +# --------------------------------------------------------------------------- +# Offline helpers +# --------------------------------------------------------------------------- + + +def test_find_var_cols(): + """`find_var_cols` identifies columns whose names end in ``_file``.""" + o = pd.DataFrame(data={"tas": [1, 2], "col2": [3, 4]}) + assert len(find_var_cols(o)) == 0 + + o = pd.DataFrame(data={"tas_file": [1, 2], "col2": [3, 4]}) + assert find_var_cols(o) == ["tas"] + + o = pd.DataFrame(data={"tas_file": [1, 2], "col2": [3, 4], "fake_file": [1, 2]}) + assert len(find_var_cols(o)) == 2 + + +def test_find_zfiles_returns_empty_without_file_columns(): + """`find_zfiles` returns nothing when the frame has no ``_file`` columns.""" + d = pd.DataFrame(data={"tas": [1, 2], "col2": [3, 4], "year": [1, 2]}) + assert len(find_zfiles(d)) == 0 -class TestStitch(unittest.TestCase): - """ - Unit tests for stitching functions in the `stitches` package. - - This class provides a set of tests to ensure the correct functionality - of the stitching functions, which are used to combine different climate - model outputs into a single coherent dataset. - """ - - RUN = "ci" - - # This is an example recipe that will be used to test the stitching functions - MY_RP = pd.DataFrame( - data={ - "target_start_yr": [1850, 1859], - "target_end_yr": [1858, 1867], - "archive_experiment": ["historical", "historical"], - "archive_variable": ["tas", "tas"], - "archive_model": ["BCC-CSM2-MR", "BCC-CSM2-MR"], - "archive_ensemble": ["r1i1p1f1", "r1i1p1f1"], - "stitching_id": ["ssp245~r1i1p1f1~1", "ssp245~r1i1p1f1~1"], - "archive_start_yr": [1859, 1886], - "archive_end_yr": [1867, 1894], - "tas_file": [ - "gs://cmip6/CMIP6/CMIP/BCC/BCC-CSM2-MR/historical/r1i1p1f1/Amon/tas/gn/v20181126/", - "gs://cmip6/CMIP6/CMIP/BCC/BCC-CSM2-MR/historical/r1i1p1f1/Amon/tas/gn/v20181126/", - ], - } + +def test_find_zfiles_collects_paths(): + """`find_zfiles` returns an ndarray of the referenced file paths.""" + d = pd.DataFrame( + data={"tas_file": ["file1.csv", "file2.csv"], "col2": [3, 4], "year": [1, 2]} ) + file_list = find_zfiles(d) + + assert isinstance(file_list, np.ndarray) + assert len(file_list) == 2 + + +def test_find_zfiles_deduplicates(): + """`find_zfiles` collapses repeated paths so each store is fetched once.""" + d = pd.DataFrame( + data={"tas_file": ["file1.csv", "file1.csv"], "col2": [3, 4], "year": [1, 2]} + ) + file_list = find_zfiles(d) + + assert len(file_list) == len(np.unique(file_list)) + assert len(file_list) != nrow(d) + + +# --------------------------------------------------------------------------- +# Global-mean stitching (needs the installed tas-data archive) +# --------------------------------------------------------------------------- + + +@pytest.mark.package_data +def test_gmat_stitching_shape(package_data, recipe): + """`gmat_stitching` returns one row per target year covered by the recipe.""" + out = gmat_stitching(recipe) + + assert isinstance(out, pd.DataFrame) + + time_steps = max(recipe["target_end_yr"]) - min(recipe["target_start_yr"]) + 1 + assert nrow(out) == time_steps + + +@pytest.mark.package_data +def test_gmat_stitching_is_row_order_invariant(package_data, recipe): + """Reversing the recipe row order must not change the stitched result.""" + out = gmat_stitching(recipe) + + reverse = recipe.iloc[::-1] + out2 = gmat_stitching(reverse) + + assert out.shape == out2.shape + pd.testing.assert_frame_equal( + out.sort_values("year").reset_index(drop=True), + out2.sort_values("year").reset_index(drop=True), + ) + + +@pytest.mark.package_data +def test_gmat_stitching_ignores_tas_file_column(package_data, recipe): + """The ``tas_file`` values are unused by `gmat_stitching`, which reads local data.""" + recipe["tas_file"] = ["fake.nc", "fake.nc"] + out = gmat_stitching(recipe) + + assert isinstance(out, pd.DataFrame) + + +@pytest.mark.package_data +@pytest.mark.parametrize("missing", ["tas_file", "target_start_yr"]) +def test_gmat_stitching_requires_columns(package_data, recipe, missing): + """Dropping a required recipe column raises `KeyError`.""" + with pytest.raises(KeyError): + gmat_stitching(recipe.drop(columns=missing)) + + +@pytest.mark.package_data +def test_gmat_stitching_rejects_unknown_model(package_data, recipe): + """A model absent from the archive raises `IndexError`.""" + recipe["archive_model"] = ["fake", "fake"] + with pytest.raises(IndexError): + gmat_stitching(recipe) + + +# --------------------------------------------------------------------------- +# Gridded stitching (needs Pangeo) +# --------------------------------------------------------------------------- + + +def _expected_monthly_steps(rp): + """Return the number of monthly time steps a recipe should produce.""" + return 12 * (max(rp["target_end_yr"]) - min(rp["target_start_yr"])) + 12 + + +@pytest.mark.network +@pytest.mark.slow +def test_internal_stitch_time_length(recipe): + """`internal_stitch` produces the expected number of monthly time steps.""" + file_list = find_zfiles(recipe) + data_list = list(map(fetch_nc, file_list)) + rslt = internal_stitch(recipe, data_list, file_list) + + assert len(rslt["tas"]["time"]) == _expected_monthly_steps(recipe) + + +@pytest.mark.network +@pytest.mark.slow +def test_gridded_stitching_writes_dataset(tmp_path, recipe): + """`gridded_stitching` writes a NetCDF file with the expected time axis.""" + out = gridded_stitching(str(tmp_path), recipe) + + with xr.open_dataset(out[0]) as data: + assert isinstance(data, xr.Dataset) + assert len(data["time"]) == _expected_monthly_steps(recipe) + + +@pytest.mark.network +@pytest.mark.slow +def test_gridded_stitching_is_row_order_invariant(tmp_path, recipe): + """Reversing the recipe row order must not change the stitched time axis.""" + forward = gridded_stitching(str(tmp_path / "fwd"), recipe) + with xr.open_dataset(forward[0]) as data: + time1 = data["tas"]["time"].values + + reverse = gridded_stitching(str(tmp_path / "rev"), recipe.iloc[::-1]) + with xr.open_dataset(reverse[0]) as data2: + time2 = data2["tas"]["time"].values + + assert max(time1 - time2) == 0 + + +@pytest.mark.network +@pytest.mark.slow +def test_gridded_stitching_rejects_bad_output_dir(recipe): + """A nonexistent output directory raises `TypeError`.""" + with pytest.raises(TypeError): + gridded_stitching("fake", recipe) + - def test_find_var_cols(self): - """Test the `find_var_cols` function for identifying variable columns.""" - o = pd.DataFrame(data={"tas": [1, 2], "col2": [3, 4]}) - self.assertEqual(len(find_var_cols(o)), 0) - - o = pd.DataFrame(data={"tas_file": [1, 2], "col2": [3, 4]}) - self.assertEqual(find_var_cols(o), ["tas"]) - - o = pd.DataFrame(data={"tas_file": [1, 2], "col2": [3, 4], "fake_file": [1, 2]}) - self.assertEqual(len(find_var_cols(o)), 2) - - def test_find_zfiles(self): - """ - Test the `find_zfiles` function to ensure it correctly identifies zipped file paths. - - This test verifies that the `find_zfiles` function correctly identifies file paths - for zipped files within a given DataFrame. - """ - d = pd.DataFrame(data={"tas": [1, 2], "col2": [3, 4], "year": [1, 2]}) - self.assertEqual(len(find_zfiles(d)), 0) - - d = pd.DataFrame( - data={ - "tas_file": ["file1.csv", "file2.csv"], - "col2": [3, 4], - "year": [1, 2], - } - ) - file_list = find_zfiles(d) - self.assertEqual(type(file_list), np.ndarray) - self.assertEqual(len(file_list), 2) - - d = pd.DataFrame( - data={ - "tas_file": ["file1.csv", "file1.csv"], - "col2": [3, 4], - "year": [1, 2], - } - ) - file_list = find_zfiles(d) - self.assertEqual(len(file_list), len(np.unique(file_list))) - self.assertTrue(len(file_list) != nrow(d)) - - def test_gmat_stitching(self): - """ - Test the output returned by `gmat_stitching`. - - This test checks the type and structure of the output to ensure it meets expected formats. - """ - - out = gmat_stitching(self.MY_RP) - - self.assertEqual(type(out), pd.core.frame.DataFrame) - - time_steps = ( - max(self.MY_RP["target_end_yr"]) - min(self.MY_RP["target_start_yr"]) + 1 - ) - self.assertEqual(nrow(out), time_steps) - - # If the recipe is read in backwards, it shouldn't matter. The output should be the same. - reverse = self.MY_RP.copy() - reverse = reverse.iloc[::-1] - out2 = gmat_stitching(reverse) - self.assertEqual(out.shape, out2.shape) - self.assertEqual(out["year"][0], out2["year"][0]) - self.assertEqual(out["value"][6], out["value"][6]) - - # Manipulate the recipe, sometimes it will be fine other times it will throw an error. - rp = self.MY_RP.copy() - - rp["tas_file"] = ["fake.nc", "fake.nc"] - out = gmat_stitching(rp) - self.assertEqual(type(out), pd.core.frame.DataFrame) - - # If the recpie is missing a column the stitching function should fail. - with self.assertRaises(KeyError): - gmat_stitching(rp.drop("tas_file")) - - with self.assertRaises(KeyError): - gmat_stitching(rp.drop("target_start_yr")) - - with self.assertRaises(IndexError): - rp["archive_model"] = ["fake", "fake"] - gmat_stitching(rp) - - def test_gridded_related(self): - """ - Test functions related to gridded data stitching. - - This test suite covers the functionality of gridded data stitching, - ensuring that the output is consistent and errors are raised when - expected. - """ - if TestStitch.RUN == "ci": - self.assertEqual(0, 0) - else: - # Set up the elements required for internal_stitch - file_list = find_zfiles(self.MY_RP) - data_list = list(map(fetch_nc, file_list)) - rslt = internal_stitch(self.MY_RP, data_list, file_list) - - time_steps = ( - 12 - * ( - max(self.MY_RP["target_end_yr"]) - - min(self.MY_RP["target_start_yr"]) - ) - + 12 - ) - self.assertEqual(len(rslt["tas"]["time"]), time_steps) - - # Now do the stitching - out = gridded_stitching(".", self.MY_RP) - data = xr.open_dataset(out[0]) - time1 = data["tas"]["time"].values - self.assertEqual(type(data), xr.core.dataset.Dataset) - self.assertEqual(len(data["time"]), time_steps) - os.remove(out[0]) - - # If the recipe is read in backwards, it shouldn't matter the output should be the same. - reverse = self.MY_RP.copy() - reverse = reverse.iloc[::-1] - out = gridded_stitching(".", reverse) - data2 = xr.open_dataset(out[0]) - time2 = data2["tas"]["time"].values - os.remove(out[0]) - self.assertEqual(max(time1 - time2), 0) - - # Manipulate the recipe, sometimes it will be fine other times it will throw an error. - rp = self.MY_RP.copy() - with self.assertRaises(TypeError): - gridded_stitching("fake", rp) - with self.assertRaises(KeyError): - gridded_stitching(".", rp.drop("tas_file")) - - -if __name__ == "__main__": - unittest.main() +@pytest.mark.network +@pytest.mark.slow +def test_gridded_stitching_requires_file_column(tmp_path, recipe): + """Dropping ``tas_file`` raises `KeyError`.""" + with pytest.raises(KeyError): + gridded_stitching(str(tmp_path), recipe.drop(columns="tas_file"))