From 80ed05674b67f42ea33c87f4d9dd089ba7d110ec Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 20 Aug 2026 11:14:44 +0100 Subject: [PATCH 1/7] feat: add the model zoo for contributed models Adds `packages/pybamm-model-zoo/`, a uv workspace member importable as `pybamm_model_zoo`, as the home for community- and partner-contributed models. One self-contained folder per model holds its code, tests, examples, citation, and a declarative `model.toml` manifest. The manifest is the only boilerplate a contributor writes. The registry, the contract test suite, the docs pages, the CI routing, and the status badges are all derived from it, and manifests are parsed rather than imported, so a model whose dependencies are missing still appears in the registry and reports a clean failure instead of taking the zoo down. `pybamm_model_zoo.testing.contract.CHECKS` is the single definition of the contract: ten checks, each scoped as portable (`model`), in-tree wiring (`packaging`), or repository hygiene (`repo`). The test suite, the manifest's `skip_contract` validation, and the documented table all derive from that registry, and the scope is what lets the same shipped module hold a third-party collection to only the portable rules. Models are `community` tier (advisory CI) or `core` tier (in the merge gate). Advisory-ness lives in the CI job, never in an `xfail` marker, so a red community model reports red without blocking a PyBaMM merge. `spm_series_resistance` is the reference entry, at `core` tier so the gating path is exercised from the start. `nox -s zoo-new` renders the template and appends the CODEOWNERS line; `tests/test_template.py` renders that same template and runs the whole portable contract plus Ruff against the result, so "follow the template and CI is green on day one" is tested rather than hoped. The weekly `model_zoo_status` workflow matrixes on PyBaMM version and pairs a model only with releases its `pybamm_requires` admits, then opens a reviewable pull request with `status.json`, refreshed badges, and the docs compatibility table. Nothing under `packages/pybamm/src/pybamm/` changes; everything outside the zoo is config, CI, or docs. Addresses #5511 --- .github/CODEOWNERS | 5 + .github/ISSUE_TEMPLATE/new_zoo_model.yml | 62 +++ .github/workflows/_nox.yml | 6 + .github/workflows/model_zoo_status.yml | 175 +++++++ .github/workflows/test_on_push.yml | 40 +- .lycheeignore | 4 + .pre-commit-config.yaml | 8 + CHANGELOG.md | 4 + CONTRIBUTING.md | 17 + docs/index.rst | 1 + docs/source/model_zoo/contributing.md | 8 + docs/source/model_zoo/index.md | 46 ++ .../model_zoo/models/spm_series_resistance.md | 6 + noxfile.py | 81 +++- packages/pybamm-model-zoo/CHANGELOG.md | 12 + packages/pybamm-model-zoo/README.md | 167 +++++++ .../badges/spm_series_resistance.json | 6 + packages/pybamm-model-zoo/conftest.py | 92 ++++ packages/pybamm-model-zoo/pyproject.toml | 67 +++ packages/pybamm-model-zoo/scripts/generate.py | 88 ++++ packages/pybamm-model-zoo/scripts/matrix.py | 91 ++++ .../pybamm-model-zoo/scripts/new_model.py | 97 ++++ .../src/pybamm_model_zoo/__init__.py | 139 ++++++ .../src/pybamm_model_zoo/_citations.py | 47 ++ .../src/pybamm_model_zoo/_compat.py | 48 ++ .../src/pybamm_model_zoo/_docs.py | 231 ++++++++++ .../src/pybamm_model_zoo/_exceptions.py | 15 + .../src/pybamm_model_zoo/_paths.py | 28 ++ .../src/pybamm_model_zoo/_registry.py | 348 ++++++++++++++ .../src/pybamm_model_zoo/_template.py | 136 ++++++ .../spm_series_resistance/CITATION.bib | 19 + .../spm_series_resistance/README.md | 59 +++ .../spm_series_resistance/__init__.py | 3 + .../examples/run_spm_series_resistance.py | 27 ++ .../spm_series_resistance/model.py | 69 +++ .../spm_series_resistance/model.toml | 22 + .../tests/test_spm_series_resistance.py | 64 +++ .../src/pybamm_model_zoo/testing/__init__.py | 12 + .../src/pybamm_model_zoo/testing/contract.py | 428 ++++++++++++++++++ .../pybamm-model-zoo/template/CITATION.bib.in | 6 + .../pybamm-model-zoo/template/README.md.in | 35 ++ .../pybamm-model-zoo/template/__init__.py.in | 3 + .../template/examples/run_${slug}.py.in | 11 + .../pybamm-model-zoo/template/model.py.in | 47 ++ .../pybamm-model-zoo/template/model.toml.in | 31 ++ .../template/tests/test_${slug}.py.in | 32 ++ .../pybamm-model-zoo/tests/test_contract.py | 77 ++++ .../pybamm-model-zoo/tests/test_examples.py | 30 ++ .../pybamm-model-zoo/tests/test_registry.py | 148 ++++++ .../pybamm-model-zoo/tests/test_template.py | 117 +++++ pyproject.toml | 8 +- uv.lock | 19 + 52 files changed, 3325 insertions(+), 17 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/new_zoo_model.yml create mode 100644 .github/workflows/model_zoo_status.yml create mode 100644 docs/source/model_zoo/contributing.md create mode 100644 docs/source/model_zoo/index.md create mode 100644 docs/source/model_zoo/models/spm_series_resistance.md create mode 100644 packages/pybamm-model-zoo/CHANGELOG.md create mode 100644 packages/pybamm-model-zoo/README.md create mode 100644 packages/pybamm-model-zoo/badges/spm_series_resistance.json create mode 100644 packages/pybamm-model-zoo/conftest.py create mode 100644 packages/pybamm-model-zoo/pyproject.toml create mode 100644 packages/pybamm-model-zoo/scripts/generate.py create mode 100644 packages/pybamm-model-zoo/scripts/matrix.py create mode 100644 packages/pybamm-model-zoo/scripts/new_model.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_citations.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_compat.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_exceptions.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_paths.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/__init__.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py create mode 100644 packages/pybamm-model-zoo/template/CITATION.bib.in create mode 100644 packages/pybamm-model-zoo/template/README.md.in create mode 100644 packages/pybamm-model-zoo/template/__init__.py.in create mode 100644 packages/pybamm-model-zoo/template/examples/run_${slug}.py.in create mode 100644 packages/pybamm-model-zoo/template/model.py.in create mode 100644 packages/pybamm-model-zoo/template/model.toml.in create mode 100644 packages/pybamm-model-zoo/template/tests/test_${slug}.py.in create mode 100644 packages/pybamm-model-zoo/tests/test_contract.py create mode 100644 packages/pybamm-model-zoo/tests/test_examples.py create mode 100644 packages/pybamm-model-zoo/tests/test_registry.py create mode 100644 packages/pybamm-model-zoo/tests/test_template.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 52025a29fe..45ced34f7b 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -3,3 +3,8 @@ # The pybammsolvers package is owned by the IDAKLU maintainers (last match wins) /packages/pybammsolvers/ @pybamm-team/idaklu-maintainers + +# The model zoo: the zoo's own machinery is owned by the maintainers, and each +# model folder by its maintainer (last match wins) +/packages/pybamm-model-zoo/ @pybamm-team/maintainers +/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/ @pybamm-team/maintainers diff --git a/.github/ISSUE_TEMPLATE/new_zoo_model.yml b/.github/ISSUE_TEMPLATE/new_zoo_model.yml new file mode 100644 index 0000000000..1f27689b9c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/new_zoo_model.yml @@ -0,0 +1,62 @@ +name: New Model Zoo Entry +description: Propose a contributed model for the PyBaMM model zoo +labels: ["model zoo"] +body: + - type: markdown + attributes: + value: | + The [model zoo](https://docs.pybamm.org/en/latest/source/model_zoo/index.html) + is where community- and partner-contributed models live: one self-contained + folder per model, with its own maintainer, tests, examples, and citation. + See [contributing a model](https://docs.pybamm.org/en/latest/source/model_zoo/contributing.html) + for the workflow — `nox -s zoo-new` generates a skeleton that passes the + contract suite as rendered. + - type: input + id: model-name + attributes: + label: What is the model? + description: A one-line description of the physics it adds + placeholder: Stacked pouch cell with through-stack thermal transport + validations: + required: true + - type: input + id: code-location + attributes: + label: Where is the code? + description: A GitHub URL, pull request, or PyPI package name + placeholder: https://github.com/AwesomeOrg/AwesomeModel + validations: + required: true + - type: input + id: license + attributes: + label: What licence is it under? + description: See the [OSI's list of approved licences](https://opensource.org/licenses) + placeholder: BSD-3-Clause + validations: + required: true + - type: input + id: citation + attributes: + label: Is the model published? + description: A DOI or preprint link, if there is one. Leave blank if not. + - type: input + id: maintainer + attributes: + label: Who will maintain it? + description: The GitHub handle to add to CODEOWNERS for the model's folder + placeholder: "@ahandle" + validations: + required: true + - type: textarea + id: dependencies + attributes: + label: Does it need any third-party packages? + description: These become a `zoo-` optional extra, never a base dependency + - type: textarea + id: validation + attributes: + label: What has been validated, and against what? + description: | + A known limit, an analytic solution, a conservation law, or a published + figure. Please also say what has *not* been validated. diff --git a/.github/workflows/_nox.yml b/.github/workflows/_nox.yml index 0db579c1bd..23ffa4d15b 100644 --- a/.github/workflows/_nox.yml +++ b/.github/workflows/_nox.yml @@ -41,6 +41,11 @@ on: default: false required: false type: boolean + timeout_minutes: + description: "Per-leg timeout. GitHub's own default is 360." + default: 360 + required: false + type: number secrets: CODECOV_TOKEN: description: "Codecov upload token; only needed when upload_coverage is true." @@ -55,6 +60,7 @@ env: jobs: nox: runs-on: ${{ matrix.leg.os }} + timeout-minutes: ${{ inputs.timeout_minutes }} permissions: contents: read diff --git a/.github/workflows/model_zoo_status.yml b/.github/workflows/model_zoo_status.yml new file mode 100644 index 0000000000..2d8620d061 --- /dev/null +++ b/.github/workflows/model_zoo_status.yml @@ -0,0 +1,175 @@ +# Nothing is pushed to main: the results land in a reviewable pull request, so a +# newly-red model needs a human to accept it. +name: Model zoo status + +on: + schedule: + # Mondays, 04:00 UTC — after the weekly lychee sweep. + - cron: "0 4 * * 1" + workflow_dispatch: + +permissions: {} + +env: + PYBAMM_DISABLE_TELEMETRY: "true" + FORCE_COLOR: 3 + +jobs: + # Manifests are parsed, not imported, so the matrix needs no pybamm install and + # pairs a model only with the releases its `pybamm_requires` admits. + discover: + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + versions: ${{ steps.matrix.outputs.versions }} + cells: ${{ steps.matrix.outputs.include }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + version: "latest" + enable-cache: true + + - id: matrix + run: | + uv run --no-project --with packaging --python 3.13 \ + packages/pybamm-model-zoo/scripts/matrix.py \ + --github-output >> "$GITHUB_OUTPUT" + + test: + needs: discover + runs-on: ubuntu-latest + timeout-minutes: 90 + permissions: + contents: read + strategy: + fail-fast: false + # One cell per version, looping over models inside it: a cell is almost all + # environment setup, while a model's checks take about a second. + matrix: + version: ${{ fromJSON(needs.discover.outputs.versions) }} + name: PyBaMM ${{ matrix.version }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + submodules: 'recursive' + persist-credentials: false + + - name: Install Linux system dependencies + uses: awalsh128/cache-apt-pkgs-action@553a35bb8ebd9fcabcb1c9451aa4c98e1b4ca8a9 # v1.6.3 + with: + packages: gfortran gcc make cmake libopenblas-dev + execute_install_scripts: true + + - name: Set up Python 3.13 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: 3.13 + + - uses: astral-sh/setup-uv@c771a70e6277c0a99b617c7a806ffedaca235ff9 # v9.0.0 + with: + version: "latest" + enable-cache: true + + # --no-sources stops the workspace routing substituting the in-repo package + # for the released one; `main` is the checkout itself. + - name: Install PyBaMM ${{ matrix.version }} and the zoo + env: + VERSION: ${{ matrix.version }} + run: | + if [ "$VERSION" = main ]; then + uv sync --frozen --extra all --extra zoo-all --group dev + else + uv venv + uv pip install \ + --no-sources \ + "pybamm[all]==$VERSION" \ + -e "./packages/pybamm-model-zoo[zoo-all]" \ + --group packages/pybamm/pyproject.toml:dev + fi + + - name: Run the contract suite for each model this version admits + env: + MPLBACKEND: Agg + VERSION: ${{ matrix.version }} + CELLS: ${{ needs.discover.outputs.cells }} + run: | + mkdir -p results + echo "$CELLS" | uv run --no-sync python -c \ + 'import json,os,sys; print("\n".join(c["model"] for c in json.load(sys.stdin) if c["version"] == os.environ["VERSION"]))' \ + > models.txt + while read -r model; do + [ -n "$model" ] || continue + if uv run --no-sync python -m pytest -m zoo \ + packages/pybamm-model-zoo --zoo-model="$model"; then + result=pass + else + result=fail + fi + printf '{"model": "%s", "version": "%s", "result": "%s"}\n' \ + "$model" "$VERSION" "$result" > "results/$model--$VERSION.json" + done < models.txt + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: zoo-status-${{ matrix.version }} + path: results/ + retention-days: 7 + + collect: + needs: [discover, test] + if: always() + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # The status pull request is pushed from this checkout. + persist-credentials: true + + - name: Set up Python 3.13 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: 3.13 + + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: zoo-status-* + merge-multiple: true + path: results + + # The same generator the pre-commit hook runs, so the schema and the + # renderers that read it cannot drift apart. + - name: Fold the results into status.json, badges, and the docs table + run: python packages/pybamm-model-zoo/scripts/generate.py --collect results + + - name: Open or update the status pull request + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + BRANCH: chore/model-zoo-status + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git checkout -B "$BRANCH" + git add packages/pybamm-model-zoo/status.json \ + packages/pybamm-model-zoo/badges \ + docs/source/model_zoo + if git diff --cached --quiet; then + echo "Model zoo status unchanged; nothing to open." >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + git commit -m "chore: update model zoo status" + git push --force origin "$BRANCH" + if [ -z "$(gh pr list --head "$BRANCH" --state open --json number --jq '.[].number')" ]; then + gh pr create \ + --head "$BRANCH" \ + --base main \ + --title "chore: update model zoo status" \ + --body "Weekly model zoo compatibility matrix. Generated by \`model_zoo_status.yml\`; review the table before merging — a newly-red model needs an issue, not a merge." + fi diff --git a/.github/workflows/test_on_push.yml b/.github/workflows/test_on_push.yml index 9f66902efe..47e5a1140d 100644 --- a/.github/workflows/test_on_push.yml +++ b/.github/workflows/test_on_push.yml @@ -36,9 +36,13 @@ jobs: solver: ${{ steps.filter.outputs.solver }} pybamm: ${{ steps.filter.outputs.pybamm }} docs: ${{ steps.filter.outputs.docs }} + model_zoo: ${{ steps.filter.outputs.model_zoo }} # Routing decisions, resolved once here rather than repeated in every job's `if`. run_tests: ${{ github.event_name == 'push' || steps.filter.outputs.pybamm == 'true' || steps.filter.outputs.solver == 'true' }} run_docs_tests: ${{ github.event_name == 'push' || steps.filter.outputs.pybamm == 'true' || steps.filter.outputs.solver == 'true' || steps.filter.outputs.docs == 'true' }} + # A core change runs the zoo too, so an upstream regression shows up here + # rather than in a contributor's next pull request. + run_zoo_tests: ${{ github.event_name == 'push' || steps.filter.outputs.pybamm == 'true' || steps.filter.outputs.solver == 'true' || steps.filter.outputs.model_zoo == 'true' }} # Solver-wheel platforms each scenario's downstream jobs actually consume. platforms: ${{ steps.route.outputs.platforms }} macos_runners: ${{ steps.route.outputs.macos_runners }} @@ -65,6 +69,9 @@ jobs: - '.github/workflows/_nox.yml' docs: - 'docs/**' + model_zoo: + - 'packages/pybamm-model-zoo/**' + - '.github/workflows/test_on_push.yml' # Map "what changed" to the solver wheels to build: solver PRs validate every # platform; pybamm/push need the sparse matrix's runners; docs-only needs Linux. @@ -121,7 +128,7 @@ jobs: # old standalone unit/integration workflows. build_solver: needs: changes - if: ${{ needs.changes.outputs.run_docs_tests == 'true' }} + if: ${{ needs.changes.outputs.run_docs_tests == 'true' || needs.changes.outputs.run_zoo_tests == 'true' }} name: Solver wheels # Pass contents:read down to the called workflow (this workflow's top-level # permissions are {}, so the reusable workflow's checkout would otherwise @@ -282,6 +289,36 @@ jobs: texlive: false legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' + # The `core` tier: models the maintainers have adopted, so a failure blocks a + # merge exactly as a core test failure would. + run_zoo_gating: + needs: [changes, build_solver] + if: ${{ needs.changes.outputs.run_zoo_tests == 'true' }} + name: Model zoo (core tier) + permissions: + contents: read + uses: ./.github/workflows/_nox.yml + with: + sessions: zoo-gating + texlive: false + timeout_minutes: 30 + legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' + + # Advisory like run_unit_tests_advisory above: the whole zoo runs, `community` + # models included, and reports red without blocking a merge. + run_zoo_tests_advisory: + needs: [changes, build_solver] + if: ${{ needs.changes.outputs.run_zoo_tests == 'true' }} + name: Model zoo + permissions: + contents: read + uses: ./.github/workflows/_nox.yml + with: + sessions: zoo + texlive: false + timeout_minutes: 60 + legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' + # Single required check for branch protection. Green when every gated job either # succeeded or was routed around by `changes`; red on any failure or cancellation. ci_gate: @@ -298,6 +335,7 @@ jobs: - run_example_tests - run_scripts_tests - run_memory_tests + - run_zoo_gating runs-on: ubuntu-latest permissions: {} name: CI gate diff --git a/.lycheeignore b/.lycheeignore index 3fb7061524..4df366b5df 100644 --- a/.lycheeignore +++ b/.lycheeignore @@ -19,3 +19,7 @@ https://us.i.posthog.com # Live site behind a LiteSpeed anti-bot WAF that returns 415 to CI/datacenter # IPs (works fine from browsers and locally) — false positive, not a dead link https://bpxstandard.com/ + +# shields.io endpoint badges for model zoo status; the ?url= form confuses the +# checker, and the JSON they read is generated by the model_zoo_status workflow +https://img.shields.io/endpoint diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ee37733c13..22d446ec9b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -59,6 +59,14 @@ repos: files: ^packages/pybammsolvers/(src/pybammsolvers/version\.py|pyproject\.toml|vcpkg\.json)$ pass_filenames: false + - id: model-zoo-docs + name: Regenerate model zoo docs pages and badges from the manifests + entry: python packages/pybamm-model-zoo/scripts/generate.py + language: python + additional_dependencies: ["tomli; python_version < '3.11'"] + files: ^(packages/pybamm-model-zoo/(src/pybamm_model_zoo/[^/]+/(model\.toml|README\.md)|status\.json|scripts/generate\.py)|docs/source/model_zoo/.*)$ + pass_filenames: false + - id: check-ci-gate name: Check every CI job is merge-gated or explicitly advisory entry: python .github/scripts/check_ci_gate.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b30f000d67..1c80abe5be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) +## Features + +- Added the model zoo (`packages/pybamm-model-zoo/`), a home for community- and partner-contributed models: one self-contained folder per model, with a declarative `model.toml` manifest as the only boilerplate a contributor writes. The registry, a ten-check contract test suite, the docs pages, the CI routing, and the status badges are all derived from the manifests, which are parsed rather than imported so a broken model reports a clean failure instead of taking the zoo down. Models are either `community` tier (advisory CI) or `core` tier (in the merge gate); nothing in `pybamm` itself changed. ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) + ## Bug fixes - `BaseModel.parameters` now includes parameter symbols stored in `Variable` scale, reference, and bounds metadata. ([#5753](https://github.com/pybamm-team/PyBaMM/pull/5753)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df7ce15dc0..1459f2317d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,6 +32,23 @@ pre-commit run --all-files If you would like to skip the failing checks and push the code for further discussion, use the `--no-verify` option with `git commit`. +## Contributing a model to the model zoo + +Contributed battery models live in the [model zoo](https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo), +not in `pybamm` itself: one self-contained folder per model, with its own +maintainer, tests, examples, and citation. A declarative `model.toml` manifest is +the only boilerplate you write — the registry, the contract test suite, the docs +page, the CI routing, and your status badge are all derived from it. + +```bash +nox -s zoo-new -- --slug my_model --name MyModel --author "A. Author" --github ahandle +nox -s zoo -- --zoo-model=my_model +``` + +See [the zoo's contributing guide](https://docs.pybamm.org/en/latest/source/model_zoo/contributing.html) +for the tier policy, what the contract suite checks for you, and the review +checklist. + ## Workflow We use [GIT](https://en.wikipedia.org/wiki/Git) and [GitHub](https://en.wikipedia.org/wiki/GitHub) to coordinate our work. When making any kind of update, we try to follow the procedure below. diff --git a/docs/index.rst b/docs/index.rst index 58d8c34c20..be2dd931bd 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -16,6 +16,7 @@ PyBaMM documentation User Guide source/api/index source/examples/index + Model Zoo Contributing **Version**: |version| diff --git a/docs/source/model_zoo/contributing.md b/docs/source/model_zoo/contributing.md new file mode 100644 index 0000000000..e5f8ac5f46 --- /dev/null +++ b/docs/source/model_zoo/contributing.md @@ -0,0 +1,8 @@ +(model_zoo_contributing)= + + + +```{include} ../../../packages/pybamm-model-zoo/README.md +``` diff --git a/docs/source/model_zoo/index.md b/docs/source/model_zoo/index.md new file mode 100644 index 0000000000..b4bc0f5b59 --- /dev/null +++ b/docs/source/model_zoo/index.md @@ -0,0 +1,46 @@ +(model_zoo)= + + + +# Model zoo + +Community- and partner-contributed PyBaMM models. Each entry is one +self-contained folder with its own maintainer, tests, examples, and +citation; the table below is generated from those folders' manifests. + +Zoo models are reached through the zoo's registry, not the `pybamm` +namespace: + +```python +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load(zoo.list_models()[0])() +solution = pybamm.Simulation(model).solve([0, 3600]) +``` + +Models in the **core** tier are covered by PyBaMM's merge gate. Models in +the **community** tier are tested on every pull request too, but their +results are advisory: a red community model never blocks a PyBaMM merge. +See [contributing a model](contributing.md) to add your own. + +| Model | Tier | Maintainer | PyBaMM | Added | +| --- | --- | --- | --- | --- | +| [Single Particle Model with a lumped series resistance](models/spm_series_resistance.md) | core | @pybamm-team/maintainers | `>=26.0` | 2026-08-20 | + +## Compatibility + +Refreshed weekly by the `model_zoo_status` workflow, which runs each model +against every release its `pybamm_requires` admits, plus `main`. + +| Model | Results | Last passing | +| --- | --- | --- | +| spm_series_resistance | not yet run | — | + +```{toctree} +:hidden: +:maxdepth: 1 + +Contributing a model +models/spm_series_resistance +``` diff --git a/docs/source/model_zoo/models/spm_series_resistance.md b/docs/source/model_zoo/models/spm_series_resistance.md new file mode 100644 index 0000000000..1bed556c10 --- /dev/null +++ b/docs/source/model_zoo/models/spm_series_resistance.md @@ -0,0 +1,6 @@ +(model-zoo-spm_series_resistance)= + + + +```{include} ../../../../packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md +``` diff --git a/noxfile.py b/noxfile.py index 9d58e2394b..18f845f3f2 100644 --- a/noxfile.py +++ b/noxfile.py @@ -39,25 +39,19 @@ def is_macos_intel(): return sys.platform == "darwin" and platform.machine() in ("x86_64", "i386") -def install_locked(session, *, extras=None, groups=None): +def install_locked(session, *, extras=None, groups=None, zoo=False, zoo_extras=None): """Install pybamm and its dependencies into the session environment. Two modes, selected by the ``PYBAMM_SOLVER_WHEELS`` environment variable: - * **Unset (local dev, solver CI):** ``uv sync --frozen`` over the workspace. - ``pybammsolvers`` is built from the in-repo source via the shared lockfile. + * **Unset:** ``uv sync --frozen``, so ``pybammsolvers`` builds from source. + * **Set to a wheel directory (the CI matrix):** install this interpreter's + wheel by path, then pybamm with ``--no-sources``. The in-repo solver + version collides with the PyPI release, so nothing weaker picks the right + artifact, and Windows has no from-source build. - * **Set to a directory of prebuilt solver wheels (the PyBaMM CI matrix):** - install the wheel matching this interpreter by explicit path, then install - pybamm with ``--no-sources`` so the workspace source routing is ignored. - This tests PyBaMM against the *prebuilt in-repo* ``pybammsolvers`` without - recompiling it in every matrix cell (and without needing a from-source - build on Windows, which only exists via cibuildwheel/vcpkg). - - ``--no-sources`` plus an explicit wheel path is required because the in-repo - solver version collides with the PyPI release, so neither ``--find-links`` - resolution nor the workspace source can be relied on to select the in-repo - artifact over the identically-versioned PyPI wheel. + ``zoo=True`` also installs the zoo with ``zoo_extras``, which only the + prebuilt-wheel path needs — a workspace sync installs every member anyway. """ env = {"UV_PROJECT_ENVIRONMENT": session.virtualenv.location} @@ -82,6 +76,7 @@ def install_locked(session, *, extras=None, groups=None): session.bin, "python.exe" if sys.platform == "win32" else "python" ) extras_str = f"[{','.join(extras)}]" if extras else "" + zoo_extras_str = f"[{','.join(zoo_extras)}]" if zoo_extras else "" cmd = [ "uv", "pip", @@ -93,6 +88,8 @@ def install_locked(session, *, extras=None, groups=None): "-e", f"./packages/pybamm{extras_str}", ] + if zoo: + cmd.extend(["-e", f"./packages/pybamm-model-zoo{zoo_extras_str}"]) for group in groups or []: # Groups (dev, docs) are defined in the pybamm package, not the root. cmd.extend(["--group", f"packages/pybamm/pyproject.toml:{group}"]) @@ -100,7 +97,7 @@ def install_locked(session, *, extras=None, groups=None): return cmd = ["uv", "sync", "--frozen"] - for extra in extras or []: + for extra in [*(extras or []), *(zoo_extras or [])]: cmd.extend(["--extra", extra]) for group in groups or []: cmd.extend(["--group", group]) @@ -320,6 +317,60 @@ def build_docs(session): ) +ZOO_TESTS = "packages/pybamm-model-zoo" + + +def install_zoo(session): + """Install pybamm plus the zoo and every model's declared dependencies.""" + set_environment_variables(PYBAMM_ENV, session=session) + install_locked( + session, extras=["all"], groups=["dev"], zoo=True, zoo_extras=["zoo-all"] + ) + + +def zoo_pytest(session, marker): + """Run the zoo suite, selecting by marker.""" + session.run("python", "-m", "pytest", "-m", marker, ZOO_TESTS, *session.posargs) + + +@nox.session(name="zoo", default=False) +def run_zoo(session): + """Run the whole model zoo suite: contract, model tests, and examples.""" + install_zoo(session) + zoo_pytest(session, "zoo") + + +@nox.session(name="zoo-gating", default=False) +def run_zoo_gating(session): + """Run only the `core`-tier zoo models, which are in PyBaMM's merge gate.""" + install_zoo(session) + zoo_pytest(session, "zoo and gating and not zoo_examples") + + +@nox.session(name="zoo-examples", default=False) +def run_zoo_examples(session): + """Run every model zoo example script.""" + install_zoo(session) + zoo_pytest(session, "zoo_examples") + + +# No install: the generator reads manifests with tomllib and never imports pybamm, +# so building an environment for it would cost minutes to do milliseconds of work. +@nox.session(name="zoo-docs", default=False, venv_backend="none") +def run_zoo_docs(session): + """Regenerate the model zoo docs pages and badges from the manifests.""" + session.run("python", f"{ZOO_TESTS}/scripts/generate.py", *session.posargs) + + +@nox.session(name="zoo-new", default=False) +def run_zoo_new(session): + """Create a new model zoo entry from the template.""" + # Only the zoo itself and pybamm's version metadata are needed, so this skips + # the extras and the dev group that install_zoo pulls in. + install_locked(session, zoo=True) + session.run("python", f"{ZOO_TESTS}/scripts/new_model.py", *session.posargs) + + @nox.session(name="pre-commit", default=True) def lint(session): """Check all files against the defined pre-commit hooks.""" diff --git a/packages/pybamm-model-zoo/CHANGELOG.md b/packages/pybamm-model-zoo/CHANGELOG.md new file mode 100644 index 0000000000..01f6a6bb01 --- /dev/null +++ b/packages/pybamm-model-zoo/CHANGELOG.md @@ -0,0 +1,12 @@ +# Changelog + +The model zoo has its own changelog so that zoo pull requests never touch +PyBaMM's. + +## [Unreleased] + +### Added + +- The model zoo: per-model manifests, a registry, a ten-check contract suite, a + template and generator, generated docs pages and status badges, and the + `spm_series_resistance` reference entry ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) diff --git a/packages/pybamm-model-zoo/README.md b/packages/pybamm-model-zoo/README.md new file mode 100644 index 0000000000..1d8ed43793 --- /dev/null +++ b/packages/pybamm-model-zoo/README.md @@ -0,0 +1,167 @@ +# PyBaMM model zoo + +Community- and partner-contributed PyBaMM models. One folder per model, holding +its code, tests, examples, citation, and a declarative `model.toml` manifest. + +The manifest is the only boilerplate you write. The registry, the contract test +suite, the docs page, the CI routing, and the status badge are all derived from +it. Manifests are parsed, never imported, so a model whose dependencies are +missing still appears in the registry and the docs and reports a clean failure +rather than taking the zoo down. + +```python +import pybamm +import pybamm_model_zoo as zoo + +zoo.list_models() +entry = zoo.info("SPMSeriesResistance") +entry.tier, entry.maintainers, entry.pybamm_requires + +model = zoo.load("SPMSeriesResistance")() +solution = pybamm.Simulation(model).solve([0, 3600]) +print(solution["Voltage [V]"](1800)) +``` + +The zoo is a `uv` workspace member, so `uv sync --extra all --group dev` from the +repository root installs it editable alongside `pybamm`. It is not published to +PyPI. + +## Adding a model + +```bash +nox -s zoo-new -- --slug my_model --name MyModel --author "A. Author" --github ahandle +``` + +That renders [`template/`](https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo/template) into +`src/pybamm_model_zoo/my_model/` and appends your `.github/CODEOWNERS` line, so +changes to your folder request you as reviewer. The rendered skeleton passes the +whole contract suite as generated — the zoo's own test suite renders the template +and runs every check against the result, so that is a tested claim rather than a +hope. + +Then: + +1. Replace the TODOs in `model.toml`, `README.md`, and `CITATION.bib`. +2. Put your physics in `model.py`. +3. Write at least one test that pins a **physical** result — a known limit, an + analytic solution, a conservation check, or a published figure. That the model + merely runs is already covered by the contract suite, so a test that only + checks it runs adds nothing. +4. Regenerate the docs: `nox -s zoo-docs`. +5. Run it: `nox -s zoo -- --zoo-model=my_model`. +6. Add a bullet to this package's `CHANGELOG.md`. Zoo pull requests never touch + PyBaMM's changelog. + +[`spm_series_resistance/`](https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance) +is the reference entry: the smallest thing that is still a real model. Copy that +folder rather than reading prose. + +### Dependencies + +A model's third-party dependencies go in a per-model **optional** extra named +`zoo-` (dashes, not underscores) in this package's `pyproject.toml`, listed +in the `zoo-all` aggregate — never in the base dependencies. Workspace members +share one lockfile and one dev venv, so a base dependency here lands in every +contributor's environment. + +The aggregate is deliberately not called `all`: `uv sync --extra all` applies the +extra to every workspace member that defines one, so an `all` extra here would +drag every model's heavy dependencies into the core dev environment. + +Declare the same requirements in your manifest, and the contract suite checks +that the two agree: + +```toml +[model.dependencies] +extra = "zoo-my-model" +packages = ["scikit-fem>=12.0.2"] +``` + +## Tiers + +| Tier | Who sets it | CI | +| --- | --- | --- | +| `community` | the default for a new model | Runs on every pull request, **advisory**: reports red without blocking a merge. | +| `core` | PyBaMM maintainers, by adopting a model | Runs in PyBaMM's merge gate. A failure blocks a merge, in core and in the zoo. | + +Advisory-ness lives in the CI job, never in an `xfail` marker: a community model +that fails, fails, and that is exactly the signal its badge reports. Being +advisory is what lets the zoo test contributed models on every pull request +without a contributed model ever blocking a PyBaMM release. The manifest tier is +what puts a model's tests behind the `gating` marker. + +## Testing + +Three layers; only the middle one is yours. + +| Layer | What | Where | +| --- | --- | --- | +| A | Contract suite — every check below, against every registered model, automatically | `tests/test_contract.py`, checks in `pybamm_model_zoo.testing.contract` | +| B | Your physics tests | `src/pybamm_model_zoo//tests/` | +| C | Your example scripts, executed | `tests/test_examples.py` over `/examples/*.py` | + +### The contract + +`pybamm_model_zoo.testing.contract.CHECKS` is the one definition of the contract: +the test suite, the manifest's `skip_contract` validation, and this table all +derive from it. Each check has a **scope**, which is what lets the same module +hold an in-tree model and a third-party collection to the right rules. + +| Check | Scope | What it asserts | +| --- | --- | --- | +| `manifest` | model | Schema valid; `slug` matches the folder name; `class` is parseable; at least one maintainer; `pybamm_requires` is a valid specifier satisfied by the installed PyBaMM. | +| `layout` | model | `README.md`, `CITATION.bib`, `examples/`, and `tests/` present, and the README has `Summary`, `Usage`, `Validation`, and `Citation` sections. | +| `import` | model | The declared class imports and subclasses `pybamm.BaseModel`. Skipped with a reason when your declared extra is absent. | +| `citation` | model | The manifest's citation key resolves in your `CITATION.bib`, and instantiating the model registers it, so `pybamm.print_citations()` credits you. | +| `well_posed` | model | `model.check_well_posedness()` passes. | +| `build` | model | `pybamm.Simulation(model).build()` succeeds. | +| `solve` | model | The model solves for the manifest's `solve_time`, and every `key_variables` entry is finite read through the interpolating call interface. | +| `packaging` | packaging | An in-tree model is importable as `pybamm_model_zoo.`, and any extra it declares exists and is aggregated into `zoo-all`. | +| `docs` | repo | The generated docs page is present *and current*, so docs cannot drift from code. | +| `codeowners` | repo | `.github/CODEOWNERS` names an owner for your folder, so ownership cannot be dropped silently. | + +A third-party collection is held only to the `model` scope; it is not wired into +this package and does not live in this repository. + +`skip_contract` waives an individual check for a genuinely unusual model — one +with no meaningful standalone solve, say. It is per-check, reviewed, and visible +in the manifest diff. The `manifest` check itself cannot be waived. + +```bash +nox -s zoo # the whole zoo, both tiers, plus examples +nox -s zoo -- --zoo-model=my_model # one model +nox -s zoo-gating # only core-tier models (the merge gate) +nox -s zoo-examples # every model's example scripts +nox -s zoo-docs # regenerate docs pages and badges +``` + +Aim for contract checks under 60 s per model and a model's own tests under +5 minutes; `solve_time` in the manifest is the lever. + +## External model collections + +A third-party package can register its own directory of manifests and be +discovered by the same registry: + +```toml +[project.entry-points."pybamm_zoo_models"] +my_lab_models = "my_lab_models" +``` + +It can also hold itself to the `model`-scope contract in its own CI, since the +checks ship in `pybamm_model_zoo.testing.contract`. In-tree models win a name +collision, so a third-party package cannot shadow one. + +## Review checklist for a zoo pull request + +Short by design — the contract suite does the rest. + +- [ ] License is OSI-approved and compatible with BSD-3-Clause. +- [ ] No edits to `packages/pybamm/src/` in the same pull request; core changes + are split out. +- [ ] Third-party dependencies declared as a `zoo-` extra. +- [ ] At least one test pins a physical result. +- [ ] The example script runs. +- [ ] The citation resolves and is registered on instantiation. +- [ ] `.github/CODEOWNERS` line added. +- [ ] `CHANGELOG.md` entry in this package. diff --git a/packages/pybamm-model-zoo/badges/spm_series_resistance.json b/packages/pybamm-model-zoo/badges/spm_series_resistance.json new file mode 100644 index 0000000000..d48abfc2c7 --- /dev/null +++ b/packages/pybamm-model-zoo/badges/spm_series_resistance.json @@ -0,0 +1,6 @@ +{ + "schemaVersion": 1, + "label": "model zoo", + "message": "untested", + "color": "lightgrey" +} diff --git a/packages/pybamm-model-zoo/conftest.py b/packages/pybamm-model-zoo/conftest.py new file mode 100644 index 0000000000..cf1c255679 --- /dev/null +++ b/packages/pybamm-model-zoo/conftest.py @@ -0,0 +1,92 @@ +"""Zoo-wide pytest configuration. + +Duplicates the core package's autouse fixtures because +``packages/pybamm/conftest.py`` is not importable from here; extracting a shared +``pybamm.testing`` pytest plugin would mean changing the core package, which this +work deliberately does not. +""" + +import numpy as np +import pytest + +import pybamm +import pybamm_model_zoo as zoo + +MODEL_TESTS_PARENT = "pybamm_model_zoo" + + +def pytest_addoption(parser): + parser.addoption( + "--zoo-model", + action="store", + default=None, + metavar="SLUG", + help=( + "run only the tests belonging to one model, so CI can test just what a " + "pull request changed" + ), + ) + + +def _slug_from_path(path): + """The model a test file belongs to, from ``/tests/test_*.py``.""" + parts = path.parts + if "tests" not in parts: + return None + index = parts.index("tests") + if index >= 2 and parts[index - 2] == MODEL_TESTS_PARENT: + return parts[index - 1] + return None + + +def _slug_of(item): + """The model an item belongs to: its own marker, else its path. + + Parametrized suites mark each case with ``zoo_model(slug)`` rather than + relying on an argument name, so renaming a fixture cannot silently empty the + merge gate. + """ + marker = item.get_closest_marker("zoo_model") + return marker.args[0] if marker else _slug_from_path(item.path) + + +def pytest_collection_modifyitems(config, items): + core_slugs = {entry.slug for entry in zoo.all_entries() if entry.tier == "core"} + selected = config.getoption("--zoo-model") + deselected = [] + remaining = [] + for item in items: + item.add_marker(pytest.mark.zoo) + if "integration" in item.path.parts: + item.add_marker(pytest.mark.integration) + elif "memory" not in item.path.parts: + item.add_marker(pytest.mark.unit) + + slug = _slug_of(item) + # Advisory-ness lives in the CI job, never in a marker: `gating` only + # says whether a failure blocks a merge. + if slug in core_slugs: + item.add_marker(pytest.mark.gating) + if selected is not None and slug != selected: + deselected.append(item) + else: + remaining.append(item) + + if deselected: + config.hook.pytest_deselected(items=deselected) + items[:] = remaining + + +@pytest.fixture(autouse=True) +def set_random_seed(): + np.random.seed(42) + + +@pytest.fixture(autouse=True) +def set_debug_value(): + pybamm.settings.debug_mode = True + + +@pytest.fixture(autouse=True) +def disable_telemetry(): + pybamm.telemetry.disable() diff --git a/packages/pybamm-model-zoo/pyproject.toml b/packages/pybamm-model-zoo/pyproject.toml new file mode 100644 index 0000000000..dbdff9a438 --- /dev/null +++ b/packages/pybamm-model-zoo/pyproject.toml @@ -0,0 +1,67 @@ +[build-system] +requires = ["hatchling>=1.31.0"] +build-backend = "hatchling.build" + +[project] +name = "pybamm-model-zoo" +version = "0.1.0" +license = "BSD-3-Clause" +description = "Community- and partner-contributed models for PyBaMM" +authors = [{ name = "The PyBaMM Team", email = "pybamm@pybamm.org" }] +maintainers = [{ name = "The PyBaMM Team", email = "pybamm@pybamm.org" }] +requires-python = ">=3.10, <3.15" +readme = { file = "README.md", content-type = "text/markdown" } +classifiers = [ + "Development Status :: 3 - Alpha", + "Intended Audience :: Science/Research", + "Programming Language :: Python :: 3", + "Topic :: Scientific/Engineering", +] +dependencies = [ + "pybamm", + "packaging>=23.0", + "tomli>=2.0.1; python_version < '3.11'", +] + +[project.urls] +Homepage = "https://pybamm.org" +Documentation = "https://docs.pybamm.org/en/latest/source/model_zoo/index.html" +Repository = "https://github.com/pybamm-team/PyBaMM" + +# A model's dependencies belong in a per-model `zoo-` extra, never in the +# base dependencies above: workspace members share one lockfile and one dev venv. +[project.optional-dependencies] +# The aggregate is deliberately not named `all`: `uv sync --extra all` applies it +# to every member, dragging every model's heavy dependencies into the dev env. +zoo-all = [] + +[tool.hatch.build.targets.wheel] +packages = ["src/pybamm_model_zoo"] + +# pytest treats a pyproject.toml as the inifile only when it carries this table, +# and the workspace root has none, so `--strict-markers` would reject the markers. +[tool.pytest.ini_options] +minversion = "9.0" +required_plugins = ["pytest-xdist", "pytest-mock"] +addopts = ["-nauto", "-vra", "--strict-config", "--strict-markers"] +testpaths = ["tests", "src/pybamm_model_zoo"] +console_output_style = "progress" +xfail_strict = true +markers = [ + "zoo: mark test as part of the model zoo suite", + "gating: mark test as merge-gating (derived from a manifest's `core` tier)", + "zoo_examples: mark test as a model zoo example script", + "zoo_model(slug): mark test as belonging to one model, for --zoo-model", + "unit: mark test as a unit test", + "integration: mark test as an integration test", +] +filterwarnings = [ + "error", + "ignore::DeprecationWarning", + "ignore::UserWarning", + "ignore::RuntimeWarning", +] +log_cli = true +log_level = "INFO" +log_format = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" +log_date_format = "%Y-%m-%d %H:%M:%S" diff --git a/packages/pybamm-model-zoo/scripts/generate.py b/packages/pybamm-model-zoo/scripts/generate.py new file mode 100644 index 0000000000..56ac0000ae --- /dev/null +++ b/packages/pybamm-model-zoo/scripts/generate.py @@ -0,0 +1,88 @@ +"""Regenerate the model zoo docs pages and status badges from the manifests. + +A thin command line over :mod:`pybamm_model_zoo._docs`, which owns the rendering +so the ``docs`` contract check can compare a page against what it should contain. + + uv run python packages/pybamm-model-zoo/scripts/generate.py [--check] + uv run python packages/pybamm-model-zoo/scripts/generate.py --collect results/ +""" + +from __future__ import annotations + +import argparse +import datetime +import json +import sys +from pathlib import Path + +# Run from a checkout without installing: the zoo's src/ is a sibling of scripts/. +sys.path.insert(0, str(Path(__file__).parents[1] / "src")) + +from pybamm_model_zoo import _docs, _paths +from pybamm_model_zoo._registry import Registry + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--check", + action="store_true", + help="report out-of-date files without writing them", + ) + parser.add_argument( + "--collect", + metavar="DIR", + type=Path, + help=( + "fold one JSON file per compatibility-matrix cell into status.json " + "before rendering" + ), + ) + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv) + + if args.collect: + generated = ( + datetime.datetime.now(datetime.timezone.utc) + .replace(microsecond=0) + .isoformat() + .replace("+00:00", "Z") + ) + status = _docs.collect_results(args.collect, generated) + _paths.STATUS_FILE.write_text( + json.dumps(status, indent=2) + "\n", encoding="utf-8" + ) + print(f"wrote {_paths.STATUS_FILE.relative_to(_paths.REPO_ROOT)}") + + entries = sorted(Registry().values(), key=lambda entry: entry.slug) + files = _docs.all_files(entries) + outdated = [ + path + for path, content in files.items() + if not path.is_file() or path.read_text(encoding="utf-8") != content + ] + removed = _docs.stale(files) + + if args.check: + for path in outdated + removed: + print(f"out of date: {path.relative_to(_paths.REPO_ROOT)}") + if outdated or removed: + print("run `nox -s zoo-docs`", file=sys.stderr) + return 1 + return 0 + + for path in outdated: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(files[path], encoding="utf-8") + print(f"wrote {path.relative_to(_paths.REPO_ROOT)}") + for path in removed: + path.unlink() + print(f"removed {path.relative_to(_paths.REPO_ROOT)}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/pybamm-model-zoo/scripts/matrix.py b/packages/pybamm-model-zoo/scripts/matrix.py new file mode 100644 index 0000000000..c188762d91 --- /dev/null +++ b/packages/pybamm-model-zoo/scripts/matrix.py @@ -0,0 +1,91 @@ +"""Emit the model zoo compatibility matrix for the weekly status workflow. + +Prints a GitHub Actions ``include`` list: one ``{model, version}`` cell per pair. +A pair a manifest's ``pybamm_requires`` excludes is left out, so a badge never +reports "failing" on a release the model never claimed to support. + + uv run --with packaging python packages/pybamm-model-zoo/scripts/matrix.py +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +import urllib.request +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parents[1] / "src")) + +from pybamm_model_zoo._registry import Registry + +PYPI_URL = "https://pypi.org/pypi/pybamm/json" +#: Final CalVer releases only: no prereleases, no yanked-empty entries. +CALVER = re.compile(r"^\d+(\.\d+)*$") +#: The checkout itself, which has no release number to match a specifier against. +MAIN = "main" + + +def version_order(version: str) -> tuple[int, list[int]]: + """Sort releases numerically, and sort ``main`` last.""" + try: + return (0, [int(part) for part in version.split(".")]) + except ValueError: + return (1, []) + + +def released_versions(count: int) -> list[str]: + """The ``count`` most recent final PyBaMM releases on PyPI, oldest first.""" + with urllib.request.urlopen(PYPI_URL) as response: + releases = json.load(response)["releases"] + published = sorted( + ( + version + for version, files in releases.items() + if files and CALVER.match(version) + ), + key=version_order, + ) + return published[-count:] + + +def matrix(versions: list[str]) -> list[dict[str, str]]: + """One cell per (model, version) pair the model's declared range admits.""" + cells = [] + for entry in sorted(Registry().values(), key=lambda entry: entry.slug): + for version in versions: + if version == MAIN or entry.admits(version): + cells.append({"model": entry.slug, "version": version}) + return cells + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--releases", + type=int, + default=2, + help="how many of the most recent PyPI releases to test against", + ) + parser.add_argument( + "--github-output", + action="store_true", + help="print `include=` and `versions=` lines for $GITHUB_OUTPUT", + ) + args = parser.parse_args(argv) + + cells = matrix([*released_versions(args.releases), MAIN]) + if args.github_output: + # The workflow matrixes on version and loops models inside the cell, so + # it needs the versions that survived filtering as well as the pairs. + versions = sorted({cell["version"] for cell in cells}, key=version_order) + print(f"include={json.dumps(cells)}") + print(f"versions={json.dumps(versions)}") + else: + print(json.dumps(cells)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/pybamm-model-zoo/scripts/new_model.py b/packages/pybamm-model-zoo/scripts/new_model.py new file mode 100644 index 0000000000..5626ace384 --- /dev/null +++ b/packages/pybamm-model-zoo/scripts/new_model.py @@ -0,0 +1,97 @@ +"""Create a new model zoo entry from the template. + + uv run python packages/pybamm-model-zoo/scripts/new_model.py \ + --slug my_model --name MyModel --author "A. Author" --github ahandle + +Also available as ``nox -s zoo-new -- --slug ... --name ...``. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +# Run from a checkout without installing: the zoo's src/ is a sibling of scripts/. +sys.path.insert(0, str(Path(__file__).parents[1] / "src")) + +from pybamm_model_zoo import _template +from pybamm_model_zoo._exceptions import ZooError +from pybamm_model_zoo._paths import CODEOWNERS, PACKAGE_ROOT, REPO_ROOT +from pybamm_model_zoo._registry import TIERS + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--slug", required=True, help="folder name, lower_snake_case") + parser.add_argument("--name", required=True, help="registry key and class name") + parser.add_argument("--author", required=True, help="maintainer's name") + parser.add_argument("--github", required=True, help="maintainer's GitHub handle") + parser.add_argument("--tier", default="community", choices=TIERS) + parser.add_argument("--license", default="BSD-3-Clause", help="SPDX identifier") + parser.add_argument( + "--dry-run", action="store_true", help="report what would be written" + ) + return parser.parse_args(argv) + + +def append_codeowners(slug: str, github: str, *, dry_run: bool) -> str: + line = _template.codeowners_line(slug, github) + if dry_run: + return line + text = CODEOWNERS.read_text(encoding="utf-8") + if line in text: + return line + separator = "" if text.endswith("\n") else "\n" + CODEOWNERS.write_text(f"{text}{separator}{line}\n", encoding="utf-8") + return line + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv) + values = _template.tokens( + slug=args.slug, + name=args.name, + author=args.author, + github=args.github, + tier=args.tier, + license=args.license, + ) + destination = PACKAGE_ROOT / args.slug + if args.dry_run: + print(f"would render the template into {destination}") + for target in _template.planned(destination, values).values(): + print(f" {target}") + print(f"would add to {CODEOWNERS}:") + print(f" {append_codeowners(args.slug, args.github, dry_run=True)}") + return 0 + + written = _template.render(destination, values) + line = append_codeowners(args.slug, args.github, dry_run=False) + + print(f"Created {len(written)} files in {destination.relative_to(REPO_ROOT)}:") + for path in written: + print(f" {path.relative_to(REPO_ROOT)}") + print(f"\nAdded to .github/CODEOWNERS:\n {line}") + print( + "\nNext:\n" + f" 1. Replace the TODOs in {args.slug}/model.toml, README.md, and " + "CITATION.bib.\n" + " 2. Put your physics in model.py, and a test that pins a physical\n" + " result in tests/.\n" + " 3. If your model needs third-party packages, add a\n" + f" '{values['extra']}' extra to\n" + " packages/pybamm-model-zoo/pyproject.toml and list it in 'zoo-all'.\n" + " 4. uv run python packages/pybamm-model-zoo/scripts/generate.py\n" + f" 5. nox -s zoo -- --zoo-model={args.slug}\n" + " 6. Add a CHANGELOG.md bullet in packages/pybamm-model-zoo/." + ) + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except ZooError as error: + print(f"error: {error}", file=sys.stderr) + raise SystemExit(1) from error diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py new file mode 100644 index 0000000000..1facf47ece --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py @@ -0,0 +1,139 @@ +"""The PyBaMM model zoo: community- and partner-contributed battery models. + +Each model lives in its own folder alongside a declarative ``model.toml`` +manifest. The manifests are the single source of truth for the registry, the +contract test suite, the docs pages, and the CI routing. + +Examples +-------- +>>> import pybamm_model_zoo as zoo +>>> "SPMSeriesResistance" in zoo.list_models() +True +>>> entry = zoo.info("SPMSeriesResistance") +>>> entry.tier +'core' +""" + +from __future__ import annotations + +from pathlib import Path + +from pybamm_model_zoo._citations import read_citations +from pybamm_model_zoo._exceptions import ( + ManifestError, + ModelUnavailableError, + ZooError, +) +from pybamm_model_zoo._registry import ( + ENTRY_POINT_GROUP, + TIERS, + ModelEntry, + Registry, +) + +__all__ = [ + "ENTRY_POINT_GROUP", + "TIERS", + "ManifestError", + "ModelEntry", + "ModelUnavailableError", + "Registry", + "ZooError", + "all_entries", + "info", + "list_models", + "load", + "read_citations", + "refresh", + "register_citation", + "registry", +] + +_registry: Registry | None = None + + +def registry() -> Registry: + """Return the model registry, building it on first use.""" + global _registry + if _registry is None: + _registry = Registry() + return _registry + + +def refresh( + paths: list[Path] | None = None, *, external_paths: list[Path] | None = None +) -> Registry: + """Rebuild the registry, optionally from explicit search paths. + + Parameters + ---------- + paths : list of Path, optional + Directories of model folders held to the in-tree contract. Defaults to the + zoo's own model directory. + external_paths : list of Path, optional + Directories of third-party model folders. Defaults to those advertised + through the ``pybamm_zoo_models`` entry point. + """ + global _registry + _registry = Registry(paths, external_paths=external_paths) + return _registry + + +def list_models() -> list[str]: + """The names of every registered model, sorted.""" + return sorted(registry()) + + +def all_entries() -> list[ModelEntry]: + """Every registered entry, sorted by slug.""" + return sorted(registry().values(), key=lambda entry: entry.slug) + + +def info(name: str) -> ModelEntry: + """Return the manifest-derived metadata for a registered model.""" + return registry()[name] + + +def load(name: str) -> type: + """Import and return a registered model class. + + Raises + ------ + KeyError + If ``name`` is not registered. + ModelUnavailableError + If the model's code or its declared extra is unavailable. + """ + return registry()[name].load() + + +def register_citation(slug: str, *keys: str) -> None: + """Credit a zoo model's references through :func:`pybamm.print_citations`. + + Call this from a model's ``__init__`` so that using the model cites its + author. With no ``keys``, the manifest's ``citation.key`` is registered. + + Parameters + ---------- + slug : str + The model's folder name. + *keys : str + Citation keys to register from the folder's ``CITATION.bib``. Defaults to + the manifest's ``citation.key``. + + Raises + ------ + ManifestError + If the folder has no ``CITATION.bib`` or a key is not in it. + """ + import pybamm + + entry = registry().by_slug(slug) + citations = read_citations(entry.path) + for key in keys or (entry.citation_key,): + if key not in citations: + raise ManifestError( + f"{entry.path / 'CITATION.bib'}: no entry for '{key}'. " + f"Found: {', '.join(sorted(citations)) or 'none'}." + ) + pybamm.citations.register(citations[key]) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_citations.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_citations.py new file mode 100644 index 0000000000..f41109b4f1 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_citations.py @@ -0,0 +1,47 @@ +"""Reading a model folder's ``CITATION.bib`` and crediting it through PyBaMM.""" + +from __future__ import annotations + +import re +from pathlib import Path + +from pybamm_model_zoo._exceptions import ManifestError + +CITATION_FILE = "CITATION.bib" +_ENTRY_START = re.compile(r"@(?P\w+)\s*\{\s*(?P[^,\s}]+)\s*,") + + +def parse_bibtex(text: str) -> dict[str, str]: + """Split BibTeX source into ``{key: entry source}``. + + A brace-matching scan rather than a full parser: it keeps the zoo free of a + hard ``pybtex`` dependency, and PyBaMM parses the entry properly when the + citation is printed. + """ + entries: dict[str, str] = {} + for match in _ENTRY_START.finditer(text): + start = match.start() + depth = 0 + for index in range(text.index("{", start), len(text)): + if text[index] == "{": + depth += 1 + elif text[index] == "}": + depth -= 1 + if depth == 0: + entries[match.group("key")] = text[start : index + 1] + break + return entries + + +def read_citations(directory: Path) -> dict[str, str]: + """Parse the ``CITATION.bib`` in ``directory``. + + Raises + ------ + ManifestError + If the file is missing. + """ + path = Path(directory) / CITATION_FILE + if not path.is_file(): + raise ManifestError(f"{path}: no such file") + return parse_bibtex(path.read_text(encoding="utf-8")) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_compat.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_compat.py new file mode 100644 index 0000000000..fac7c72222 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_compat.py @@ -0,0 +1,48 @@ +"""Workarounds for PyBaMM behaviour the zoo cannot fix from here. + +The zoo changes nothing under ``packages/pybamm/src`` by design, so the upstream +quirks it has to work around are collected here instead of being copied into each +model. Every function names the upstream change that would retire it, so the list +doubles as the inventory of core fixes the zoo is waiting on. +""" + +from __future__ import annotations + +from typing import Any + + +def spm_default_options(options: dict[str, Any] | None) -> dict[str, Any]: + """Options an ``SPM`` subclass needs but does not inherit. + + ``pybamm.lithium_ion.SPM.__init__`` defaults ``"x-average side reactions"`` + with ``self.__class__ in [SPM, MPM]``, so a subclass is left with ``"false"`` + and then rejected by the option validator. Retire this once SPM carries the + default as a class attribute its subclasses inherit. + """ + return {"x-average side reactions": "true", **(options or {})} + + +def cited_keys() -> set[str]: + """The citation keys PyBaMM would credit if asked to print right now. + + Reads ``pybamm.citations`` private state: keys registered by key, plus those + registered as raw BibTeX, which PyBaMM leaves unparsed until print time. + Retire this once ``pybamm.Citations`` exposes its registered keys publicly. + """ + import pybamm + from pybamm_model_zoo._citations import parse_bibtex + + keys = set(pybamm.citations._papers_to_cite) + for citation in pybamm.citations._unknown_citations: + keys.update(parse_bibtex(citation)) + return keys + + +def reset_citations() -> None: + """Clear PyBaMM's citation registry so a check can observe one model's own. + + Retire alongside :func:`cited_keys`. + """ + import pybamm + + pybamm.citations._reset() diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py new file mode 100644 index 0000000000..52a9e78db3 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py @@ -0,0 +1,231 @@ +"""Rendering the committed model zoo docs pages and status badges. + +The files are generated and committed rather than built inside Sphinx, which +keeps the ``-W`` docs build deterministic and offline. Because the renderers live +here rather than in the script, the ``docs`` contract check can compare a page +against what it *should* contain instead of merely asserting it exists. + +``scripts/generate.py`` is the command-line wrapper around this module. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from pybamm_model_zoo._paths import BADGES_DIR, DOCS_DIR, STATUS_FILE +from pybamm_model_zoo._registry import ModelEntry + +MODELS_DIR = DOCS_DIR / "models" +GENERATED_BY = ( + "" +) +# Depth from docs/source/model_zoo/models/.md back to the repository root. +README_PREFIX = "../../../../packages/pybamm-model-zoo/src/pybamm_model_zoo" +BADGE_COLORS = {"pass": "brightgreen", "fail": "red", "untested": "lightgrey"} +BADGE_LABEL = "model zoo" + +_INDEX_HEADER = """\ +(model_zoo)= + +{generated} + +# Model zoo + +Community- and partner-contributed PyBaMM models. Each entry is one +self-contained folder with its own maintainer, tests, examples, and +citation; the table below is generated from those folders' manifests. + +Zoo models are reached through the zoo's registry, not the `pybamm` +namespace: + +```python +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load(zoo.list_models()[0])() +solution = pybamm.Simulation(model).solve([0, 3600]) +``` + +Models in the **core** tier are covered by PyBaMM's merge gate. Models in +the **community** tier are tested on every pull request too, but their +results are advisory: a red community model never blocks a PyBaMM merge. +See [contributing a model](contributing.md) to add your own. + +| Model | Tier | Maintainer | PyBaMM | Added | +| --- | --- | --- | --- | --- | +""" + +_COMPATIBILITY_HEADER = """\ + +## Compatibility + +Refreshed weekly by the `model_zoo_status` workflow, which runs each model +against every release its `pybamm_requires` admits, plus `main`. + +| Model | Results | Last passing | +| --- | --- | --- | +""" + +_TOCTREE = """ +```{toctree} +:hidden: +:maxdepth: 1 + +Contributing a model +""" + + +def version_key(version: str) -> tuple[int, list[int]]: + """Sort CalVer releases numerically, and sort anything else (``main``) last.""" + try: + return (0, [int(part) for part in version.split(".")]) + except ValueError: + return (1, []) + + +def read_status(path: Path | None = None) -> dict: + """The committed compatibility results, or an empty set of them.""" + path = path or STATUS_FILE + if not path.is_file(): + return {"models": {}} + return json.loads(path.read_text(encoding="utf-8")) + + +def collect_results(results_dir: Path, generated: str) -> dict: + """Fold one JSON file per matrix cell into the ``status.json`` shape.""" + models: dict[str, dict[str, str]] = {} + for path in sorted(Path(results_dir).glob("*.json")): + record = json.loads(path.read_text(encoding="utf-8")) + models.setdefault(record["model"], {})[record["version"]] = record["result"] + + status: dict = {"generated": generated, "models": {}} + for model, results in sorted(models.items()): + passing = [ + version + for version, result in results.items() + if result == "pass" and version != "main" + ] + status["models"][model] = { + "results": dict( + sorted(results.items(), key=lambda item: version_key(item[0])) + ), + "last_pass": max(passing, key=version_key) if passing else None, + } + return status + + +def badge(record: dict) -> dict: + """The shields.io endpoint payload for one model's status record.""" + results = record.get("results", {}) + failing = sorted( + (version for version, result in results.items() if result != "pass"), + key=version_key, + ) + if not results: + message, color = "untested", BADGE_COLORS["untested"] + elif failing: + message, color = f"failing on {', '.join(failing)}", BADGE_COLORS["fail"] + else: + message = f"passing ({record.get('last_pass') or 'latest'})" + color = BADGE_COLORS["pass"] + return { + "schemaVersion": 1, + "label": BADGE_LABEL, + "message": message, + "color": color, + } + + +def maintainer_links(entry: ModelEntry) -> str: + """Maintainers, linked to their profiles. + + Team handles are left as plain text: GitHub team pages are not reachable + anonymously, so linking one would be a dead link to most readers. + """ + return ", ".join( + f"@{maintainer.github}" + if "/" in maintainer.github + else f"[@{maintainer.github}](https://github.com/{maintainer.github})" + for maintainer in entry.maintainers + ) + + +def status_cell(record: dict) -> str: + results = record.get("results", {}) + if not results: + return "not yet run" + return ", ".join( + f"`{version}`: {result}" + for version, result in sorted( + results.items(), key=lambda item: version_key(item[0]) + ) + ) + + +def index_page(entries: list[ModelEntry], status: dict) -> str: + """The model zoo landing page: the model table and the compatibility table.""" + records = status.get("models", {}) + rows = [ + f"| [{entry.title}](models/{entry.slug}.md) | {entry.tier} | " + f"{maintainer_links(entry)} | `{entry.pybamm_requires}` | {entry.added} |" + for entry in entries + ] + status_rows = [ + f"| {entry.slug} | {status_cell(records.get(entry.slug, {}))} | " + f"{records.get(entry.slug, {}).get('last_pass') or '—'} |" + for entry in entries + ] + toctree = ( + _TOCTREE + "".join(f"models/{entry.slug}\n" for entry in entries) + "```\n" + ) + return ( + _INDEX_HEADER.format(generated=GENERATED_BY) + + "\n".join(rows) + + "\n" + + _COMPATIBILITY_HEADER + + "\n".join(status_rows) + + "\n" + + toctree + ) + + +def model_page(entry: ModelEntry) -> str: + """A per-model page that includes the model's own README verbatim.""" + return ( + f"(model-zoo-{entry.slug})=\n\n" + f"{GENERATED_BY}\n\n" + f"```{{include}} {README_PREFIX}/{entry.slug}/README.md\n" + f"```\n" + ) + + +def pages_for(entry: ModelEntry, status: dict | None = None) -> dict[Path, str]: + """The generated files owned by one model, mapped to their intended content.""" + records = (status if status is not None else read_status()).get("models", {}) + return { + MODELS_DIR / f"{entry.slug}.md": model_page(entry), + BADGES_DIR / f"{entry.slug}.json": json.dumps( + badge(records.get(entry.slug, {})), indent=2 + ) + + "\n", + } + + +def all_files(entries: list[ModelEntry], status: dict | None = None) -> dict[Path, str]: + """Every file this module owns, mapped to its intended content.""" + status = status if status is not None else read_status() + files = {DOCS_DIR / "index.md": index_page(entries, status)} + for entry in entries: + files.update(pages_for(entry, status)) + return files + + +def stale(files: dict[Path, str]) -> list[Path]: + """Generated files left behind by a model that no longer exists.""" + return [ + path + for path in sorted([*MODELS_DIR.glob("*.md"), *BADGES_DIR.glob("*.json")]) + if path not in files + ] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_exceptions.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_exceptions.py new file mode 100644 index 0000000000..a109c41a4a --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_exceptions.py @@ -0,0 +1,15 @@ +"""Exceptions raised by the PyBaMM model zoo.""" + +from __future__ import annotations + + +class ZooError(Exception): + """Base class for every model zoo error.""" + + +class ManifestError(ZooError): + """A ``model.toml`` is missing, unparseable, or fails validation.""" + + +class ModelUnavailableError(ZooError): + """A registered model exists but its code could not be imported.""" diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_paths.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_paths.py new file mode 100644 index 0000000000..aafdbdda45 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_paths.py @@ -0,0 +1,28 @@ +"""Filesystem locations the zoo's tooling shares. + +The ``Path(__file__).parents[N]`` walks that tie this package to the repository +layout live here only, so moving a directory is one edit rather than four. The +repository-relative paths are meaningful only in a checkout; the in-tree contract +checks that use them are scoped accordingly. +""" + +from __future__ import annotations + +from pathlib import Path + +#: The directory holding the in-tree model folders. +PACKAGE_ROOT = Path(__file__).parent +ZOO_ROOT = PACKAGE_ROOT.parents[1] +REPO_ROOT = ZOO_ROOT.parents[1] + +ZOO_PYPROJECT = ZOO_ROOT / "pyproject.toml" +TEMPLATE_ROOT = ZOO_ROOT / "template" +BADGES_DIR = ZOO_ROOT / "badges" +STATUS_FILE = ZOO_ROOT / "status.json" +DOCS_DIR = REPO_ROOT / "docs" / "source" / "model_zoo" +CODEOWNERS = REPO_ROOT / ".github" / "CODEOWNERS" + + +def codeowners_folder(slug: str) -> str: + """The repository-relative folder a model's CODEOWNERS line must cover.""" + return f"/{(PACKAGE_ROOT / slug).relative_to(REPO_ROOT).as_posix()}/" diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py new file mode 100644 index 0000000000..361f1acbf7 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py @@ -0,0 +1,348 @@ +"""Manifest discovery and the model registry. + +Manifests are *parsed*, never imported, so a model whose code is broken or whose +dependencies are missing still appears in the registry and reports a clean failure +at :func:`load` time. + +Validation here is only the structural minimum needed to key an entry, so one +malformed field fails one model instead of taking the whole registry down. +""" + +from __future__ import annotations + +import importlib +import re +import sys +import warnings +from collections.abc import Iterator, Mapping +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +from pybamm_model_zoo._exceptions import ManifestError, ModelUnavailableError +from pybamm_model_zoo._paths import PACKAGE_ROOT + +if sys.version_info >= (3, 11): + import tomllib +else: + import tomli as tomllib + +MANIFEST_NAME = "model.toml" +ENTRY_POINT_GROUP = "pybamm_zoo_models" +TIERS = ("community", "core") +DEFAULT_SOLVE_TIME = 3600.0 +DEFAULT_KEY_VARIABLES = ("Voltage [V]",) +#: A model folder's name, and the registry key it declares. +SLUG_PATTERN = re.compile(r"^[a-z][a-z0-9_]*$") +NAME_PATTERN = re.compile(r"^[A-Za-z][A-Za-z0-9_]*$") + + +def split_class_path(class_path: str) -> tuple[str, str]: + """Split a manifest's ``module.path:AttributeName`` into its two halves. + + Returns empty strings for whichever half is absent, so callers can report a + malformed value rather than having to catch anything. + """ + module_path, separator, attribute = class_path.partition(":") + return (module_path, attribute) if separator else ("", "") + + +@dataclass(frozen=True) +class Maintainer: + """A person responsible for reviewing changes to a model.""" + + name: str + github: str + + +@dataclass(frozen=True) +class Dependencies: + """A model's third-party dependencies, declared as a zoo extra.""" + + extra: str | None = None + packages: tuple[str, ...] = () + + +@dataclass(frozen=True) +class TestSpec: + """How the contract suite should exercise a model.""" + + parameter_set: str | None = None + solve_time: float = DEFAULT_SOLVE_TIME + key_variables: tuple[str, ...] = DEFAULT_KEY_VARIABLES + skip_contract: frozenset[str] = frozenset() + + +@dataclass(frozen=True) +class ModelEntry: + """One registered model, as described by its manifest. + + Attributes are read leniently so that a manifest with a bad field still yields + an entry; ``check_manifest`` is what turns a bad field into a failure. + """ + + slug: str + name: str + path: Path + raw: dict[str, Any] = field(repr=False, default_factory=dict) + external: bool = False + + @property + def manifest_path(self) -> Path: + return self.path / MANIFEST_NAME + + @property + def _model(self) -> dict[str, Any]: + return self.raw.get("model", {}) + + @property + def title(self) -> str: + return self._model.get("title", "") + + @property + def summary(self) -> str: + return self._model.get("summary", "") + + @property + def class_path(self) -> str: + return self._model.get("class", "") + + @property + def tier(self) -> str: + return self._model.get("tier", "community") + + @property + def pybamm_requires(self) -> str: + return self._model.get("pybamm_requires", "") + + @property + def added(self) -> str: + return self._model.get("added", "") + + @property + def license(self) -> str: + return self._model.get("license", "") + + @property + def maintainers(self) -> tuple[Maintainer, ...]: + return tuple( + Maintainer(name=entry.get("name", ""), github=entry.get("github", "")) + for entry in self._model.get("maintainers", []) + if isinstance(entry, dict) + ) + + @property + def citation_key(self) -> str: + return self._model.get("citation", {}).get("key", "") + + @property + def dependencies(self) -> Dependencies: + block = self._model.get("dependencies", {}) + return Dependencies( + extra=block.get("extra") or None, + packages=tuple(block.get("packages", [])), + ) + + @property + def tests(self) -> TestSpec: + block = self._model.get("tests", {}) + key_variables = tuple(block.get("key_variables", DEFAULT_KEY_VARIABLES)) + return TestSpec( + parameter_set=block.get("parameter_set") or None, + solve_time=float(block.get("solve_time", DEFAULT_SOLVE_TIME)), + key_variables=key_variables, + skip_contract=frozenset(block.get("skip_contract", [])), + ) + + @property + def module_path(self) -> str: + return split_class_path(self.class_path)[0] + + @property + def attribute(self) -> str: + return split_class_path(self.class_path)[1] + + def admits(self, version: str) -> bool: + """Whether ``version`` satisfies the manifest's ``pybamm_requires``. + + Raises + ------ + ManifestError + If ``pybamm_requires`` is not a valid version specifier. + """ + # Imported here so the manifest-only paths (the docs generator, the CI + # matrix) keep working in an environment with nothing but the stdlib. + from packaging.specifiers import InvalidSpecifier, SpecifierSet + from packaging.version import Version + + try: + specifier = SpecifierSet(self.pybamm_requires) + except InvalidSpecifier as error: + raise ManifestError( + f"{self.manifest_path}: pybamm_requires " + f"'{self.pybamm_requires}' is not a valid specifier ({error})" + ) from error + # A development checkout reports a .devN version, which a bare specifier + # excludes; the question here is only whether the range is satisfied. + return specifier.contains(Version(version), prereleases=True) + + def load(self) -> type: + """Import and return the model class. + + Raises + ------ + ManifestError + If the manifest does not declare a parseable ``class``. + ModelUnavailableError + If the module cannot be imported or lacks the named attribute. + """ + if not self.module_path or not self.attribute: + raise ManifestError( + f"{self.manifest_path}: 'class' must be 'module.path:AttributeName', " + f"got {self.class_path!r}" + ) + try: + module = importlib.import_module(self.module_path) + except ImportError as error: + extra = self.dependencies.extra + hint = ( + f" It declares the extra '{extra}'; install it with " + f"`uv sync --extra {extra}`." + if extra + else "" + ) + raise ModelUnavailableError( + f"'{self.name}' could not be imported.{hint}" + ) from error + try: + return getattr(module, self.attribute) + except AttributeError as error: + raise ModelUnavailableError( + f"'{self.name}' declares {self.class_path!r} but " + f"'{self.module_path}' has no attribute '{self.attribute}'" + ) from error + + +def read_manifest(path: Path) -> dict[str, Any]: + """Parse a ``model.toml``. + + Raises + ------ + ManifestError + If the file is missing or is not valid TOML. + """ + try: + with path.open("rb") as manifest: + return tomllib.load(manifest) + except FileNotFoundError as error: + raise ManifestError(f"{path}: no such manifest") from error + except tomllib.TOMLDecodeError as error: + raise ManifestError(f"{path}: invalid TOML ({error})") from error + + +def _entry_from_manifest(path: Path, *, external: bool) -> ModelEntry: + raw = read_manifest(path) + model = raw.get("model") + if not isinstance(model, dict): + raise ManifestError(f"{path}: missing a [model] table") + slug = model.get("slug") + name = model.get("name") + for label, value in (("slug", slug), ("name", name)): + if not isinstance(value, str) or not value: + raise ManifestError(f"{path}: [model].{label} must be a non-empty string") + return ModelEntry( + slug=slug, name=name, path=path.parent, raw=raw, external=external + ) + + +class Registry(Mapping[str, ModelEntry]): + """Mapping of model name to :class:`ModelEntry`, discovered from manifests.""" + + def __init__( + self, + paths: list[Path] | None = None, + *, + external_paths: list[Path] | None = None, + ) -> None: + self._entries: dict[str, ModelEntry] = {} + self._by_slug: dict[str, ModelEntry] = {} + for root in [builtin_root()] if paths is None else paths: + self._discover(root, external=False) + for root in ( + external_model_paths() if external_paths is None else external_paths + ): + self._discover(root, external=True) + + def _discover(self, root: Path, *, external: bool) -> None: + for manifest in sorted(Path(root).glob(f"*/{MANIFEST_NAME}")): + entry = _entry_from_manifest(manifest, external=external) + existing = self._entries.get(entry.name) + if existing is None: + self._entries[entry.name] = entry + self._by_slug[entry.slug] = entry + elif external: + # In-tree models win, so a third-party package cannot shadow one. + warnings.warn( + f"ignoring external model '{entry.name}' from {manifest}: " + f"the name is already registered by {existing.manifest_path}", + stacklevel=2, + ) + else: + raise ManifestError( + f"{manifest}: duplicate model name '{entry.name}', already " + f"declared by {existing.manifest_path}" + ) + + def __getitem__(self, name: str) -> ModelEntry: + try: + return self._entries[name] + except KeyError: + known = ", ".join(sorted(self._entries)) or "none" + raise KeyError( + f"'{name}' is not a registered model. Registered: {known}." + ) from None + + def __iter__(self) -> Iterator[str]: + return iter(self._entries) + + def __len__(self) -> int: + return len(self._entries) + + def by_slug(self, slug: str) -> ModelEntry: + """Return the entry whose folder name is ``slug``.""" + try: + return self._by_slug[slug] + except KeyError: + known = ", ".join(sorted(self._by_slug)) or "none" + raise KeyError( + f"no registered model with slug '{slug}'. Known: {known}." + ) from None + + +def builtin_root() -> Path: + """The directory holding the in-tree model folders.""" + return PACKAGE_ROOT + + +def external_model_paths() -> list[Path]: + """Model directories advertised by third-party packages via the entry point.""" + paths: list[Path] = [] + for entry_point in _iter_entry_points(): + try: + module = importlib.import_module(entry_point.value) + except ImportError: # pragma: no cover - depends on the environment + warnings.warn( + f"could not import '{entry_point.value}' advertised by the " + f"'{ENTRY_POINT_GROUP}' entry point '{entry_point.name}'", + stacklevel=2, + ) + continue + for location in module.__path__: + paths.append(Path(location)) + return paths + + +def _iter_entry_points(): + from importlib.metadata import entry_points + + return entry_points(group=ENTRY_POINT_GROUP) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py new file mode 100644 index 0000000000..28ad19a584 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py @@ -0,0 +1,136 @@ +"""Rendering the new-model template. + +Shared by ``scripts/new_model.py`` and ``tests/test_template.py``, so the +skeleton a contributor gets is exactly the one CI proves compliant. + +Placeholders use :class:`string.Template`'s ``$name`` syntax rather than braces, +because the rendered files include BibTeX and MyST, both of which use braces +themselves. +""" + +from __future__ import annotations + +import datetime +import re +from importlib.metadata import PackageNotFoundError, version +from pathlib import Path +from string import Template + +from pybamm_model_zoo._exceptions import ZooError +from pybamm_model_zoo._paths import TEMPLATE_ROOT, codeowners_folder +from pybamm_model_zoo._registry import NAME_PATTERN, SLUG_PATTERN + +TEMPLATE_SUFFIX = ".in" +#: Matches what ``string.Template`` would substitute, for asserting none is left. +PLACEHOLDER_PATTERN = Template.pattern + + +def template_root() -> Path: + """The directory holding the token-substituted skeleton.""" + if not TEMPLATE_ROOT.is_dir(): + raise ZooError( + f"{TEMPLATE_ROOT}: no template directory. The template ships with the " + f"repository, so render it from a checkout rather than an installed " + f"wheel." + ) + return TEMPLATE_ROOT + + +def default_pybamm_requires() -> str: + """A floor of the installed PyBaMM's major version — what it was written for. + + Reads the distribution metadata rather than importing PyBaMM, so scaffolding a + model does not pay for an import it has no other use for. + """ + from packaging.version import Version + + try: + installed = version("pybamm") + except PackageNotFoundError as error: + raise ZooError( + "pybamm is not installed, so the template cannot record the version " + "this model was written against. Install it, or pass " + "--pybamm-requires explicitly." + ) from error + return f">={Version(installed).major}" + + +def citation_key_for(author: str, year: int) -> str: + """A BibTeX key from an author's surname and a year, e.g. ``Author2026``.""" + words = author.split() + surname = re.sub(r"[^A-Za-z]", "", words[-1]) if words else "Model" + return f"{surname.capitalize()}{year}" + + +def tokens( + *, + slug: str, + name: str, + author: str, + github: str, + tier: str = "community", + year: int | None = None, + added: str | None = None, + pybamm_requires: str | None = None, + license: str = "BSD-3-Clause", +) -> dict[str, str]: + """Build the substitution map, validating the contributor's inputs.""" + if not SLUG_PATTERN.match(slug): + raise ZooError(f"slug '{slug}' must be lower_snake_case, e.g. 'my_new_model'") + if not NAME_PATTERN.match(name): + raise ZooError( + f"name '{name}' must be a valid Python identifier, e.g. 'MyModel'" + ) + today = datetime.date.today() + year = year if year is not None else today.year + return { + "slug": slug, + "ModelName": name, + "Author": author, + "github": github.lstrip("@"), + "Year": str(year), + "CitationKey": citation_key_for(author, year), + "extra": f"zoo-{slug.replace('_', '-')}", + "tier": tier, + "added": added or today.isoformat(), + "pybamm_requires": pybamm_requires or default_pybamm_requires(), + "license": license, + } + + +def substitute(text: str, values: dict[str, str]) -> str: + """Fill in every ``$placeholder``, refusing to leave one unresolved.""" + try: + return Template(text).substitute(values) + except (KeyError, ValueError) as error: + raise ZooError(f"could not render template: {error}") from error + + +def planned(destination: Path, values: dict[str, str]) -> dict[Path, Path]: + """Map each template file to the path it renders to under ``destination``.""" + root = template_root() + return { + source: Path(destination) + / substitute(source.relative_to(root).with_suffix("").as_posix(), values) + for source in sorted(root.rglob(f"*{TEMPLATE_SUFFIX}")) + } + + +def render(destination: Path, values: dict[str, str]) -> list[Path]: + """Render the template into ``destination``, returning the files written.""" + destination = Path(destination) + if destination.exists() and any(destination.iterdir()): + raise ZooError(f"{destination}: already exists and is not empty") + written = [] + for source, target in planned(destination, values).items(): + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text( + substitute(source.read_text(encoding="utf-8"), values), encoding="utf-8" + ) + written.append(target) + return written + + +def codeowners_line(slug: str, github: str) -> str: + """The ``.github/CODEOWNERS`` line that makes a contributor their own reviewer.""" + return f"{codeowners_folder(slug)} @{github.lstrip('@')}" diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib new file mode 100644 index 0000000000..ba398e93d1 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib @@ -0,0 +1,19 @@ +@software{PyBaMMModelZoo2026, + title = {{PyBaMM model zoo: Single Particle Model with a lumped series resistance}}, + author = {{The PyBaMM Team}}, + year = {2026}, + url = {https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo}, + note = {Reference entry of the PyBaMM model zoo}, +} + +@article{Marquis2019, + title = {{An asymptotic derivation of a single particle model with electrolyte}}, + author = {Marquis, Scott G. and Sulzer, Valentin and Timms, Robert and Please, Colin P. and Chapman, S. Jon}, + journal = {Journal of The Electrochemical Society}, + volume = {166}, + number = {15}, + pages = {A3693--A3706}, + year = {2019}, + publisher = {The Electrochemical Society}, + doi = {10.1149/2.0341915jes}, +} diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md new file mode 100644 index 0000000000..cf7fee222b --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md @@ -0,0 +1,59 @@ +# SPMSeriesResistance + +![status](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/pybamm-team/PyBaMM/main/packages/pybamm-model-zoo/badges/spm_series_resistance.json) + +## Summary + +The Single Particle Model with a lumped ohmic series resistance: the terminal +voltage is the SPM voltage minus `I R`, where `R` is a new parameter, +`"Series resistance [Ohm]"`. It stands in for everything outside the +electrochemistry — tabs, welds, busbars, cabling — when a measured cell shows a +constant offset that the electrochemical model alone cannot account for. Prefer +it over post-processing the SPM voltage when the drop should also move the +voltage cut-offs, the reported power, and the ECM resistance, all of which the +model derives from the shifted voltage. + +This is the model zoo's **reference entry**: it is deliberately the smallest thing +that is still a real model. Copy this folder as the starting point for your own. + +## Usage + +```python +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load("SPMSeriesResistance")() +parameter_values = model.default_parameter_values +parameter_values["Series resistance [Ohm]"] = 0.05 + +simulation = pybamm.Simulation(model, parameter_values=parameter_values) +solution = simulation.solve([0, 1800]) +print(solution["Voltage [V]"](900)) +``` + +## Validation + +* At `R = 0` the model reproduces `pybamm.lithium_ion.SPM` voltage to + `rtol=1e-6` over a 1800 s 1C discharge. +* At `R > 0` under constant current, the voltage offset from the `R = 0` solution + equals `I R` to `rtol=1e-5` — the residual is interpolation error between two + independently adaptive solves, not a physical difference. +* Both checks run in `tests/test_spm_series_resistance.py`. + +Not validated: any operating mode where the external circuit reads the terminal +voltage back, since the drop is applied after the circuit submodel has been +built. Under power or voltage control the *internal* voltage is what the circuit +holds, and the `"voltage as a state"` option raises `pybamm.OptionError` for the +same reason. Thermal coupling of the `I^2 R` loss is not included: the resistance +is outside the cell in this model, so its heat is not fed to the thermal +submodel. + +## Citation + +See `CITATION.bib`. Cite `PyBaMMModelZoo2026` for this entry and +`Marquis2019` for the underlying SPM; both are registered automatically, so +`pybamm.print_citations()` lists them after you use the model. + +## Maintainer + +The PyBaMM Team (@pybamm-team/maintainers) — tier: core diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py new file mode 100644 index 0000000000..39db89855d --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py @@ -0,0 +1,3 @@ +from pybamm_model_zoo.spm_series_resistance.model import SPMSeriesResistance + +__all__ = ["SPMSeriesResistance"] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py new file mode 100644 index 0000000000..51cc35c47f --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py @@ -0,0 +1,27 @@ +"""Compare the SPM with and without a lumped series resistance.""" + +import numpy as np + +import pybamm +import pybamm_model_zoo as zoo + + +def solve(series_resistance): + model = zoo.load("SPMSeriesResistance")() + parameter_values = model.default_parameter_values + parameter_values["Series resistance [Ohm]"] = series_resistance + simulation = pybamm.Simulation(model, parameter_values=parameter_values) + return simulation.solve([0, 1800]) + + +solutions = {resistance: solve(resistance) for resistance in (0.0, 0.05)} +times = np.linspace(0, 1800, 7) + +print(" t [s] V(R=0) [V] V(R=0.05) [V] drop [V]") +for time in times: + without = solutions[0.0]["Voltage [V]"](time) + with_resistance = solutions[0.05]["Voltage [V]"](time) + print( + f"{time:7.0f} {without:10.5f} {with_resistance:13.5f} " + f"{without - with_resistance:8.5f}" + ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py new file mode 100644 index 0000000000..eca6e4f2bb --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py @@ -0,0 +1,69 @@ +"""Single Particle Model with a lumped ohmic series resistance.""" + +from __future__ import annotations + +import pybamm +import pybamm_model_zoo +from pybamm_model_zoo import _compat + +SLUG = "spm_series_resistance" +DEFAULT_SERIES_RESISTANCE = 0.01 + + +class SPMSeriesResistance(pybamm.lithium_ion.SPM): + """SPM whose terminal voltage carries a lumped ohmic drop, ``V - I R``. + + The resistance ``R`` stands in for everything outside the electrochemistry: + tabs, welds, busbars, and cabling. It enters as a new parameter, + ``"Series resistance [Ohm]"``, and is applied to the terminal voltage before + the base class derives the cut-off events, power, and battery voltage from + it, so those all see the drop too. + + Parameters + ---------- + options : dict, optional + Model options, as for :class:`pybamm.lithium_ion.SPM`. The + ``"voltage as a state"`` option is not supported. + name : str, optional + The model name. + build : bool, optional + Whether to build the model on instantiation. + + Examples + -------- + >>> import pybamm_model_zoo as zoo + >>> model = zoo.load("SPMSeriesResistance")() + >>> "Series resistance overpotential [V]" in model.variables + True + """ + + def __init__( + self, + options: dict | None = None, + name: str = "Single Particle Model with series resistance", + build: bool = True, + ) -> None: + super().__init__( + options=_compat.spm_default_options(options), name=name, build=build + ) + pybamm_model_zoo.register_citation(SLUG) + + def set_voltage_variables(self) -> None: + if self.options["voltage as a state"] == "true": + raise pybamm.OptionError( + "SPMSeriesResistance does not support 'voltage as a state': the " + "algebraic constraint would pin the state to the voltage before " + "the series drop is applied." + ) + resistance = pybamm.Parameter("Series resistance [Ohm]") + overpotential = -self.variables["Current [A]"] * resistance + for key in ("Voltage [V]", "Terminal voltage [V]"): + self.variables[key] = self.variables[key] + overpotential + self.variables["Series resistance overpotential [V]"] = overpotential + super().set_voltage_variables() + + @property + def default_parameter_values(self) -> pybamm.ParameterValues: + values = super().default_parameter_values + values.update({"Series resistance [Ohm]": DEFAULT_SERIES_RESISTANCE}) + return values diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml new file mode 100644 index 0000000000..0ef45ff4ba --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml @@ -0,0 +1,22 @@ +[model] +slug = "spm_series_resistance" +name = "SPMSeriesResistance" +title = "Single Particle Model with a lumped series resistance" +summary = "SPM with an ohmic V = V_SPM - I*R terminal drop, for tab, weld, and cable resistance." +class = "pybamm_model_zoo.spm_series_resistance:SPMSeriesResistance" +tier = "core" +pybamm_requires = ">=26.0" +added = "2026-08-20" +license = "BSD-3-Clause" + +[[model.maintainers]] +name = "The PyBaMM Team" +github = "pybamm-team/maintainers" + +[model.citation] +key = "PyBaMMModelZoo2026" + +[model.tests] +solve_time = 3600 +key_variables = ["Voltage [V]", "Battery voltage [V]"] +skip_contract = [] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py new file mode 100644 index 0000000000..4f078345ab --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py @@ -0,0 +1,64 @@ +"""Physics tests for the reference model zoo entry. + +These are the contributor's own tests (the zoo's "Layer B"): the contract suite +already checks that the model imports, is well posed, builds, and solves, so +these pin *physical* results instead. +""" + +import numpy as np +import pytest + +import pybamm +import pybamm_model_zoo as zoo + +SOLVE_TIME = 1800 +SERIES_RESISTANCE = 0.05 + + +def solve(series_resistance): + model = zoo.load("SPMSeriesResistance")() + parameter_values = model.default_parameter_values + parameter_values["Series resistance [Ohm]"] = series_resistance + simulation = pybamm.Simulation(model, parameter_values=parameter_values) + return simulation.solve([0, SOLVE_TIME]) + + +class TestSPMSeriesResistance: + def test_default_parameter_values_carry_the_resistance(self): + model = zoo.load("SPMSeriesResistance")() + assert "Series resistance [Ohm]" in model.default_parameter_values + + def test_reduces_to_spm_at_zero_resistance(self): + core = pybamm.lithium_ion.SPM() + reference = pybamm.Simulation(core).solve([0, SOLVE_TIME]) + solution = solve(0.0) + times = np.linspace(0, SOLVE_TIME, 50) + np.testing.assert_allclose( + solution["Voltage [V]"](times), + reference["Voltage [V]"](times), + rtol=1e-6, + ) + + def test_voltage_offset_equals_current_times_resistance(self): + without = solve(0.0) + with_resistance = solve(SERIES_RESISTANCE) + times = np.linspace(0, SOLVE_TIME, 50) + current = without["Current [A]"](times) + np.testing.assert_allclose( + without["Voltage [V]"](times) - with_resistance["Voltage [V]"](times), + current * SERIES_RESISTANCE, + rtol=1e-5, + ) + + def test_overpotential_variable_matches_the_drop(self): + solution = solve(SERIES_RESISTANCE) + times = np.linspace(0, SOLVE_TIME, 50) + np.testing.assert_allclose( + solution["Series resistance overpotential [V]"](times), + -solution["Current [A]"](times) * SERIES_RESISTANCE, + rtol=1e-12, + ) + + def test_voltage_as_a_state_is_rejected(self): + with pytest.raises(pybamm.OptionError, match=r"voltage as a state"): + zoo.load("SPMSeriesResistance")({"voltage as a state": "true"}) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/__init__.py new file mode 100644 index 0000000000..4546e45656 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/__init__.py @@ -0,0 +1,12 @@ +"""Reusable test helpers for model zoos. + +The contract checks are shipped rather than kept in the test tree so that a +third-party package advertising itself through the ``pybamm_zoo_models`` entry +point can hold its own models to the same standard in its own CI. +""" + +from __future__ import annotations + +from pybamm_model_zoo.testing import contract + +__all__ = ["contract"] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py new file mode 100644 index 0000000000..ede056095e --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py @@ -0,0 +1,428 @@ +"""The contract every model zoo entry must satisfy. + +:data:`CHECKS` is the single definition of that contract: the test suite, the +manifest's ``skip_contract`` validation, and the documentation table all derive +from it, so adding a check is one edit rather than four. + +Every check carries a scope, which is what lets a third-party collection be held +to the portable ``MODEL`` rules alone. Nothing here imports ``pytest``, so an +external runner can drive them. +""" + +from __future__ import annotations + +import re +from collections.abc import Callable +from dataclasses import dataclass +from importlib.metadata import PackageNotFoundError, version +from typing import Any + +import numpy as np +from packaging.requirements import Requirement +from packaging.version import Version + +import pybamm +from pybamm_model_zoo import _compat, _paths +from pybamm_model_zoo._citations import CITATION_FILE, read_citations +from pybamm_model_zoo._exceptions import ManifestError +from pybamm_model_zoo._registry import ( + NAME_PATTERN, + SLUG_PATTERN, + TIERS, + ModelEntry, + read_manifest, + split_class_path, +) + +#: A portable rule any zoo model must satisfy, wherever it lives. +MODEL = "model" +#: How an in-tree model is wired into this package. +PACKAGING = "packaging" +#: This repository's own hygiene — a documentation page, an owner. +REPO = "repo" + +#: Keys a ``[model]`` table may carry. Anything else is a typo. +MODEL_KEYS = frozenset( + { + "slug", + "name", + "title", + "summary", + "class", + "tier", + "pybamm_requires", + "added", + "license", + "maintainers", + "citation", + "dependencies", + "tests", + } +) +#: README headings the docs pages and the contributor guide both rely on. +REQUIRED_README_SECTIONS = ("Summary", "Usage", "Validation", "Citation") +_DATE_PATTERN = re.compile(r"^\d{4}-\d{2}-\d{2}$") + + +@dataclass(frozen=True) +class Check: + """One contract check, and what it takes to run one. + + Attributes + ---------- + name : str + The name a manifest waives it by, and the test id it appears under. + run : callable + Takes the entry, raises on failure. + scope : str + ``MODEL``, ``PACKAGING``, or ``REPO`` — see the module docstring. + needs_model : bool + Whether it imports the model, so it must be skipped when the model's + declared extra is not installed. + waivable : bool + Whether a manifest's ``skip_contract`` may waive it. + """ + + name: str + run: Callable[[ModelEntry], None] + scope: str = MODEL + needs_model: bool = False + waivable: bool = True + + +#: The contract, in the order it is worth reading failures in. +CHECKS: dict[str, Check] = {} + + +def _register(name: str, **attributes: Any) -> Callable[[Callable], Callable]: + def decorate(function: Callable[[ModelEntry], None]): + CHECKS[name] = Check(name=name, run=function, **attributes) + return function + + return decorate + + +def checks_in_scope(*scopes: str) -> list[Check]: + """Every check belonging to one of ``scopes``, in registration order.""" + return [check for check in CHECKS.values() if check.scope in scopes] + + +@_register("manifest", waivable=False) +def check_manifest(entry: ModelEntry) -> None: + """Every field of the manifest is present, well formed, and consistent.""" + where = entry.manifest_path + model = entry.raw.get("model", {}) + + if unknown := sorted(set(model) - MODEL_KEYS): + raise AssertionError(f"{where}: unknown [model] key(s) {unknown}") + + assert SLUG_PATTERN.match(entry.slug), ( + f"{where}: slug '{entry.slug}' must be lower_snake_case" + ) + assert entry.slug == entry.path.name, ( + f"{where}: slug '{entry.slug}' must equal the folder name '{entry.path.name}'" + ) + assert NAME_PATTERN.match(entry.name), ( + f"{where}: name '{entry.name}' must be a valid Python identifier" + ) + for label, value in (("title", entry.title), ("summary", entry.summary)): + assert value.strip(), f"{where}: {label} must be a non-empty string" + + module_path, attribute = split_class_path(entry.class_path) + assert module_path and attribute, ( + f"{where}: class must be 'module.path:AttributeName', got {entry.class_path!r}" + ) + + assert entry.tier in TIERS, ( + f"{where}: tier must be one of {list(TIERS)}, got '{entry.tier}'" + ) + assert _DATE_PATTERN.match(entry.added), ( + f"{where}: added must be an ISO date (YYYY-MM-DD), got '{entry.added}'" + ) + assert entry.license.strip(), f"{where}: license must be an SPDX identifier" + + assert entry.maintainers, f"{where}: at least one [[model.maintainers]] is required" + for maintainer in entry.maintainers: + assert maintainer.name.strip(), f"{where}: a maintainer is missing a name" + assert maintainer.github.strip(), ( + f"{where}: maintainer '{maintainer.name}' is missing a github handle" + ) + + assert entry.citation_key.strip(), f"{where}: [model.citation] key is required" + + check_pybamm_requires(entry) + _check_tests_block(entry) + _check_dependency_declaration(entry) + + +def check_pybamm_requires(entry: ModelEntry) -> None: + """The declared PyBaMM range is a valid specifier, satisfied by this install. + + Applicability is a *selection* decision elsewhere — the weekly compatibility + matrix only pairs a model with versions its range admits — so reaching this + check with an unsatisfied range means the manifest claims a PyBaMM the tests + are not running against, which is a manifest error worth failing loudly. + """ + where = entry.manifest_path + assert entry.pybamm_requires.strip(), ( + f"{where}: pybamm_requires is required, e.g. '>=26.8'" + ) + try: + satisfied = entry.admits(pybamm.__version__) + except ManifestError as error: + raise AssertionError(str(error)) from error + assert satisfied, ( + f"{where}: pybamm_requires '{entry.pybamm_requires}' is not satisfied by " + f"the installed PyBaMM {pybamm.__version__}" + ) + + +def _check_tests_block(entry: ModelEntry) -> None: + where = entry.manifest_path + tests = entry.tests + assert tests.solve_time > 0, f"{where}: [model.tests] solve_time must be positive" + assert tests.key_variables, ( + f"{where}: [model.tests] key_variables must name at least one variable" + ) + if unknown := sorted(tests.skip_contract - set(CHECKS)): + raise AssertionError( + f"{where}: skip_contract names unknown check(s) {unknown}; " + f"valid checks are {list(CHECKS)}" + ) + if unwaivable := sorted( + name for name in tests.skip_contract if not CHECKS[name].waivable + ): + raise AssertionError(f"{where}: check(s) {unwaivable} cannot be waived") + if tests.parameter_set: + assert tests.parameter_set in pybamm.parameter_sets, ( + f"{where}: parameter_set '{tests.parameter_set}' is not a registered " + f"PyBaMM parameter set" + ) + + +def _check_dependency_declaration(entry: ModelEntry) -> None: + """The manifest's own dependency block is coherent.""" + where = entry.manifest_path + dependencies = entry.dependencies + if dependencies.packages: + assert dependencies.extra, ( + f"{where}: [model.dependencies] declares packages but no extra; " + f"third-party dependencies must be installable as a zoo extra" + ) + if not dependencies.extra: + return + assert dependencies.extra == expected_extra(entry.slug), ( + f"{where}: extra must be named '{expected_extra(entry.slug)}', " + f"got '{dependencies.extra}'" + ) + for requirement in dependencies.packages: + Requirement(requirement) # raises InvalidRequirement on a malformed pin + + +def expected_extra(slug: str) -> str: + """The name of the zoo extra a model's dependencies must be installable by.""" + return f"zoo-{slug.replace('_', '-')}" + + +@_register("layout") +def check_layout(entry: ModelEntry) -> None: + """Required files are present and the README carries the required sections.""" + for name in ("README.md", CITATION_FILE): + assert (entry.path / name).is_file(), f"{entry.path / name}: missing" + for name in ("examples", "tests"): + directory = entry.path / name + assert directory.is_dir(), f"{directory}: missing" + assert any(directory.glob("**/*.py")), f"{directory}: contains no Python files" + + readme = (entry.path / "README.md").read_text(encoding="utf-8") + headings = set(re.findall(r"^#+\s*(.+?)\s*$", readme, flags=re.MULTILINE)) + missing = [ + section for section in REQUIRED_README_SECTIONS if section not in headings + ] + assert not missing, ( + f"{entry.path / 'README.md'}: missing section heading(s) {missing}" + ) + + +@_register("import", needs_model=True) +def check_import(entry: ModelEntry) -> type[pybamm.BaseModel]: + """The declared class imports and is a PyBaMM model.""" + model_class = entry.load() + assert isinstance(model_class, type) and issubclass( + model_class, pybamm.BaseModel + ), f"{entry.name}: {entry.class_path} is not a pybamm.BaseModel subclass" + return model_class + + +@_register("citation", needs_model=True) +def check_citation(entry: ModelEntry) -> None: + """The manifest's key resolves, and instantiating the model credits it.""" + citations = read_citations(entry.path) + assert entry.citation_key in citations, ( + f"{entry.path / CITATION_FILE}: no entry for '{entry.citation_key}'; " + f"found {sorted(citations)}" + ) + _compat.reset_citations() + instantiate(entry) + assert entry.citation_key in _compat.cited_keys(), ( + f"{entry.name}: instantiating the model does not register " + f"'{entry.citation_key}', so pybamm.print_citations() will not credit its " + f"author. Call pybamm_model_zoo.register_citation('{entry.slug}') in " + f"__init__." + ) + + +@_register("well_posed", needs_model=True) +def check_well_posed(entry: ModelEntry) -> None: + """The model's equations form a well-posed system.""" + instantiate(entry).check_well_posedness() + + +@_register("build", needs_model=True) +def check_build(entry: ModelEntry) -> None: + """Parameters, geometry, mesh, and discretisation all process the model.""" + _simulation_for(entry).build() + + +@_register("solve", needs_model=True) +def check_solve(entry: ModelEntry) -> pybamm.Solution: + """The model solves, and its key variables are finite throughout.""" + solution = _simulation_for(entry).solve([0, entry.tests.solve_time]) + for name in entry.tests.key_variables: + assert name in solution.all_models[0].variables, ( + f"{entry.name}: key variable '{name}' is not a model variable" + ) + # Read through the interpolating call interface, not the raw arrays. + values = np.asarray(solution[name](solution.t)) + assert values.size, f"{entry.name}: '{name}' returned no values" + assert np.all(np.isfinite(values)), ( + f"{entry.name}: '{name}' is not finite everywhere" + ) + return solution + + +@_register("packaging", scope=PACKAGING) +def check_packaging(entry: ModelEntry) -> None: + """An in-tree model is importable as part of the zoo, with its extra declared.""" + assert (entry.path / "__init__.py").is_file(), ( + f"{entry.path / '__init__.py'}: missing, so the folder is not importable" + ) + expected_module = f"pybamm_model_zoo.{entry.slug}" + module_path = entry.module_path + assert module_path == expected_module or module_path.startswith( + f"{expected_module}." + ), ( + f"{entry.manifest_path}: class must live under '{expected_module}', " + f"got '{module_path}'" + ) + _check_extra_is_declared(entry) + + +def _check_extra_is_declared(entry: ModelEntry) -> None: + """The zoo's pyproject offers the extra the manifest declares, via ``zoo-all``.""" + extra = entry.dependencies.extra + if not extra: + return + pyproject = _paths.ZOO_PYPROJECT + extras = ( + read_manifest(pyproject).get("project", {}).get("optional-dependencies", {}) + ) + assert extra in extras, ( + f"{pyproject}: no '{extra}' extra, but {entry.manifest_path} declares one" + ) + declared = {Requirement(item).name for item in extras[extra]} + missing = sorted( + {Requirement(item).name for item in entry.dependencies.packages} - declared + ) + assert not missing, ( + f"{pyproject}: extra '{extra}' is missing {missing}, declared by " + f"{entry.manifest_path}" + ) + aggregated: set[str] = set() + for item in extras.get("zoo-all", []): + requirement = Requirement(item) + if requirement.name == "pybamm-model-zoo": + aggregated |= requirement.extras + assert extra in aggregated, ( + f"{pyproject}: the 'zoo-all' extra must include 'pybamm-model-zoo[{extra}]', " + f"or `uv sync --extra zoo-all` will not install {entry.slug}" + ) + + +@_register("docs", scope=REPO) +def check_docs(entry: ModelEntry) -> None: + """The generated docs page for the model is present and current.""" + from pybamm_model_zoo import _docs + + for path, content in _docs.pages_for(entry).items(): + assert path.is_file(), ( + f"{path}: missing. Run `nox -s zoo-docs` to regenerate the model zoo docs." + ) + assert path.read_text(encoding="utf-8") == content, ( + f"{path}: out of date. Run `nox -s zoo-docs` to regenerate the model " + f"zoo docs." + ) + assert entry.slug in (_paths.DOCS_DIR / "index.md").read_text(encoding="utf-8"), ( + f"{_paths.DOCS_DIR / 'index.md'}: does not list '{entry.slug}'. Run " + f"`nox -s zoo-docs` to regenerate the model zoo docs." + ) + + +@_register("codeowners", scope=REPO) +def check_codeowners(entry: ModelEntry) -> None: + """``.github/CODEOWNERS`` names an owner for the model's folder.""" + folder = _paths.codeowners_folder(entry.slug) + for line in _paths.CODEOWNERS.read_text(encoding="utf-8").splitlines(): + stripped = line.strip() + if stripped.startswith(folder) and "@" in stripped: + return + handle = entry.maintainers[0].github if entry.maintainers else "handle" + raise AssertionError( + f"{_paths.CODEOWNERS}: no owner for '{folder}'. Add a line such as " + f"'{folder} @{handle}' so changes to the model request its maintainer " + f"for review." + ) + + +def instantiate(entry: ModelEntry) -> pybamm.BaseModel: + """Build a fresh instance of a registered model.""" + return check_import(entry)() + + +def parameter_values_for( + entry: ModelEntry, model: pybamm.BaseModel +) -> pybamm.ParameterValues: + """The parameter values the contract exercises a model with. + + The manifest's ``tests.parameter_set`` when it names one, otherwise the + model's own defaults — which is where a model adds its extra parameters. + """ + if entry.tests.parameter_set: + return pybamm.ParameterValues(entry.tests.parameter_set) + return model.default_parameter_values + + +def _simulation_for(entry: ModelEntry) -> pybamm.Simulation: + """A simulation over a fresh instance, as a user would set one up. + + Going through ``Simulation`` rather than driving the parameter, mesh, and + discretisation steps by hand keeps the ``build`` check exercising the same + pipeline as the ``solve`` check. + """ + model = instantiate(entry) + return pybamm.Simulation(model, parameter_values=parameter_values_for(entry, model)) + + +def missing_dependencies(entry: ModelEntry) -> list[str]: + """Requirements from the model's extra that are not installed.""" + missing = [] + for item in entry.dependencies.packages: + requirement = Requirement(item) + try: + installed = Version(version(requirement.name)) + except PackageNotFoundError: + missing.append(item) + continue + if not requirement.specifier.contains(installed, prereleases=True): + missing.append(item) + return missing diff --git a/packages/pybamm-model-zoo/template/CITATION.bib.in b/packages/pybamm-model-zoo/template/CITATION.bib.in new file mode 100644 index 0000000000..1b7ccc44d2 --- /dev/null +++ b/packages/pybamm-model-zoo/template/CITATION.bib.in @@ -0,0 +1,6 @@ +@article{${CitationKey}, + title = {{TODO: the title of the work this model implements}}, + author = {${Author}}, + year = {${Year}}, + note = {TODO: replace with the journal, volume, pages, and DOI}, +} diff --git a/packages/pybamm-model-zoo/template/README.md.in b/packages/pybamm-model-zoo/template/README.md.in new file mode 100644 index 0000000000..4e4be0c098 --- /dev/null +++ b/packages/pybamm-model-zoo/template/README.md.in @@ -0,0 +1,35 @@ +# ${ModelName} + +![status](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/pybamm-team/PyBaMM/main/packages/pybamm-model-zoo/badges/${slug}.json) + +## Summary + +TODO: one paragraph. What physics does this add, and when should someone prefer +it over the closest model already in PyBaMM? + +## Usage + +```python +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load("${ModelName}")() +simulation = pybamm.Simulation(model) +solution = simulation.solve([0, 3600]) +print(solution["Voltage [V]"](1800)) +``` + +## Validation + +TODO: what has been checked, against what, and to what tolerance — and state +plainly what has *not* been validated. A model with an honest list of limits is +more useful than one that implies it has none. + +## Citation + +See `CITATION.bib`. Please cite ${CitationKey} when using this model; it is +registered automatically, so `pybamm.print_citations()` credits it after use. + +## Maintainer + +${Author} (@${github}) — tier: ${tier} diff --git a/packages/pybamm-model-zoo/template/__init__.py.in b/packages/pybamm-model-zoo/template/__init__.py.in new file mode 100644 index 0000000000..fc7edcd986 --- /dev/null +++ b/packages/pybamm-model-zoo/template/__init__.py.in @@ -0,0 +1,3 @@ +from pybamm_model_zoo.${slug}.model import ${ModelName} + +__all__ = ["${ModelName}"] diff --git a/packages/pybamm-model-zoo/template/examples/run_${slug}.py.in b/packages/pybamm-model-zoo/template/examples/run_${slug}.py.in new file mode 100644 index 0000000000..100e9925fb --- /dev/null +++ b/packages/pybamm-model-zoo/template/examples/run_${slug}.py.in @@ -0,0 +1,11 @@ +"""Run ${ModelName} and report its terminal voltage.""" + +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load("${ModelName}")() +simulation = pybamm.Simulation(model) +solution = simulation.solve([0, 1800]) + +for time in (0, 900, 1800): + print(f"t = {time:5d} s V = {solution['Voltage [V]'](time):.5f} V") diff --git a/packages/pybamm-model-zoo/template/model.py.in b/packages/pybamm-model-zoo/template/model.py.in new file mode 100644 index 0000000000..b12795bafb --- /dev/null +++ b/packages/pybamm-model-zoo/template/model.py.in @@ -0,0 +1,47 @@ +"""TODO: one line describing ${ModelName}.""" + +from __future__ import annotations + +import pybamm +import pybamm_model_zoo +from pybamm_model_zoo import _compat + +SLUG = "${slug}" + + +class ${ModelName}(pybamm.lithium_ion.SPM): + """TODO: what physics this adds, and when to prefer it. + + As shipped this is plain SPM, so the contract suite passes from the first + commit. Add your physics by overriding the submodel setters you need — see + ``spm_series_resistance`` for a worked example, and the PyBaMM developer + docs for the submodel protocol. + + Parameters + ---------- + options : dict, optional + Model options, as for :class:`pybamm.lithium_ion.SPM`. + name : str, optional + The model name. + build : bool, optional + Whether to build the model on instantiation. + """ + + def __init__( + self, + options: dict | None = None, + name: str = "${ModelName}", + build: bool = True, + ) -> None: + super().__init__( + options=_compat.spm_default_options(options), name=name, build=build + ) + pybamm_model_zoo.register_citation(SLUG) + + # TODO: add your physics. New parameters need a default here so that the + # contract suite can build and solve the model without a bespoke set. + # @property + # def default_parameter_values(self) -> pybamm.ParameterValues: + # values = super().default_parameter_values + # values.update({"My new parameter [units]": 1.0}) + # return values diff --git a/packages/pybamm-model-zoo/template/model.toml.in b/packages/pybamm-model-zoo/template/model.toml.in new file mode 100644 index 0000000000..7ef67d8c93 --- /dev/null +++ b/packages/pybamm-model-zoo/template/model.toml.in @@ -0,0 +1,31 @@ +[model] +slug = "${slug}" +name = "${ModelName}" +title = "TODO: one line naming the physics this model adds" +summary = "TODO: one sentence on what it does and when to prefer it." +class = "pybamm_model_zoo.${slug}:${ModelName}" +tier = "${tier}" +pybamm_requires = "${pybamm_requires}" +added = "${added}" +license = "${license}" + +[[model.maintainers]] +name = "${Author}" +github = "${github}" + +[model.citation] +key = "${CitationKey}" + +# Third-party dependencies go in a `${extra}` extra in the zoo's +# pyproject.toml, listed in `zoo-all`, never in its base dependencies. +# Uncomment and fill in if your model needs any. +# [model.dependencies] +# extra = "${extra}" +# packages = ["scikit-fem>=12.0.2"] + +[model.tests] +# Omit parameter_set to use the model's own default_parameter_values, which is +# where a model with extra parameters should supply their defaults. +solve_time = 3600 +key_variables = ["Voltage [V]"] +skip_contract = [] diff --git a/packages/pybamm-model-zoo/template/tests/test_${slug}.py.in b/packages/pybamm-model-zoo/template/tests/test_${slug}.py.in new file mode 100644 index 0000000000..9a35f52cd1 --- /dev/null +++ b/packages/pybamm-model-zoo/template/tests/test_${slug}.py.in @@ -0,0 +1,32 @@ +"""Physics tests for ${ModelName}. + +The contract suite already checks that this model imports, is well posed, +builds, and solves. These tests are for pinning *physical* results: a known +limit, an analytic solution, a conservation law, or a published figure. + +The test below holds while the model is still plain SPM, and will fail the +moment you change the physics — that failure is your prompt to replace it with +a test that pins whatever your model is supposed to get right. +""" + +import numpy as np + +import pybamm +import pybamm_model_zoo as zoo + +SOLVE_TIME = 1800 + + +class Test${ModelName}: + def test_reduces_to_spm(self): + # TODO: replace with a test of your own physics. + solution = pybamm.Simulation(zoo.load("${ModelName}")()).solve( + [0, SOLVE_TIME] + ) + reference = pybamm.Simulation(pybamm.lithium_ion.SPM()).solve([0, SOLVE_TIME]) + times = np.linspace(0, SOLVE_TIME, 50) + np.testing.assert_allclose( + solution["Voltage [V]"](times), + reference["Voltage [V]"](times), + rtol=1e-6, + ) diff --git a/packages/pybamm-model-zoo/tests/test_contract.py b/packages/pybamm-model-zoo/tests/test_contract.py new file mode 100644 index 0000000000..4e2a274165 --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_contract.py @@ -0,0 +1,77 @@ +"""The contract suite: every registered model, held to every check. + +Contributors write none of this. Adding a model folder with a manifest adds a +column to this matrix automatically, and adding a check to +:data:`pybamm_model_zoo.testing.contract.CHECKS` adds a row. +""" + +import pytest + +import pybamm_model_zoo as zoo +from pybamm_model_zoo import _docs +from pybamm_model_zoo.testing import contract + +# An externally-registered model is held only to the portable rules: it is not +# wired into this package and does not live in this repository. +IN_TREE_SCOPES = (contract.MODEL, contract.PACKAGING, contract.REPO) +EXTERNAL_SCOPES = (contract.MODEL,) + + +def contract_cases(): + return [ + pytest.param( + entry, + check, + id=f"{entry.slug}-{check.name}", + marks=pytest.mark.zoo_model(entry.slug), + ) + for entry in zoo.all_entries() + for check in contract.checks_in_scope( + *(EXTERNAL_SCOPES if entry.external else IN_TREE_SCOPES) + ) + ] + + +@pytest.mark.parametrize(("entry", "check"), contract_cases()) +def test_contract(entry, check): + if check.name in entry.tests.skip_contract: + # A reviewed, per-check escape hatch, visible in the manifest diff. + pytest.skip(f"{entry.slug}: '{check.name}' waived by {entry.manifest_path}") + if check.needs_model and (missing := contract.missing_dependencies(entry)): + pytest.skip( + f"{entry.slug}: extra '{entry.dependencies.extra}' is not installed " + f"(missing {missing})" + ) + check.run(entry) + + +class TestContractItself: + def test_at_least_one_model_is_registered(self): + assert zoo.list_models(), ( + "the registry is empty, so the contract suite would vacuously pass" + ) + + def test_every_check_is_well_formed(self): + assert contract.CHECKS, "the contract is empty" + for name, check in contract.CHECKS.items(): + assert check.name == name + assert check.scope in IN_TREE_SCOPES, f"{name}: unknown scope" + assert check.run.__doc__, ( + f"{name}: needs a docstring saying what it asserts" + ) + + +class TestGeneratedFiles: + """The index page and the absence of leftovers, which no per-model check sees.""" + + def test_docs_and_badges_are_current(self): + files = _docs.all_files(zoo.all_entries()) + outdated = [ + path + for path, content in files.items() + if not path.is_file() or path.read_text(encoding="utf-8") != content + ] + assert not outdated + _docs.stale(files), ( + "out of date, run `nox -s zoo-docs`: " + f"{sorted(str(path) for path in outdated + _docs.stale(files))}" + ) diff --git a/packages/pybamm-model-zoo/tests/test_examples.py b/packages/pybamm-model-zoo/tests/test_examples.py new file mode 100644 index 0000000000..3a73b332b7 --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_examples.py @@ -0,0 +1,30 @@ +"""Run every model's example scripts, mirroring the core package's test_scripts.""" + +import runpy + +import pytest + +import pybamm_model_zoo as zoo +from pybamm_model_zoo.testing import contract + + +def example_scripts(): + return [ + pytest.param( + entry, + script, + id=f"{entry.slug}/{script.name}", + marks=pytest.mark.zoo_model(entry.slug), + ) + for entry in zoo.all_entries() + for script in sorted((entry.path / "examples").glob("**/*.py")) + ] + + +class TestExamples: + @pytest.mark.zoo_examples + @pytest.mark.parametrize(("entry", "script"), example_scripts()) + def test_example_script(self, entry, script): + if missing := contract.missing_dependencies(entry): + pytest.skip(f"{entry.slug}: missing {missing}") + runpy.run_path(str(script)) diff --git a/packages/pybamm-model-zoo/tests/test_registry.py b/packages/pybamm-model-zoo/tests/test_registry.py new file mode 100644 index 0000000000..bf89daf03f --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_registry.py @@ -0,0 +1,148 @@ +"""Unit tests for manifest parsing and the registry itself.""" + +import textwrap +from pathlib import Path + +import pytest + +import pybamm_model_zoo as zoo +from pybamm_model_zoo._citations import parse_bibtex +from pybamm_model_zoo._registry import Registry +from pybamm_model_zoo.testing import contract + +MANIFEST = """ +[model] +slug = "{slug}" +name = "{name}" +title = "A title" +summary = "A summary." +class = "pybamm_model_zoo.{slug}:{name}" +tier = "community" +pybamm_requires = ">=26.0" +added = "2026-01-01" +license = "BSD-3-Clause" + +[[model.maintainers]] +name = "A. Author" +github = "ahandle" + +[model.citation] +key = "Author2026" +""" + + +def write_model(root: Path, slug: str, name: str, body: str | None = None) -> Path: + folder = root / slug + folder.mkdir(parents=True) + (folder / "model.toml").write_text( + body if body is not None else MANIFEST.format(slug=slug, name=name) + ) + return folder + + +class TestRegistry: + def test_discovers_the_reference_model(self): + assert "SPMSeriesResistance" in zoo.list_models() + entry = zoo.info("SPMSeriesResistance") + assert entry.slug == "spm_series_resistance" + assert entry.tier == "core" + assert entry.maintainers[0].github == "pybamm-team/maintainers" + + def test_defaults_are_applied_for_optional_fields(self, tmp_path): + write_model(tmp_path, "minimal_model", "MinimalModel") + entry = Registry([tmp_path])["MinimalModel"] + assert entry.tier == "community" + assert entry.tests.solve_time == 3600 + assert entry.tests.key_variables == ("Voltage [V]",) + assert entry.dependencies.extra is None + assert not entry.external + + def test_unknown_name_lists_what_is_registered(self, tmp_path): + write_model(tmp_path, "minimal_model", "MinimalModel") + with pytest.raises(KeyError, match=r"MinimalModel"): + Registry([tmp_path])["Nope"] + + def test_by_slug(self, tmp_path): + write_model(tmp_path, "minimal_model", "MinimalModel") + registry = Registry([tmp_path]) + assert registry.by_slug("minimal_model").name == "MinimalModel" + with pytest.raises(KeyError, match=r"minimal_model"): + registry.by_slug("other") + + def test_invalid_toml_is_reported_with_its_path(self, tmp_path): + write_model(tmp_path, "broken_model", "Broken", body="[model\nslug =") + with pytest.raises(zoo.ManifestError, match=r"invalid TOML"): + Registry([tmp_path]) + + def test_missing_model_table_is_reported(self, tmp_path): + write_model(tmp_path, "broken_model", "Broken", body="[other]\nkey = 1\n") + with pytest.raises(zoo.ManifestError, match=r"missing a \[model\] table"): + Registry([tmp_path]) + + def test_duplicate_names_are_rejected(self, tmp_path): + write_model(tmp_path, "one_model", "Same") + write_model(tmp_path, "two_model", "Same") + with pytest.raises(zoo.ManifestError, match=r"duplicate model name"): + Registry([tmp_path]) + + def test_external_models_do_not_shadow_in_tree_ones(self, tmp_path): + in_tree = tmp_path / "in_tree" + external = tmp_path / "external" + write_model(in_tree, "minimal_model", "MinimalModel") + write_model(external, "minimal_model", "MinimalModel") + with pytest.warns(UserWarning, match=r"ignoring external model"): + registry = Registry([in_tree], external_paths=[external]) + assert registry["MinimalModel"].path.parent == in_tree + + def test_external_entries_are_flagged(self, tmp_path): + write_model(tmp_path, "minimal_model", "MinimalModel") + assert Registry([], external_paths=[tmp_path])["MinimalModel"].external + + +class TestLoad: + def test_load_returns_the_class(self): + model_class = zoo.load("SPMSeriesResistance") + assert model_class.__name__ == "SPMSeriesResistance" + + def test_unparseable_class_path(self, tmp_path): + body = MANIFEST.format(slug="minimal_model", name="MinimalModel").replace( + 'class = "pybamm_model_zoo.minimal_model:MinimalModel"', 'class = "nope"' + ) + write_model(tmp_path, "minimal_model", "MinimalModel", body=body) + with pytest.raises(zoo.ManifestError, match=r"module.path:AttributeName"): + Registry([tmp_path])["MinimalModel"].load() + + def test_missing_module_names_the_extra(self, tmp_path): + body = MANIFEST.format( + slug="minimal_model", name="MinimalModel" + ) + textwrap.dedent( + """ + [model.dependencies] + extra = "zoo-minimal-model" + packages = ["not-a-real-package>=1.0"] + """ + ) + write_model(tmp_path, "minimal_model", "MinimalModel", body=body) + entry = Registry([tmp_path])["MinimalModel"] + with pytest.raises(zoo.ModelUnavailableError, match=r"zoo-minimal-model"): + entry.load() + assert contract.missing_dependencies(entry) == ["not-a-real-package>=1.0"] + + +class TestCitationParsing: + def test_parses_multiple_entries(self): + entries = parse_bibtex( + "@article{A2020, title = {{Nested {braces} here}},}\n" + "@misc{B2021, note = {x},}\n" + ) + assert sorted(entries) == ["A2020", "B2021"] + assert entries["A2020"].startswith("@article{A2020") + assert entries["A2020"].endswith("}") + + def test_reference_model_citation_resolves(self): + entry = zoo.info("SPMSeriesResistance") + assert entry.citation_key in zoo.read_citations(entry.path) + + def test_register_citation_rejects_an_unknown_key(self): + with pytest.raises(zoo.ManifestError, match=r"no entry for 'Nope'"): + zoo.register_citation("spm_series_resistance", "Nope") diff --git a/packages/pybamm-model-zoo/tests/test_template.py b/packages/pybamm-model-zoo/tests/test_template.py new file mode 100644 index 0000000000..50c9ab47fc --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_template.py @@ -0,0 +1,117 @@ +"""Render the template and hold the result to the whole portable contract. + +This is what makes "follow the template and CI is green on day one" a tested +claim rather than a hope: if a contract check and the template ever disagree, +this test fails instead of the next contributor's first pull request. The checks +are read from the contract registry, so a new check cannot quietly skip the +template. +""" + +import shutil +import subprocess +import sys + +import pytest + +import pybamm_model_zoo as zoo +from pybamm_model_zoo import _paths, _template +from pybamm_model_zoo.testing import contract + +SLUG = "template_smoke_model" +NAME = "TemplateSmokeModel" +# Everything but the REPO scope: a freshly rendered model has no docs page or +# CODEOWNERS line until it is committed. +TEMPLATE_SCOPES = (contract.MODEL, contract.PACKAGING) + + +@pytest.fixture +def rendered(tmp_path): + """A rendered template, importable as ``pybamm_model_zoo.``.""" + values = _template.tokens( + slug=SLUG, + name=NAME, + author="A. Author", + github="ahandle", + year=2026, + added="2026-01-01", + ) + _template.render(tmp_path / SLUG, values) + # Extending the package's search path is what lets the manifest's in-tree + # `pybamm_model_zoo.` class path resolve from a temporary directory. + zoo.__path__.append(str(tmp_path)) + try: + yield zoo.refresh([tmp_path]).by_slug(SLUG) + finally: + zoo.__path__.remove(str(tmp_path)) + if hasattr(zoo, SLUG): + delattr(zoo, SLUG) + _forget_module(f"{zoo.__name__}.{SLUG}") + zoo.refresh() + + +def _forget_module(prefix): + for name in [ + name for name in sys.modules if name == prefix or name.startswith(f"{prefix}.") + ]: + del sys.modules[name] + + +class TestTemplate: + def test_renders_every_file(self, rendered): + for name in ("model.toml", "README.md", "CITATION.bib", "__init__.py"): + assert (rendered.path / name).is_file() + assert (rendered.path / "examples" / f"run_{SLUG}.py").is_file() + assert (rendered.path / "tests" / f"test_{SLUG}.py").is_file() + + def test_leaves_no_unsubstituted_placeholders(self, rendered): + for path in sorted(rendered.path.rglob("*")): + if path.is_file(): + assert not _template.PLACEHOLDER_PATTERN.search( + path.read_text(encoding="utf-8") + ), f"{path}: unsubstituted template placeholder" + + @pytest.mark.parametrize( + "check", + contract.checks_in_scope(*TEMPLATE_SCOPES), + ids=lambda check: check.name, + ) + def test_contract(self, rendered, check): + check.run(rendered) + + def test_rejects_a_bad_slug(self): + with pytest.raises(zoo.ZooError, match=r"lower_snake_case"): + _template.tokens(slug="MyModel", name="MyModel", author="A", github="a") + + def test_codeowners_line_names_the_contributor(self): + line = _template.codeowners_line(SLUG, "@ahandle") + assert line.endswith(" @ahandle") + assert line.startswith(_paths.codeowners_folder(SLUG)) + + +class TestRenderedStyle: + """A rendered template must also pass the repository's style job. + + The template files are not themselves linted (they carry placeholders and a + `.in` suffix), so without this the skeleton could drift out of Ruff's + formatting and a contributor's first commit would be reformatted under them. + """ + + @pytest.mark.skipif(shutil.which("ruff") is None, reason="ruff is not installed") + @pytest.mark.parametrize( + "command", [("check", "--no-cache"), ("format", "--check", "--no-cache")] + ) + def test_rendered_python_is_style_clean(self, rendered, command): + result = subprocess.run( + [ + "ruff", + *command, + "--config", + str(_paths.REPO_ROOT / "pyproject.toml"), + ".", + ], + cwd=rendered.path, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/pyproject.toml b/pyproject.toml index e079d3da1f..c02641b724 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,6 +10,7 @@ members = ["packages/*"] [tool.uv.sources] pybammsolvers = { workspace = true } +pybamm = { workspace = true } # Prevent uv's ephemeral build venv from being torn down between CMake configure # and scikit-build-core's editable auto-rebuild (which otherwise can't find @@ -93,6 +94,11 @@ ignore = [ "packages/pybamm/tests/*" = ["T20", "S101"] # `exec` is how this module evaluates the Python it generates "packages/pybamm/src/pybamm/expression_tree/operations/evaluate_python.py" = ["S102"] +# The zoo's contract checks are test helpers that happen to ship in src/, so +# `assert` is the right tool there as well as in the test tree. +"packages/pybamm-model-zoo/**/tests/*" = ["T20", "S101"] +"packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/*" = ["S101"] +"packages/pybamm-model-zoo/scripts/*" = ["T20"] "docs/*" = ["T20"] "examples/*" = ["T20"] "**.ipynb" = ["E402", "E703"] @@ -105,7 +111,7 @@ ignore = [ # PyPI dependency) and the committed import ordering reflects that; the solver's # own packages/pybammsolvers/ruff.toml governs its subtree. [tool.ruff.lint.isort] -known-first-party = ["pybamm"] +known-first-party = ["pybamm", "pybamm_model_zoo"] # repo-review runs at the repo root. Carry over the ignore list that lived in # the package pyproject on main, plus the checks that don't apply to a diff --git a/uv.lock b/uv.lock index 3d94417527..30538e3b33 100644 --- a/uv.lock +++ b/uv.lock @@ -23,6 +23,7 @@ resolution-markers = [ [manifest] members = [ "pybamm", + "pybamm-model-zoo", "pybammsolvers", ] @@ -3262,6 +3263,24 @@ docs = [ { name = "sphinxcontrib-bibtex" }, ] +[[package]] +name = "pybamm-model-zoo" +version = "0.1.0" +source = { editable = "packages/pybamm-model-zoo" } +dependencies = [ + { name = "packaging" }, + { name = "pybamm" }, + { name = "tomli", marker = "python_full_version < '3.11'" }, +] + +[package.metadata] +requires-dist = [ + { name = "packaging", specifier = ">=23.0" }, + { name = "pybamm", editable = "packages/pybamm" }, + { name = "tomli", marker = "python_full_version < '3.11'", specifier = ">=2.0.1" }, +] +provides-extras = ["zoo-all"] + [[package]] name = "pybammsolvers" version = "0.9.1" From f8ffddfe9fc13a47df9067ea5c933d86c4d7d299 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 20 Aug 2026 12:12:12 +0100 Subject: [PATCH 2/7] chore: update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c80abe5be..bd67e540ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Features -- Added the model zoo (`packages/pybamm-model-zoo/`), a home for community- and partner-contributed models: one self-contained folder per model, with a declarative `model.toml` manifest as the only boilerplate a contributor writes. The registry, a ten-check contract test suite, the docs pages, the CI routing, and the status badges are all derived from the manifests, which are parsed rather than imported so a broken model reports a clean failure instead of taking the zoo down. Models are either `community` tier (advisory CI) or `core` tier (in the merge gate); nothing in `pybamm` itself changed. ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) +- Added the model zoo (`packages/pybamm-model-zoo/`), a home for community- and partner-contributed models: one self-contained folder per model, with a declarative `model.toml` manifest as the only boilerplate a contributor writes. Models are either `community` tier (advisory CI) or `core` tier (in the merge gate); nothing in `pybamm` itself changed. ([#5727](https://github.com/pybamm-team/PyBaMM/issues/5727)) ## Bug fixes From 75bc4ab893438bda960fdf3f89794e66fa2be7ba Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 20 Aug 2026 12:45:22 +0100 Subject: [PATCH 3/7] fix: hold `pybamm_requires` only to an install that names a release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zoo's manifest contract compared `pybamm_requires` against `pybamm.__version__`, which hatch-vcs derives from `git describe --match "pybamm-v*"`. The CI checkout is shallow — `_nox.yml` defaults to `fetch-depth: 1` — so no tag is in reach, setuptools_scm falls back to guessing `0.0.1.dev1+g`, and `>=26.0` went unsatisfied, taking the merge gate with it. It passed locally only because a full clone has the tags. `names_a_release` recognises that guess (PyBaMM has never published a `0.0` series) and skips the range comparison, so an sdist, a fork, or a shallow clone no longer reports a model as incompatible with the PyBaMM it is actually running against. This is the carve-out `scripts/matrix.py` already makes for its `main` cell, which the contract check was missing. Specifier *validity* stays unconditional — that half never needed git. The zoo CI jobs then pass `fetch_depth: 0` so the version resolves and the comparison is exercised rather than silently skipped, and the weekly status workflow does the same for the `main` cell, which installs PyBaMM from the checkout. --- .github/workflows/_nox.yml | 5 ++- .github/workflows/model_zoo_status.yml | 3 ++ .github/workflows/test_on_push.yml | 2 ++ .../src/pybamm_model_zoo/testing/contract.py | 16 +++++++++ .../pybamm-model-zoo/tests/test_registry.py | 35 +++++++++++++++++++ 5 files changed, 60 insertions(+), 1 deletion(-) diff --git a/.github/workflows/_nox.yml b/.github/workflows/_nox.yml index 23ffa4d15b..f5dd3cd3a6 100644 --- a/.github/workflows/_nox.yml +++ b/.github/workflows/_nox.yml @@ -27,7 +27,10 @@ on: required: true type: string fetch_depth: - description: "Checkout depth; the docs build needs the full history (0)." + description: >- + Checkout depth. Sessions needing git history pass 0: the docs build, + and the zoo, which holds a manifest's `pybamm_requires` to the PyBaMM + version hatch-vcs derives from the release tags. default: 1 required: false type: number diff --git a/.github/workflows/model_zoo_status.yml b/.github/workflows/model_zoo_status.yml index 2d8620d061..7bbbfd0228 100644 --- a/.github/workflows/model_zoo_status.yml +++ b/.github/workflows/model_zoo_status.yml @@ -57,6 +57,9 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: submodules: 'recursive' + # The `main` cell installs PyBaMM from this checkout, and hatch-vcs + # needs the release tags to give it a version the zoo can check. + fetch-depth: 0 persist-credentials: false - name: Install Linux system dependencies diff --git a/.github/workflows/test_on_push.yml b/.github/workflows/test_on_push.yml index 47e5a1140d..e60d09e123 100644 --- a/.github/workflows/test_on_push.yml +++ b/.github/workflows/test_on_push.yml @@ -301,6 +301,7 @@ jobs: with: sessions: zoo-gating texlive: false + fetch_depth: 0 timeout_minutes: 30 legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' @@ -316,6 +317,7 @@ jobs: with: sessions: zoo texlive: false + fetch_depth: 0 timeout_minutes: 60 legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py index ede056095e..f4d572f06c 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py @@ -155,6 +155,17 @@ def check_manifest(entry: ModelEntry) -> None: _check_dependency_declaration(entry) +def names_a_release(version: str) -> bool: + """Whether an installed PyBaMM version names a release it can be held to. + + With no ``pybamm-v*`` tag in reach — a shallow CI clone, an sdist, a fork — + hatch-vcs has nothing to describe and guesses a ``0.0.x``, a series PyBaMM + has never published. Such an install cannot say which release it is, so a + declared range has nothing to be checked against. + """ + return Version(version).release[:2] != (0, 0) + + def check_pybamm_requires(entry: ModelEntry) -> None: """The declared PyBaMM range is a valid specifier, satisfied by this install. @@ -162,6 +173,9 @@ def check_pybamm_requires(entry: ModelEntry) -> None: matrix only pairs a model with versions its range admits — so reaching this check with an unsatisfied range means the manifest claims a PyBaMM the tests are not running against, which is a manifest error worth failing loudly. + + The specifier is validated wherever this runs; the range itself is only held + to an install that names a release. See :func:`names_a_release`. """ where = entry.manifest_path assert entry.pybamm_requires.strip(), ( @@ -171,6 +185,8 @@ def check_pybamm_requires(entry: ModelEntry) -> None: satisfied = entry.admits(pybamm.__version__) except ManifestError as error: raise AssertionError(str(error)) from error + if not names_a_release(pybamm.__version__): + return assert satisfied, ( f"{where}: pybamm_requires '{entry.pybamm_requires}' is not satisfied by " f"the installed PyBaMM {pybamm.__version__}" diff --git a/packages/pybamm-model-zoo/tests/test_registry.py b/packages/pybamm-model-zoo/tests/test_registry.py index bf89daf03f..cff6747d30 100644 --- a/packages/pybamm-model-zoo/tests/test_registry.py +++ b/packages/pybamm-model-zoo/tests/test_registry.py @@ -5,6 +5,7 @@ import pytest +import pybamm import pybamm_model_zoo as zoo from pybamm_model_zoo._citations import parse_bibtex from pybamm_model_zoo._registry import Registry @@ -129,6 +130,40 @@ def test_missing_module_names_the_extra(self, tmp_path): assert contract.missing_dependencies(entry) == ["not-a-real-package>=1.0"] +class TestPybammRequires: + """The declared range, against installs that can and cannot name a release.""" + + def entry(self, tmp_path, requires): + body = MANIFEST.format(slug="minimal_model", name="MinimalModel").replace( + 'pybamm_requires = ">=26.0"', f'pybamm_requires = "{requires}"' + ) + write_model(tmp_path, "minimal_model", "MinimalModel", body=body) + return Registry([tmp_path])["MinimalModel"] + + def test_unsatisfied_range_fails_on_a_real_release(self, tmp_path, monkeypatch): + monkeypatch.setattr(pybamm, "__version__", "26.8.0.0") + with pytest.raises(AssertionError, match=r"is not satisfied by"): + contract.check_pybamm_requires(self.entry(tmp_path, ">=99.0")) + + # setuptools_scm's guess with no tag in reach, and hatch-vcs's own fallback. + @pytest.mark.parametrize("version", ["0.0.1.dev1+gabc1234", "0.0.0"]) + def test_range_is_not_held_to_an_install_that_names_no_release( + self, tmp_path, monkeypatch, version + ): + assert not contract.names_a_release(version) + monkeypatch.setattr(pybamm, "__version__", version) + contract.check_pybamm_requires(self.entry(tmp_path, ">=99.0")) + + def test_invalid_specifier_fails_wherever_it_runs(self, tmp_path, monkeypatch): + monkeypatch.setattr(pybamm, "__version__", "0.0.1.dev1+gabc1234") + with pytest.raises(AssertionError, match=r"not a valid specifier"): + contract.check_pybamm_requires(self.entry(tmp_path, "=>26.0")) + + @pytest.mark.parametrize("version", ["0.1.0", "25.1.0", "26.8.0.1.dev4+gabc1234"]) + def test_a_dev_build_off_a_real_tag_still_names_a_release(self, version): + assert contract.names_a_release(version) + + class TestCitationParsing: def test_parses_multiple_entries(self): entries = parse_bibtex( From f138e8d405bd2caa42bee8e2814b35d7e00698e6 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 20 Aug 2026 13:03:56 +0100 Subject: [PATCH 4/7] fix: suppress the five Bandit false positives Codacy gates on Codacy's quality gate allows zero new issues and the zoo commit added five, all from Bandit. Reproduced locally with the repo's own `bandit.yml`, which reports exactly the same five, and none is a real vulnerability: - B105 in `_docs.py`: Bandit reads a dict key of `"pass"` as a password field, so the shields.io colour `"brightgreen"` looks like a hardcoded credential. - B404, B603 and B607 in `test_template.py`: importing and calling `subprocess` to run the repo's own ruff over a rendered template. The argv is a literal list; no external input reaches it. - B310 in `scripts/matrix.py`: `urlopen` on a module-level https constant. Suppressed inline as `# nosec - `, the convention already used in `src/pybamm/codegen/compilation.py` and the unit tests, rather than widening `bandit.yml`, which would disable these rules repo-wide. Bandit now reports zero over the zoo. Nothing changes behaviour: the one code edit is splitting `BADGE_COLORS` across lines so the suppression sits on the key that triggers it. --- packages/pybamm-model-zoo/scripts/matrix.py | 2 +- packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py | 6 +++++- packages/pybamm-model-zoo/tests/test_template.py | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/pybamm-model-zoo/scripts/matrix.py b/packages/pybamm-model-zoo/scripts/matrix.py index c188762d91..40ce153b44 100644 --- a/packages/pybamm-model-zoo/scripts/matrix.py +++ b/packages/pybamm-model-zoo/scripts/matrix.py @@ -37,7 +37,7 @@ def version_order(version: str) -> tuple[int, list[int]]: def released_versions(count: int) -> list[str]: """The ``count`` most recent final PyBaMM releases on PyPI, oldest first.""" - with urllib.request.urlopen(PYPI_URL) as response: + with urllib.request.urlopen(PYPI_URL) as response: # nosec B310 - literal https URL releases = json.load(response)["releases"] published = sorted( ( diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py index 52a9e78db3..266270c013 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py @@ -23,7 +23,11 @@ ) # Depth from docs/source/model_zoo/models/.md back to the repository root. README_PREFIX = "../../../../packages/pybamm-model-zoo/src/pybamm_model_zoo" -BADGE_COLORS = {"pass": "brightgreen", "fail": "red", "untested": "lightgrey"} +BADGE_COLORS = { + "pass": "brightgreen", # nosec B105 - a shields.io colour, not a credential + "fail": "red", + "untested": "lightgrey", +} BADGE_LABEL = "model zoo" _INDEX_HEADER = """\ diff --git a/packages/pybamm-model-zoo/tests/test_template.py b/packages/pybamm-model-zoo/tests/test_template.py index 50c9ab47fc..989bef079a 100644 --- a/packages/pybamm-model-zoo/tests/test_template.py +++ b/packages/pybamm-model-zoo/tests/test_template.py @@ -8,7 +8,7 @@ """ import shutil -import subprocess +import subprocess # nosec B404 - runs the repo's own ruff over a rendered template import sys import pytest @@ -101,7 +101,7 @@ class TestRenderedStyle: "command", [("check", "--no-cache"), ("format", "--check", "--no-cache")] ) def test_rendered_python_is_style_clean(self, rendered, command): - result = subprocess.run( + result = subprocess.run( # nosec B603 B607 - literal argv, no external input [ "ruff", *command, From 675f2077ffb24cf0221cb91d59d37134e2a2e213 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Thu, 20 Aug 2026 15:06:26 +0100 Subject: [PATCH 5/7] fix: internal review Replace the reference entry, and close the gaps the review found in the zoo's own machinery. `spm_series_resistance` applied its `I R` drop in `set_voltage_variables`, which `build_model` calls *after* `_build_model` has built the external circuit's equations, so every control law that reads the terminal voltage back was holding the pre-drop one. Measured at the `R = 0.05` its own tests used: a requested 4 W delivered 3.94 W, 3.5 Ohm held 3.49 Ohm, 3.6 V held 3.59 V. Experiments were unaffected, since `Simulation` attaches the experiment controller to an already-built model. PyBaMM has this physics already as `{"contact resistance": "true"}`, which applies the drop in `get_coupled_variables` and so gets all of that right, plus the `I^2 R` heating and `"voltage as a state"`. The reference entry is now `linearised_spm`, from #3187: SPM with both porous electrodes' open-circuit potential replaced by its tangent at the starting stoichiometry, for fitting a diffusivity to a GITT pulse. The gradient is PyBaMM's symbolic derivative of whichever `U` the parameter set supplies, so the model adds no parameters, and it reports the `dE/dd` a Weppner-Huggins fit otherwise has to read off a titration curve. It substitutes a submodel instead of rewriting variables after the build, so no control law can reach a stale voltage, and it duplicates nothing in core -- #3187 was declined for core as too narrow, which is the case for a zoo entry. A 5 s pulse recovers an imposed 1e-14 m2.s-1 to within 5%, and the residual shrinks monotonically as the pulse shortens (-28.5% at 80 s, -11.4% at 20 s, -2.9% at 5 s), which is the sqrt(t) approximation's own spherical-geometry bias rather than model error. Then the machinery: - Gate the zoo's own tests. `gating` was derived from a `core` slug alone, so the registry, template, contract-infrastructure and generated-file tests -- which belong to no model -- were advisory, including the one asserting the contract suite is not vacuous. A test with no slug now gates however the models happen to be tiered, and `zoo-gating` no longer excludes a core model's examples. - Reject an external model whose *slug* shadows an in-tree one, not just its name. `by_slug` is what resolves citations and the generated per-model files, so a differently-named entry with a colliding slug displaced a built-in model and left `register_citation` reading the wrong folder. - Record a matrix cell that never reported as `missing` instead of dropping it, and colour the badge for it. A leg that died before uploading its artifact used to vanish from the table with the badge left green; the collector now passes the discovered matrix through `--expect`. - Hold a model's manifest and the extra behind it to the same requirements, compared both ways and on the specifier rather than the distribution name. CI installs every model's extra at once, so a one-way name-only comparison let a model lean on a package it never declared. - Keep the previous `status.json` timestamp when no result changed, so the weekly workflow stops opening a pull request that says nothing. - Say what the compatibility matrix covers: the newest releases a manifest admits, which is what `scripts/matrix.py --releases` defaults to, not every release it admits. The status pipeline had no tests at all; `tests/test_status.py` now covers collection, badge precedence and stamping. 55 zoo tests to 73. --- .github/CODEOWNERS | 2 +- .github/workflows/model_zoo_status.yml | 10 +- docs/source/model_zoo/index.md | 13 +- ...series_resistance.md => linearised_spm.md} | 4 +- noxfile.py | 4 +- packages/pybamm-model-zoo/CHANGELOG.md | 2 +- packages/pybamm-model-zoo/README.md | 10 +- ...es_resistance.json => linearised_spm.json} | 0 packages/pybamm-model-zoo/conftest.py | 6 +- packages/pybamm-model-zoo/scripts/generate.py | 33 ++++- .../src/pybamm_model_zoo/__init__.py | 4 +- .../src/pybamm_model_zoo/_docs.py | 52 ++++++- .../src/pybamm_model_zoo/_registry.py | 27 +++- .../CITATION.bib | 14 +- .../pybamm_model_zoo/linearised_spm/README.md | 98 ++++++++++++++ .../linearised_spm/__init__.py | 6 + .../examples/run_linearised_spm.py | 43 ++++++ .../pybamm_model_zoo/linearised_spm/model.py | 127 ++++++++++++++++++ .../linearised_spm/model.toml | 27 ++++ .../tests/test_linearised_spm.py | 123 +++++++++++++++++ .../spm_series_resistance/README.md | 59 -------- .../spm_series_resistance/__init__.py | 3 - .../examples/run_spm_series_resistance.py | 27 ---- .../spm_series_resistance/model.py | 69 ---------- .../spm_series_resistance/model.toml | 22 --- .../tests/test_spm_series_resistance.py | 64 --------- .../src/pybamm_model_zoo/testing/contract.py | 64 +++++++-- .../pybamm-model-zoo/template/model.py.in | 4 +- .../pybamm-model-zoo/tests/test_contract.py | 38 ++++++ .../pybamm-model-zoo/tests/test_registry.py | 34 +++-- .../pybamm-model-zoo/tests/test_status.py | 94 +++++++++++++ 31 files changed, 778 insertions(+), 305 deletions(-) rename docs/source/model_zoo/models/{spm_series_resistance.md => linearised_spm.md} (68%) rename packages/pybamm-model-zoo/badges/{spm_series_resistance.json => linearised_spm.json} (100%) rename packages/pybamm-model-zoo/src/pybamm_model_zoo/{spm_series_resistance => linearised_spm}/CITATION.bib (56%) create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/README.md create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/__init__.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/examples/run_linearised_spm.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.toml create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml delete mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py create mode 100644 packages/pybamm-model-zoo/tests/test_status.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 45ced34f7b..b10db78752 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -7,4 +7,4 @@ # The model zoo: the zoo's own machinery is owned by the maintainers, and each # model folder by its maintainer (last match wins) /packages/pybamm-model-zoo/ @pybamm-team/maintainers -/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/ @pybamm-team/maintainers +/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/ @pybamm-team/maintainers diff --git a/.github/workflows/model_zoo_status.yml b/.github/workflows/model_zoo_status.yml index 7bbbfd0228..7cfb7b9111 100644 --- a/.github/workflows/model_zoo_status.yml +++ b/.github/workflows/model_zoo_status.yml @@ -148,9 +148,15 @@ jobs: path: results # The same generator the pre-commit hook runs, so the schema and the - # renderers that read it cannot drift apart. + # renderers that read it cannot drift apart. `--expect` is what stops a leg + # that died before uploading from vanishing with the badge left green. - name: Fold the results into status.json, badges, and the docs table - run: python packages/pybamm-model-zoo/scripts/generate.py --collect results + env: + CELLS: ${{ needs.discover.outputs.cells }} + run: | + printf '%s' "$CELLS" > cells.json + python packages/pybamm-model-zoo/scripts/generate.py \ + --collect results --expect cells.json - name: Open or update the status pull request env: diff --git a/docs/source/model_zoo/index.md b/docs/source/model_zoo/index.md index b4bc0f5b59..7870a4bc55 100644 --- a/docs/source/model_zoo/index.md +++ b/docs/source/model_zoo/index.md @@ -26,21 +26,24 @@ See [contributing a model](contributing.md) to add your own. | Model | Tier | Maintainer | PyBaMM | Added | | --- | --- | --- | --- | --- | -| [Single Particle Model with a lumped series resistance](models/spm_series_resistance.md) | core | @pybamm-team/maintainers | `>=26.0` | 2026-08-20 | +| [Single Particle Model with a linearised open-circuit potential](models/linearised_spm.md) | core | @pybamm-team/maintainers | `>=26.0` | 2026-08-20 | ## Compatibility -Refreshed weekly by the `model_zoo_status` workflow, which runs each model -against every release its `pybamm_requires` admits, plus `main`. +Refreshed weekly by the `model_zoo_status` workflow. Each model is run against +`main` and against the most recent PyBaMM releases its `pybamm_requires` +admits; `scripts/matrix.py --releases` sets how far back that window reaches, +and the columns below are the window as it stands. A cell reading `missing` is +one whose job never reported, not a pass. | Model | Results | Last passing | | --- | --- | --- | -| spm_series_resistance | not yet run | — | +| linearised_spm | not yet run | — | ```{toctree} :hidden: :maxdepth: 1 Contributing a model -models/spm_series_resistance +models/linearised_spm ``` diff --git a/docs/source/model_zoo/models/spm_series_resistance.md b/docs/source/model_zoo/models/linearised_spm.md similarity index 68% rename from docs/source/model_zoo/models/spm_series_resistance.md rename to docs/source/model_zoo/models/linearised_spm.md index 1bed556c10..4496ce3a30 100644 --- a/docs/source/model_zoo/models/spm_series_resistance.md +++ b/docs/source/model_zoo/models/linearised_spm.md @@ -1,6 +1,6 @@ -(model-zoo-spm_series_resistance)= +(model-zoo-linearised_spm)= -```{include} ../../../../packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md +```{include} ../../../../packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/README.md ``` diff --git a/noxfile.py b/noxfile.py index 18f845f3f2..ebafeaa36e 100644 --- a/noxfile.py +++ b/noxfile.py @@ -342,9 +342,9 @@ def run_zoo(session): @nox.session(name="zoo-gating", default=False) def run_zoo_gating(session): - """Run only the `core`-tier zoo models, which are in PyBaMM's merge gate.""" + """Run what is in PyBaMM's merge gate: `core`-tier models and the zoo itself.""" install_zoo(session) - zoo_pytest(session, "zoo and gating and not zoo_examples") + zoo_pytest(session, "zoo and gating") @nox.session(name="zoo-examples", default=False) diff --git a/packages/pybamm-model-zoo/CHANGELOG.md b/packages/pybamm-model-zoo/CHANGELOG.md index 01f6a6bb01..c05c4e17f0 100644 --- a/packages/pybamm-model-zoo/CHANGELOG.md +++ b/packages/pybamm-model-zoo/CHANGELOG.md @@ -9,4 +9,4 @@ PyBaMM's. - The model zoo: per-model manifests, a registry, a ten-check contract suite, a template and generator, generated docs pages and status badges, and the - `spm_series_resistance` reference entry ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) + `linearised_spm` reference entry ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) diff --git a/packages/pybamm-model-zoo/README.md b/packages/pybamm-model-zoo/README.md index 1d8ed43793..5bb2476bcc 100644 --- a/packages/pybamm-model-zoo/README.md +++ b/packages/pybamm-model-zoo/README.md @@ -14,12 +14,12 @@ import pybamm import pybamm_model_zoo as zoo zoo.list_models() -entry = zoo.info("SPMSeriesResistance") +entry = zoo.info("LinearisedSPM") entry.tier, entry.maintainers, entry.pybamm_requires -model = zoo.load("SPMSeriesResistance")() -solution = pybamm.Simulation(model).solve([0, 3600]) -print(solution["Voltage [V]"](1800)) +model = zoo.load("LinearisedSPM")() +solution = pybamm.Simulation(model).solve([0, 300]) +print(solution["Voltage [V]"](150)) ``` The zoo is a `uv` workspace member, so `uv sync --extra all --group dev` from the @@ -52,7 +52,7 @@ Then: 6. Add a bullet to this package's `CHANGELOG.md`. Zoo pull requests never touch PyBaMM's changelog. -[`spm_series_resistance/`](https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance) +[`linearised_spm/`](https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm) is the reference entry: the smallest thing that is still a real model. Copy that folder rather than reading prose. diff --git a/packages/pybamm-model-zoo/badges/spm_series_resistance.json b/packages/pybamm-model-zoo/badges/linearised_spm.json similarity index 100% rename from packages/pybamm-model-zoo/badges/spm_series_resistance.json rename to packages/pybamm-model-zoo/badges/linearised_spm.json diff --git a/packages/pybamm-model-zoo/conftest.py b/packages/pybamm-model-zoo/conftest.py index cf1c255679..7abb9800db 100644 --- a/packages/pybamm-model-zoo/conftest.py +++ b/packages/pybamm-model-zoo/conftest.py @@ -64,8 +64,10 @@ def pytest_collection_modifyitems(config, items): slug = _slug_of(item) # Advisory-ness lives in the CI job, never in a marker: `gating` only - # says whether a failure blocks a merge. - if slug in core_slugs: + # says whether a failure blocks a merge. A test belonging to no model is + # the zoo's own machinery -- the registry, the template, the contract + # suite itself -- which gates however the models happen to be tiered. + if slug is None or slug in core_slugs: item.add_marker(pytest.mark.gating) if selected is not None and slug != selected: deselected.append(item) diff --git a/packages/pybamm-model-zoo/scripts/generate.py b/packages/pybamm-model-zoo/scripts/generate.py index 56ac0000ae..a76163d50e 100644 --- a/packages/pybamm-model-zoo/scripts/generate.py +++ b/packages/pybamm-model-zoo/scripts/generate.py @@ -4,7 +4,8 @@ so the ``docs`` contract check can compare a page against what it should contain. uv run python packages/pybamm-model-zoo/scripts/generate.py [--check] - uv run python packages/pybamm-model-zoo/scripts/generate.py --collect results/ + uv run python packages/pybamm-model-zoo/scripts/generate.py \ + --collect results/ [--expect cells.json] """ from __future__ import annotations @@ -38,20 +39,38 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: "before rendering" ), ) + parser.add_argument( + "--expect", + metavar="FILE", + type=Path, + help=( + "the matrix the run set out to cover, as the JSON list of " + "{model, version} cells that scripts/matrix.py emitted; a cell " + "missing from --collect is recorded as such instead of dropped" + ), + ) return parser.parse_args(argv) +def now() -> str: + """The current UTC time, as the timestamp format ``status.json`` carries.""" + return ( + datetime.datetime.now(datetime.timezone.utc) + .replace(microsecond=0) + .isoformat() + .replace("+00:00", "Z") + ) + + def main(argv: list[str] | None = None) -> int: args = parse_args(argv) if args.collect: - generated = ( - datetime.datetime.now(datetime.timezone.utc) - .replace(microsecond=0) - .isoformat() - .replace("+00:00", "Z") + expected = ( + json.loads(args.expect.read_text(encoding="utf-8")) if args.expect else None ) - status = _docs.collect_results(args.collect, generated) + collected = _docs.collect_results(args.collect, expected=expected) + status = _docs.stamp(collected, _docs.read_status(), now()) _paths.STATUS_FILE.write_text( json.dumps(status, indent=2) + "\n", encoding="utf-8" ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py index 1facf47ece..1b7a762c7b 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py @@ -7,9 +7,9 @@ Examples -------- >>> import pybamm_model_zoo as zoo ->>> "SPMSeriesResistance" in zoo.list_models() +>>> "LinearisedSPM" in zoo.list_models() True ->>> entry = zoo.info("SPMSeriesResistance") +>>> entry = zoo.info("LinearisedSPM") >>> entry.tier 'core' """ diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py index 266270c013..1c665a0fd7 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py @@ -26,9 +26,13 @@ BADGE_COLORS = { "pass": "brightgreen", # nosec B105 - a shields.io colour, not a credential "fail": "red", + "missing": "orange", "untested": "lightgrey", } BADGE_LABEL = "model zoo" +#: A matrix cell that was expected but reported nothing, so its leg never +#: finished. Never treated as a pass: a timed-out job must not read as green. +MISSING = "missing" _INDEX_HEADER = """\ (model_zoo)= @@ -65,8 +69,11 @@ ## Compatibility -Refreshed weekly by the `model_zoo_status` workflow, which runs each model -against every release its `pybamm_requires` admits, plus `main`. +Refreshed weekly by the `model_zoo_status` workflow. Each model is run against +`main` and against the most recent PyBaMM releases its `pybamm_requires` +admits; `scripts/matrix.py --releases` sets how far back that window reaches, +and the columns below are the window as it stands. A cell reading `missing` is +one whose job never reported, not a pass. | Model | Results | Last passing | | --- | --- | --- | @@ -97,14 +104,26 @@ def read_status(path: Path | None = None) -> dict: return json.loads(path.read_text(encoding="utf-8")) -def collect_results(results_dir: Path, generated: str) -> dict: - """Fold one JSON file per matrix cell into the ``status.json`` shape.""" +def collect_results(results_dir: Path, expected: list[dict] | None = None) -> dict: + """Fold one JSON file per matrix cell into the ``status.json`` shape. + + Parameters + ---------- + results_dir : Path + Directory of ``{model, version, result}`` files, one per reported cell. + expected : list of dict, optional + Every ``{model, version}`` cell the matrix set out to run. A cell with no + file is recorded as :data:`MISSING` rather than omitted, so a leg that + died before uploading cannot silently leave a green badge behind. + """ models: dict[str, dict[str, str]] = {} for path in sorted(Path(results_dir).glob("*.json")): record = json.loads(path.read_text(encoding="utf-8")) models.setdefault(record["model"], {})[record["version"]] = record["result"] + for cell in expected or []: + models.setdefault(cell["model"], {}).setdefault(cell["version"], MISSING) - status: dict = {"generated": generated, "models": {}} + status: dict = {"models": {}} for model, results in sorted(models.items()): passing = [ version @@ -120,17 +139,38 @@ def collect_results(results_dir: Path, generated: str) -> dict: return status +def stamp(collected: dict, previous: dict, generated: str) -> dict: + """``collected``, timestamped, keeping ``previous``'s stamp if it still holds. + + Restamping an unchanged result would put a diff in front of a reviewer every + week and say nothing by it. + """ + if collected["models"] == previous.get("models"): + generated = previous.get("generated") or generated + return {"generated": generated, **collected} + + def badge(record: dict) -> dict: """The shields.io endpoint payload for one model's status record.""" results = record.get("results", {}) failing = sorted( - (version for version, result in results.items() if result != "pass"), + (version for version, result in results.items() if result == "fail"), + key=version_key, + ) + missing = sorted( + ( + version + for version, result in results.items() + if result not in ("pass", "fail") + ), key=version_key, ) if not results: message, color = "untested", BADGE_COLORS["untested"] elif failing: message, color = f"failing on {', '.join(failing)}", BADGE_COLORS["fail"] + elif missing: + message, color = f"no result on {', '.join(missing)}", BADGE_COLORS["missing"] else: message = f"passing ({record.get('last_pass') or 'latest'})" color = BADGE_COLORS["pass"] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py index 361f1acbf7..858ec97683 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py @@ -273,23 +273,40 @@ def __init__( ): self._discover(root, external=True) + def _shadowed(self, entry: ModelEntry) -> tuple[str, ModelEntry] | None: + """The registered entry ``entry`` would displace, and how it clashes. + + Both keys have to be checked: ``by_slug`` resolves citations and the + generated per-model files, so a clashing slug shadows a model just as + effectively as a clashing name. + """ + for label, registered in ( + (f"name '{entry.name}'", self._entries.get(entry.name)), + (f"slug '{entry.slug}'", self._by_slug.get(entry.slug)), + ): + if registered is not None: + return label, registered + return None + def _discover(self, root: Path, *, external: bool) -> None: for manifest in sorted(Path(root).glob(f"*/{MANIFEST_NAME}")): entry = _entry_from_manifest(manifest, external=external) - existing = self._entries.get(entry.name) - if existing is None: + clash = self._shadowed(entry) + if clash is None: self._entries[entry.name] = entry self._by_slug[entry.slug] = entry - elif external: + continue + label, existing = clash + if external: # In-tree models win, so a third-party package cannot shadow one. warnings.warn( f"ignoring external model '{entry.name}' from {manifest}: " - f"the name is already registered by {existing.manifest_path}", + f"{label} is already registered by {existing.manifest_path}", stacklevel=2, ) else: raise ManifestError( - f"{manifest}: duplicate model name '{entry.name}', already " + f"{manifest}: duplicate model {label}, already " f"declared by {existing.manifest_path}" ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/CITATION.bib similarity index 56% rename from packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib rename to packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/CITATION.bib index ba398e93d1..d877fd5a3a 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/CITATION.bib +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/CITATION.bib @@ -1,11 +1,23 @@ @software{PyBaMMModelZoo2026, - title = {{PyBaMM model zoo: Single Particle Model with a lumped series resistance}}, + title = {{PyBaMM model zoo: Single Particle Model with a linearised open-circuit potential}}, author = {{The PyBaMM Team}}, year = {2026}, url = {https://github.com/pybamm-team/PyBaMM/tree/main/packages/pybamm-model-zoo}, note = {Reference entry of the PyBaMM model zoo}, } +@article{WeppnerHuggins1977, + title = {{Determination of the kinetic parameters of mixed-conducting electrodes and application to the system Li3Sb}}, + author = {Weppner, W. and Huggins, R. A.}, + journal = {Journal of The Electrochemical Society}, + volume = {124}, + number = {10}, + pages = {1569--1578}, + year = {1977}, + publisher = {The Electrochemical Society}, + doi = {10.1149/1.2133112}, +} + @article{Marquis2019, title = {{An asymptotic derivation of a single particle model with electrolyte}}, author = {Marquis, Scott G. and Sulzer, Valentin and Timms, Robert and Please, Colin P. and Chapman, S. Jon}, diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/README.md b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/README.md new file mode 100644 index 0000000000..cc27689b4c --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/README.md @@ -0,0 +1,98 @@ +# LinearisedSPM + +![status](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/pybamm-team/PyBaMM/main/packages/pybamm-model-zoo/badges/linearised_spm.json) + +## Summary + +The Single Particle Model with both porous electrodes' open-circuit potentials +replaced by their tangent at the stoichiometry the simulation starts from, +`U(x) -> U(x_0) + U'(x_0) (x - x_0)`, and `"intercalation kinetics"` defaulting +to `"linear"`. The gradient is PyBaMM's symbolic derivative of whichever `U` the +parameter set supplies, so the model adds no parameters of its own. + +This is the model for **GITT analysis**. Over a pulse short enough that the +particles stay near `x_0`, the voltage transient is the Weppner-Huggins form: it +varies as `sqrt(t)` with a slope set by the diffusivity and by `dU/dx`. Fitting +a diffusivity to a measured pulse needs `dU/dx`, which is normally read off a +separately measured titration curve; here the model holds it exactly and reports +it as `" electrode open-circuit potential gradient [V]"`. + +Prefer `pybamm.lithium_ion.SPM` for anything that ranges far from `x_0`. The +tangent is a local approximation, and over a full discharge it departs from the +real open-circuit voltage — which is exactly why this is a zoo entry and not a +core PyBaMM option ([#3187](https://github.com/pybamm-team/PyBaMM/issues/3187)). + +This is also the model zoo's **reference entry**: it is deliberately the +smallest thing that is still a real model. Copy this folder as the starting +point for your own. + +## Usage + +```python +import numpy as np +import pybamm +import pybamm_model_zoo as zoo + +model = zoo.load("LinearisedSPM")({"working electrode": "positive"}) +parameter_values = model.default_parameter_values +parameter_values["Positive particle diffusivity [m2.s-1]"] = 1e-14 + +simulation = pybamm.Simulation( + model, + parameter_values=parameter_values, + var_pts={**model.default_var_pts, "r_p": 200}, +) +solution = simulation.solve([0, 5]) + +times = np.linspace(0.25, 5, 60) +slope = np.polyfit(np.sqrt(times), solution["Voltage [V]"](times), 1)[0] +gradient = solution["Positive electrode open-circuit potential gradient [V]"](0.25) +print(slope, gradient) +``` + +`examples/run_linearised_spm.py` completes the inversion and prints the fitted +diffusivity for three pulse lengths. + +## Variables + +Beyond SPM's own, per porous electrode: + +* `" electrode linearisation stoichiometry"` — the `x_0` linearised about. +* `" electrode linearisation open-circuit potential [V]"` — `U(x_0)`. +* `" electrode open-circuit potential gradient [V]"` — `dU/dx` at `x_0`, + the `dE/dδ` a Weppner-Huggins fit needs. + +## Validation + +* Each electrode's reported open-circuit potential equals + `U(x_0) + U'(x_0) (x - x_0)` to `rtol=1e-12`, from the model's own reported + `x_0`, `U(x_0)`, and gradient. +* The reported gradient matches a central difference of the parameter set's OCP + function to `rtol=1e-5`. +* At `t = 0` the bulk open-circuit voltage equals `pybamm.lithium_ion.SPM`'s to + `rtol=1e-12`: linearising changes nothing at the point linearised about. +* A 5 s GITT pulse on a positive half cell recovers an imposed + `1e-14 m2.s-1` diffusivity to within 5% through the Weppner-Huggins relation, + and the residual error shrinks monotonically as the pulse shortens (−28.5% at + 80 s, −11.4% at 20 s, −2.9% at 5 s). That residual is the `sqrt(t)` + approximation's own spherical-geometry bias, not an error in the model, which + is why GITT pulses are kept short in practice. +* All of these run in `tests/test_linearised_spm.py`. + +Not validated, and rejected rather than silently ignored: the +`"open-circuit potential"` option, since this model supplies its own — any value +other than `"single"` raises `pybamm.OptionError`. Nothing linearises the +kinetics' exchange current density or a planar electrode's plating potential, so +the model is not exactly linear in the state; over a short pulse both terms are +near constant. + +## Citation + +See `CITATION.bib`. Cite `WeppnerHuggins1977` for the method, +`PyBaMMModelZoo2026` for this entry, and `Marquis2019` for the underlying SPM; +all three are registered automatically, so `pybamm.print_citations()` lists them +after you use the model. + +## Maintainer + +The PyBaMM Team (@pybamm-team/maintainers) — tier: core diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/__init__.py new file mode 100644 index 0000000000..1b47e775b3 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/__init__.py @@ -0,0 +1,6 @@ +from pybamm_model_zoo.linearised_spm.model import ( + LinearisedOpenCircuitPotential, + LinearisedSPM, +) + +__all__ = ["LinearisedOpenCircuitPotential", "LinearisedSPM"] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/examples/run_linearised_spm.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/examples/run_linearised_spm.py new file mode 100644 index 0000000000..cb3dca258e --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/examples/run_linearised_spm.py @@ -0,0 +1,43 @@ +"""Fit a diffusivity to a GITT pulse, the way the Weppner-Huggins method does.""" + +import numpy as np + +import pybamm +import pybamm_model_zoo as zoo + +FARADAY = 96485.33212 +DIFFUSIVITY = 1e-14 + +model = zoo.load("LinearisedSPM")({"working electrode": "positive"}) +parameter_values = model.default_parameter_values +parameter_values["Positive particle diffusivity [m2.s-1]"] = DIFFUSIVITY +radius = parameter_values["Positive particle radius [m]"] +concentration_max = parameter_values[ + "Maximum concentration in positive electrode [mol.m-3]" +] +simulation = pybamm.Simulation( + model, + parameter_values=parameter_values, + var_pts={**model.default_var_pts, "r_p": 200}, +) + +print(f"particle diffusion time R^2/D = {radius**2 / DIFFUSIVITY:.0f} s") +print(" pulse [s] dV/dsqrt(t) [V.s-0.5] D fitted [m2.s-1] error") +for pulse in (80.0, 20.0, 5.0): + solution = simulation.solve([0, pulse]) + times = np.linspace(pulse / 20, pulse, 60) + gradient = solution["Positive electrode open-circuit potential gradient [V]"]( + times[0] + ) + flux = ( + solution["X-averaged positive electrode interfacial current density [A.m-2]"]( + times[0] + ) + / FARADAY + ) + slope = np.polyfit(np.sqrt(times), solution["Voltage [V]"](times), 1)[0] + fitted = (4 / np.pi) * (gradient * flux / (concentration_max * slope)) ** 2 + print( + f"{pulse:11.0f} {slope:21.3e} {fitted:17.3e} " + f"{fitted / DIFFUSIVITY - 1:+6.1%}" + ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py new file mode 100644 index 0000000000..77037d2f84 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py @@ -0,0 +1,127 @@ +"""Single Particle Model with a linearised open-circuit potential.""" + +from __future__ import annotations + +import pybamm +import pybamm_model_zoo +from pybamm_model_zoo import _compat + +SLUG = "linearised_spm" + + +class LinearisedOpenCircuitPotential( + pybamm.open_circuit_potential.BaseOpenCircuitPotential +): + """Open-circuit potential linearised about the initial stoichiometry. + + Replaces ``U(x)`` with its tangent at the stoichiometry the simulation starts + from, ``U(x_0) + U'(x_0) (x - x_0)``. The gradient is PyBaMM's own symbolic + derivative of whatever ``U`` the parameter set supplies, so no separate + slope parameter is needed. + """ + + def get_coupled_variables(self, variables: dict) -> dict: + _, Domain = self.domain_Domain + phase_name = self.phase_name + sto_surf, sto_bulk, T, T_bulk = self._get_stoichiometry_and_temperature( + variables + ) + sto_0 = self.phase_param.sto_init_av + gradient = self.phase_param.U(sto_0, T).diff(sto_0) + gradient_bulk = self.phase_param.U(sto_0, T_bulk).diff(sto_0) + + ocp_surf = self.phase_param.U(sto_0, T) + gradient * (sto_surf - sto_0) + ocp_bulk = self.phase_param.U(sto_0, T_bulk) + gradient_bulk * ( + sto_bulk - sto_0 + ) + # Evaluated at the linearisation point too, so the model stays linear + # in the state when a thermal submodel reads it. + dUdT = self.phase_param.dUdT(sto_0) + + variables.update(self._get_standard_ocp_variables(ocp_surf, ocp_bulk, dUdT)) + self._alias_ocp_as_equilibrium(variables) + variables.update( + { + f"{Domain} electrode {phase_name}linearisation stoichiometry": sto_0, + f"{Domain} electrode {phase_name}linearisation open-circuit " + "potential [V]": self.phase_param.U(sto_0, T_bulk), + f"{Domain} electrode {phase_name}open-circuit potential " + "gradient [V]": gradient_bulk, + } + ) + return variables + + +class LinearisedSPM(pybamm.lithium_ion.SPM): + """SPM linearised about the stoichiometry it starts from, for GITT analysis. + + Both porous electrodes' open-circuit potentials are replaced by their + tangent at the initial stoichiometry, and ``"intercalation kinetics"`` + defaults to ``"linear"``. Over a pulse short enough to stay near that point + the voltage transient is the Weppner-Huggins ``sqrt(t)`` form, with a slope + set by the diffusivity and by ``dU/dx``, which the model reports. Prefer + :class:`pybamm.lithium_ion.SPM` for anything ranging far from the starting + stoichiometry, where the tangent is no longer a good approximation. + + Parameters + ---------- + options : dict, optional + Model options, as for :class:`pybamm.lithium_ion.SPM`. The + ``"open-circuit potential"`` option must be ``"single"``, since this + model supplies its own. + name : str, optional + The model name. + build : bool, optional + Whether to build the model on instantiation. + + Examples + -------- + >>> import pybamm_model_zoo as zoo + >>> model = zoo.load("LinearisedSPM")() + >>> "Positive electrode open-circuit potential gradient [V]" in model.variables + True + """ + + def __init__( + self, + options: dict | None = None, + name: str = "Linearised Single Particle Model", + build: bool = True, + ) -> None: + super().__init__( + options=_compat.spm_default_options( + {"intercalation kinetics": "linear", **(options or {})} + ), + name=name, + build=build, + ) + pybamm_model_zoo.register_citation( + SLUG, "PyBaMMModelZoo2026", "WeppnerHuggins1977" + ) + + def set_open_circuit_potential_submodel(self) -> None: + # Let the base class wire up every electrode, then take over the porous + # ones; a planar electrode's plating potential is not linearised. + super().set_open_circuit_potential_submodel() + for domain in ("negative", "positive"): + if self.options.electrode_types[domain] != "porous": + continue + domain_options = getattr(self.options, domain) + for phase in self.options.phases[domain]: + option = getattr(domain_options, phase)["open-circuit potential"] + if option != "single": + raise pybamm.OptionError( + f"LinearisedSPM supplies its own open-circuit potential, " + f"so it cannot also apply 'open-circuit potential': " + f"'{option}' in the {domain} electrode." + ) + self.submodels[f"{domain} {phase} open-circuit potential"] = ( + LinearisedOpenCircuitPotential( + self.param, + domain, + "lithium-ion main", + self.options, + phase, + self.x_average, + ) + ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.toml b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.toml new file mode 100644 index 0000000000..7fdda66f7b --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.toml @@ -0,0 +1,27 @@ +[model] +slug = "linearised_spm" +name = "LinearisedSPM" +title = "Single Particle Model with a linearised open-circuit potential" +summary = "SPM linearised about its starting stoichiometry, for fitting diffusivity to GITT pulses." +class = "pybamm_model_zoo.linearised_spm:LinearisedSPM" +tier = "core" +pybamm_requires = ">=26.0" +added = "2026-08-20" +license = "BSD-3-Clause" + +[[model.maintainers]] +name = "The PyBaMM Team" +github = "pybamm-team/maintainers" + +[model.citation] +key = "PyBaMMModelZoo2026" + +[model.tests] +# A GITT pulse, not a discharge: the linearisation is local to the starting +# stoichiometry, so a full discharge is outside what this model claims. +solve_time = 300 +key_variables = [ + "Voltage [V]", + "Positive electrode open-circuit potential gradient [V]", +] +skip_contract = [] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py new file mode 100644 index 0000000000..701db9dc5d --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py @@ -0,0 +1,123 @@ +"""Physics tests for the reference model zoo entry. + +These are the contributor's own tests (the zoo's "Layer B"): the contract suite +already checks that the model imports, is well posed, builds, and solves, so +these pin *physical* results instead. +""" + +import numpy as np +import pytest + +import pybamm +import pybamm_model_zoo as zoo + +FARADAY = 96485.33212 +PULSE = 60.0 +#: Radial points needed to resolve the sqrt(D t) boundary layer of a short pulse. +PARTICLE_POINTS = 200 + + +def half_cell(diffusivity, pulse=PULSE, particle_points=PARTICLE_POINTS): + """A GITT pulse on the positive electrode against a lithium counter-electrode.""" + model = zoo.load("LinearisedSPM")({"working electrode": "positive"}) + parameter_values = model.default_parameter_values + parameter_values["Positive particle diffusivity [m2.s-1]"] = diffusivity + simulation = pybamm.Simulation( + model, + parameter_values=parameter_values, + var_pts={**model.default_var_pts, "r_p": particle_points}, + ) + return parameter_values, simulation.solve([0, pulse]) + + +def recovered_diffusivity(parameter_values, solution, pulse=PULSE): + """Invert the Weppner-Huggins relation for the diffusivity, as a GITT fit does.""" + times = np.linspace(pulse / 20, pulse, 60) + voltage = solution["Voltage [V]"](times) + gradient = solution["Positive electrode open-circuit potential gradient [V]"]( + times[0] + ) + flux = ( + solution["X-averaged positive electrode interfacial current density [A.m-2]"]( + times[0] + ) + / FARADAY + ) + concentration_max = parameter_values[ + "Maximum concentration in positive electrode [mol.m-3]" + ] + slope = np.polyfit(np.sqrt(times), voltage, 1)[0] + return (4 / np.pi) * (gradient * flux / (concentration_max * slope)) ** 2 + + +class TestLinearisedSPM: + def test_open_circuit_potential_is_the_tangent(self): + model = zoo.load("LinearisedSPM")() + solution = pybamm.Simulation(model).solve([0, 300]) + times = np.linspace(0, 300, 20) + for domain in ("negative", "positive"): + reported = solution[ + f"X-averaged {domain} electrode open-circuit potential [V]" + ](times) + capitalised = domain.capitalize() + tangent = solution[ + f"{capitalised} electrode linearisation open-circuit potential [V]" + ](times) + solution[ + f"{capitalised} electrode open-circuit potential gradient [V]" + ](times) * ( + solution[f"X-averaged {domain} particle surface stoichiometry"](times) + - solution[f"{capitalised} electrode linearisation stoichiometry"]( + times + ) + ) + np.testing.assert_allclose(reported, tangent, rtol=1e-12) + + def test_agrees_with_spm_at_the_linearisation_point(self): + solution = pybamm.Simulation(zoo.load("LinearisedSPM")()).solve([0, 300]) + reference = pybamm.Simulation(pybamm.lithium_ion.SPM()).solve([0, 300]) + np.testing.assert_allclose( + solution["Bulk open-circuit voltage [V]"](0.0), + reference["Bulk open-circuit voltage [V]"](0.0), + rtol=1e-12, + ) + + def test_gradient_is_the_derivative_of_the_parameter_sets_ocp(self): + model = zoo.load("LinearisedSPM")() + solution = pybamm.Simulation(model).solve([0, 300]) + parameter_values = model.default_parameter_values + for domain in ("Negative", "Positive"): + stoichiometry = solution[f"{domain} electrode linearisation stoichiometry"]( + 0.0 + ) + step = 1e-6 + ocp = parameter_values[f"{domain} electrode OCP [V]"] + expected = (ocp(stoichiometry + step) - ocp(stoichiometry - step)) / ( + 2 * step + ) + np.testing.assert_allclose( + solution[f"{domain} electrode open-circuit potential gradient [V]"]( + 0.0 + ), + expected, + rtol=1e-5, + ) + + def test_gitt_pulse_recovers_the_diffusivity(self): + diffusivity = 1e-14 + parameter_values, solution = half_cell(diffusivity, pulse=5.0) + recovered = recovered_diffusivity(parameter_values, solution, pulse=5.0) + np.testing.assert_allclose(recovered, diffusivity, rtol=0.05) + + def test_weppner_huggins_bias_shrinks_with_the_pulse(self): + """The residual is the sqrt(t) approximation, so it must vanish with it.""" + diffusivity = 1e-14 + errors = [] + for pulse in (80.0, 20.0, 5.0): + parameter_values, solution = half_cell(diffusivity, pulse=pulse) + recovered = recovered_diffusivity(parameter_values, solution, pulse=pulse) + errors.append(abs(recovered / diffusivity - 1)) + assert errors[0] > errors[1] > errors[2], errors + + def test_rejects_an_open_circuit_potential_it_would_ignore(self): + with pytest.raises(pybamm.OptionError, match=r"open-circuit potential"): + zoo.load("LinearisedSPM")({"open-circuit potential": "current sigmoid"}) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md deleted file mode 100644 index cf7fee222b..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/README.md +++ /dev/null @@ -1,59 +0,0 @@ -# SPMSeriesResistance - -![status](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/pybamm-team/PyBaMM/main/packages/pybamm-model-zoo/badges/spm_series_resistance.json) - -## Summary - -The Single Particle Model with a lumped ohmic series resistance: the terminal -voltage is the SPM voltage minus `I R`, where `R` is a new parameter, -`"Series resistance [Ohm]"`. It stands in for everything outside the -electrochemistry — tabs, welds, busbars, cabling — when a measured cell shows a -constant offset that the electrochemical model alone cannot account for. Prefer -it over post-processing the SPM voltage when the drop should also move the -voltage cut-offs, the reported power, and the ECM resistance, all of which the -model derives from the shifted voltage. - -This is the model zoo's **reference entry**: it is deliberately the smallest thing -that is still a real model. Copy this folder as the starting point for your own. - -## Usage - -```python -import pybamm -import pybamm_model_zoo as zoo - -model = zoo.load("SPMSeriesResistance")() -parameter_values = model.default_parameter_values -parameter_values["Series resistance [Ohm]"] = 0.05 - -simulation = pybamm.Simulation(model, parameter_values=parameter_values) -solution = simulation.solve([0, 1800]) -print(solution["Voltage [V]"](900)) -``` - -## Validation - -* At `R = 0` the model reproduces `pybamm.lithium_ion.SPM` voltage to - `rtol=1e-6` over a 1800 s 1C discharge. -* At `R > 0` under constant current, the voltage offset from the `R = 0` solution - equals `I R` to `rtol=1e-5` — the residual is interpolation error between two - independently adaptive solves, not a physical difference. -* Both checks run in `tests/test_spm_series_resistance.py`. - -Not validated: any operating mode where the external circuit reads the terminal -voltage back, since the drop is applied after the circuit submodel has been -built. Under power or voltage control the *internal* voltage is what the circuit -holds, and the `"voltage as a state"` option raises `pybamm.OptionError` for the -same reason. Thermal coupling of the `I^2 R` loss is not included: the resistance -is outside the cell in this model, so its heat is not fed to the thermal -submodel. - -## Citation - -See `CITATION.bib`. Cite `PyBaMMModelZoo2026` for this entry and -`Marquis2019` for the underlying SPM; both are registered automatically, so -`pybamm.print_citations()` lists them after you use the model. - -## Maintainer - -The PyBaMM Team (@pybamm-team/maintainers) — tier: core diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py deleted file mode 100644 index 39db89855d..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -from pybamm_model_zoo.spm_series_resistance.model import SPMSeriesResistance - -__all__ = ["SPMSeriesResistance"] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py deleted file mode 100644 index 51cc35c47f..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/examples/run_spm_series_resistance.py +++ /dev/null @@ -1,27 +0,0 @@ -"""Compare the SPM with and without a lumped series resistance.""" - -import numpy as np - -import pybamm -import pybamm_model_zoo as zoo - - -def solve(series_resistance): - model = zoo.load("SPMSeriesResistance")() - parameter_values = model.default_parameter_values - parameter_values["Series resistance [Ohm]"] = series_resistance - simulation = pybamm.Simulation(model, parameter_values=parameter_values) - return simulation.solve([0, 1800]) - - -solutions = {resistance: solve(resistance) for resistance in (0.0, 0.05)} -times = np.linspace(0, 1800, 7) - -print(" t [s] V(R=0) [V] V(R=0.05) [V] drop [V]") -for time in times: - without = solutions[0.0]["Voltage [V]"](time) - with_resistance = solutions[0.05]["Voltage [V]"](time) - print( - f"{time:7.0f} {without:10.5f} {with_resistance:13.5f} " - f"{without - with_resistance:8.5f}" - ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py deleted file mode 100644 index eca6e4f2bb..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.py +++ /dev/null @@ -1,69 +0,0 @@ -"""Single Particle Model with a lumped ohmic series resistance.""" - -from __future__ import annotations - -import pybamm -import pybamm_model_zoo -from pybamm_model_zoo import _compat - -SLUG = "spm_series_resistance" -DEFAULT_SERIES_RESISTANCE = 0.01 - - -class SPMSeriesResistance(pybamm.lithium_ion.SPM): - """SPM whose terminal voltage carries a lumped ohmic drop, ``V - I R``. - - The resistance ``R`` stands in for everything outside the electrochemistry: - tabs, welds, busbars, and cabling. It enters as a new parameter, - ``"Series resistance [Ohm]"``, and is applied to the terminal voltage before - the base class derives the cut-off events, power, and battery voltage from - it, so those all see the drop too. - - Parameters - ---------- - options : dict, optional - Model options, as for :class:`pybamm.lithium_ion.SPM`. The - ``"voltage as a state"`` option is not supported. - name : str, optional - The model name. - build : bool, optional - Whether to build the model on instantiation. - - Examples - -------- - >>> import pybamm_model_zoo as zoo - >>> model = zoo.load("SPMSeriesResistance")() - >>> "Series resistance overpotential [V]" in model.variables - True - """ - - def __init__( - self, - options: dict | None = None, - name: str = "Single Particle Model with series resistance", - build: bool = True, - ) -> None: - super().__init__( - options=_compat.spm_default_options(options), name=name, build=build - ) - pybamm_model_zoo.register_citation(SLUG) - - def set_voltage_variables(self) -> None: - if self.options["voltage as a state"] == "true": - raise pybamm.OptionError( - "SPMSeriesResistance does not support 'voltage as a state': the " - "algebraic constraint would pin the state to the voltage before " - "the series drop is applied." - ) - resistance = pybamm.Parameter("Series resistance [Ohm]") - overpotential = -self.variables["Current [A]"] * resistance - for key in ("Voltage [V]", "Terminal voltage [V]"): - self.variables[key] = self.variables[key] + overpotential - self.variables["Series resistance overpotential [V]"] = overpotential - super().set_voltage_variables() - - @property - def default_parameter_values(self) -> pybamm.ParameterValues: - values = super().default_parameter_values - values.update({"Series resistance [Ohm]": DEFAULT_SERIES_RESISTANCE}) - return values diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml deleted file mode 100644 index 0ef45ff4ba..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/model.toml +++ /dev/null @@ -1,22 +0,0 @@ -[model] -slug = "spm_series_resistance" -name = "SPMSeriesResistance" -title = "Single Particle Model with a lumped series resistance" -summary = "SPM with an ohmic V = V_SPM - I*R terminal drop, for tab, weld, and cable resistance." -class = "pybamm_model_zoo.spm_series_resistance:SPMSeriesResistance" -tier = "core" -pybamm_requires = ">=26.0" -added = "2026-08-20" -license = "BSD-3-Clause" - -[[model.maintainers]] -name = "The PyBaMM Team" -github = "pybamm-team/maintainers" - -[model.citation] -key = "PyBaMMModelZoo2026" - -[model.tests] -solve_time = 3600 -key_variables = ["Voltage [V]", "Battery voltage [V]"] -skip_contract = [] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py deleted file mode 100644 index 4f078345ab..0000000000 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/spm_series_resistance/tests/test_spm_series_resistance.py +++ /dev/null @@ -1,64 +0,0 @@ -"""Physics tests for the reference model zoo entry. - -These are the contributor's own tests (the zoo's "Layer B"): the contract suite -already checks that the model imports, is well posed, builds, and solves, so -these pin *physical* results instead. -""" - -import numpy as np -import pytest - -import pybamm -import pybamm_model_zoo as zoo - -SOLVE_TIME = 1800 -SERIES_RESISTANCE = 0.05 - - -def solve(series_resistance): - model = zoo.load("SPMSeriesResistance")() - parameter_values = model.default_parameter_values - parameter_values["Series resistance [Ohm]"] = series_resistance - simulation = pybamm.Simulation(model, parameter_values=parameter_values) - return simulation.solve([0, SOLVE_TIME]) - - -class TestSPMSeriesResistance: - def test_default_parameter_values_carry_the_resistance(self): - model = zoo.load("SPMSeriesResistance")() - assert "Series resistance [Ohm]" in model.default_parameter_values - - def test_reduces_to_spm_at_zero_resistance(self): - core = pybamm.lithium_ion.SPM() - reference = pybamm.Simulation(core).solve([0, SOLVE_TIME]) - solution = solve(0.0) - times = np.linspace(0, SOLVE_TIME, 50) - np.testing.assert_allclose( - solution["Voltage [V]"](times), - reference["Voltage [V]"](times), - rtol=1e-6, - ) - - def test_voltage_offset_equals_current_times_resistance(self): - without = solve(0.0) - with_resistance = solve(SERIES_RESISTANCE) - times = np.linspace(0, SOLVE_TIME, 50) - current = without["Current [A]"](times) - np.testing.assert_allclose( - without["Voltage [V]"](times) - with_resistance["Voltage [V]"](times), - current * SERIES_RESISTANCE, - rtol=1e-5, - ) - - def test_overpotential_variable_matches_the_drop(self): - solution = solve(SERIES_RESISTANCE) - times = np.linspace(0, SOLVE_TIME, 50) - np.testing.assert_allclose( - solution["Series resistance overpotential [V]"](times), - -solution["Current [A]"](times) * SERIES_RESISTANCE, - rtol=1e-12, - ) - - def test_voltage_as_a_state_is_rejected(self): - with pytest.raises(pybamm.OptionError, match=r"voltage as a state"): - zoo.load("SPMSeriesResistance")({"voltage as a state": "true"}) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py index f4d572f06c..0ce48b0a3f 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py @@ -15,10 +15,12 @@ from collections.abc import Callable from dataclasses import dataclass from importlib.metadata import PackageNotFoundError, version +from pathlib import Path from typing import Any import numpy as np from packaging.requirements import Requirement +from packaging.utils import canonicalize_name from packaging.version import Version import pybamm @@ -346,14 +348,7 @@ def _check_extra_is_declared(entry: ModelEntry) -> None: assert extra in extras, ( f"{pyproject}: no '{extra}' extra, but {entry.manifest_path} declares one" ) - declared = {Requirement(item).name for item in extras[extra]} - missing = sorted( - {Requirement(item).name for item in entry.dependencies.packages} - declared - ) - assert not missing, ( - f"{pyproject}: extra '{extra}' is missing {missing}, declared by " - f"{entry.manifest_path}" - ) + _check_requirements_agree(entry, pyproject, extras[extra]) aggregated: set[str] = set() for item in extras.get("zoo-all", []): requirement = Requirement(item) @@ -365,6 +360,59 @@ def _check_extra_is_declared(entry: ModelEntry) -> None: ) +def _check_requirements_agree( + entry: ModelEntry, pyproject: Path, extra_items: list[str] +) -> None: + """A manifest and the extra behind it declare the same requirements. + + Compared in both directions, and on the specifier rather than the name + alone. CI installs every model's extra at once, so a one-way name-only + comparison lets a model lean on a package it never declared, or claim a + stronger bound than the extra actually installs, and still pass. + """ + declared = _requirements(entry.dependencies.packages) + provided = _requirements(extra_items) + extra = entry.dependencies.extra + missing = sorted(set(declared) - set(provided)) + assert not missing, ( + f"{pyproject}: extra '{extra}' is missing {missing}, declared by " + f"{entry.manifest_path}" + ) + undeclared = sorted(set(provided) - set(declared)) + assert not undeclared, ( + f"{entry.manifest_path}: [model.dependencies].packages does not declare " + f"{undeclared}, which extra '{extra}' installs" + ) + disagreeing = sorted( + name + for name, requirement in declared.items() + if _shape(requirement) != _shape(provided[name]) + ) + assert not disagreeing, ( + f"{entry.manifest_path}: {disagreeing} declared differently here than in " + f"extra '{extra}' of {pyproject}; the manifest is what the compatibility " + f"table reports, so the two have to say the same thing" + ) + + +def _requirements(items: tuple[str, ...] | list[str]) -> dict[str, Requirement]: + """Requirement specifications, keyed by canonical distribution name.""" + parsed = {} + for item in items: + requirement = Requirement(item) + parsed[canonicalize_name(requirement.name)] = requirement + return parsed + + +def _shape(requirement: Requirement) -> tuple: + """What a requirement asks for, with the name left out: it is the key.""" + return ( + requirement.specifier, + frozenset(requirement.extras), + str(requirement.marker or ""), + ) + + @_register("docs", scope=REPO) def check_docs(entry: ModelEntry) -> None: """The generated docs page for the model is present and current.""" diff --git a/packages/pybamm-model-zoo/template/model.py.in b/packages/pybamm-model-zoo/template/model.py.in index b12795bafb..f9d2603f80 100644 --- a/packages/pybamm-model-zoo/template/model.py.in +++ b/packages/pybamm-model-zoo/template/model.py.in @@ -14,8 +14,8 @@ class ${ModelName}(pybamm.lithium_ion.SPM): As shipped this is plain SPM, so the contract suite passes from the first commit. Add your physics by overriding the submodel setters you need — see - ``spm_series_resistance`` for a worked example, and the PyBaMM developer - docs for the submodel protocol. + ``linearised_spm`` for a worked example, and the PyBaMM developer docs for + the submodel protocol. Parameters ---------- diff --git a/packages/pybamm-model-zoo/tests/test_contract.py b/packages/pybamm-model-zoo/tests/test_contract.py index 4e2a274165..33e563fd4c 100644 --- a/packages/pybamm-model-zoo/tests/test_contract.py +++ b/packages/pybamm-model-zoo/tests/test_contract.py @@ -9,6 +9,7 @@ import pybamm_model_zoo as zoo from pybamm_model_zoo import _docs +from pybamm_model_zoo._registry import ModelEntry from pybamm_model_zoo.testing import contract # An externally-registered model is held only to the portable rules: it is not @@ -61,6 +62,43 @@ def test_every_check_is_well_formed(self): ) +class TestDependencyAgreement: + """A manifest and the extra behind it, held to each other in both directions.""" + + def check(self, tmp_path, packages, extra_items): + entry = ModelEntry( + slug="a_model", + name="AModel", + path=tmp_path, + raw={ + "model": { + "dependencies": {"extra": "zoo-a-model", "packages": packages} + } + }, + ) + contract._check_requirements_agree( + entry, tmp_path / "pyproject.toml", extra_items + ) + + def test_matching_requirements_pass(self, tmp_path): + self.check(tmp_path, ["scikit-fem>=12.0.2"], ["scikit-fem>=12.0.2"]) + + def test_a_package_the_extra_omits_is_caught(self, tmp_path): + with pytest.raises(AssertionError, match=r"is missing \['scikit-fem'\]"): + self.check(tmp_path, ["scikit-fem>=12.0.2"], []) + + def test_a_package_the_manifest_omits_is_caught(self, tmp_path): + with pytest.raises(AssertionError, match=r"does not declare \['scikit-fem'\]"): + self.check(tmp_path, [], ["scikit-fem>=12.0.2"]) + + def test_a_disagreeing_constraint_is_caught(self, tmp_path): + with pytest.raises(AssertionError, match=r"declared differently"): + self.check(tmp_path, ["scikit-fem>=13"], ["scikit-fem>=12.0.2"]) + + def test_names_are_compared_canonically(self, tmp_path): + self.check(tmp_path, ["Scikit_FEM>=12.0.2"], ["scikit-fem>=12.0.2"]) + + class TestGeneratedFiles: """The index page and the absence of leftovers, which no per-model check sees.""" diff --git a/packages/pybamm-model-zoo/tests/test_registry.py b/packages/pybamm-model-zoo/tests/test_registry.py index cff6747d30..a0409af194 100644 --- a/packages/pybamm-model-zoo/tests/test_registry.py +++ b/packages/pybamm-model-zoo/tests/test_registry.py @@ -43,9 +43,9 @@ def write_model(root: Path, slug: str, name: str, body: str | None = None) -> Pa class TestRegistry: def test_discovers_the_reference_model(self): - assert "SPMSeriesResistance" in zoo.list_models() - entry = zoo.info("SPMSeriesResistance") - assert entry.slug == "spm_series_resistance" + assert "LinearisedSPM" in zoo.list_models() + entry = zoo.info("LinearisedSPM") + assert entry.slug == "linearised_spm" assert entry.tier == "core" assert entry.maintainers[0].github == "pybamm-team/maintainers" @@ -86,14 +86,28 @@ def test_duplicate_names_are_rejected(self, tmp_path): with pytest.raises(zoo.ManifestError, match=r"duplicate model name"): Registry([tmp_path]) - def test_external_models_do_not_shadow_in_tree_ones(self, tmp_path): + def test_duplicate_slugs_are_rejected(self, tmp_path): + one, two = tmp_path / "one", tmp_path / "two" + write_model(one, "same_slug", "OneName") + write_model(two, "same_slug", "OtherName") + with pytest.raises(zoo.ManifestError, match=r"duplicate model slug"): + Registry([one, two]) + + @pytest.mark.parametrize( + ("slug", "name"), + [("minimal_model", "MinimalModel"), ("minimal_model", "Different")], + ) + def test_external_models_do_not_shadow_in_tree_ones(self, tmp_path, slug, name): + """Neither key may be shadowed: `by_slug` is what resolves citations.""" in_tree = tmp_path / "in_tree" external = tmp_path / "external" write_model(in_tree, "minimal_model", "MinimalModel") - write_model(external, "minimal_model", "MinimalModel") + write_model(external, slug, name) with pytest.warns(UserWarning, match=r"ignoring external model"): registry = Registry([in_tree], external_paths=[external]) - assert registry["MinimalModel"].path.parent == in_tree + assert registry.by_slug("minimal_model").path.parent == in_tree + assert not registry.by_slug("minimal_model").external + assert name not in registry or registry[name].path.parent == in_tree def test_external_entries_are_flagged(self, tmp_path): write_model(tmp_path, "minimal_model", "MinimalModel") @@ -102,8 +116,8 @@ def test_external_entries_are_flagged(self, tmp_path): class TestLoad: def test_load_returns_the_class(self): - model_class = zoo.load("SPMSeriesResistance") - assert model_class.__name__ == "SPMSeriesResistance" + model_class = zoo.load("LinearisedSPM") + assert model_class.__name__ == "LinearisedSPM" def test_unparseable_class_path(self, tmp_path): body = MANIFEST.format(slug="minimal_model", name="MinimalModel").replace( @@ -175,9 +189,9 @@ def test_parses_multiple_entries(self): assert entries["A2020"].endswith("}") def test_reference_model_citation_resolves(self): - entry = zoo.info("SPMSeriesResistance") + entry = zoo.info("LinearisedSPM") assert entry.citation_key in zoo.read_citations(entry.path) def test_register_citation_rejects_an_unknown_key(self): with pytest.raises(zoo.ManifestError, match=r"no entry for 'Nope'"): - zoo.register_citation("spm_series_resistance", "Nope") + zoo.register_citation("linearised_spm", "Nope") diff --git a/packages/pybamm-model-zoo/tests/test_status.py b/packages/pybamm-model-zoo/tests/test_status.py new file mode 100644 index 0000000000..caf8b34b91 --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_status.py @@ -0,0 +1,94 @@ +"""The weekly compatibility status: folding matrix results into badges.""" + +import json + +from pybamm_model_zoo import _docs + + +def write_results(directory, records): + directory.mkdir(exist_ok=True) + for record in records: + path = directory / f"{record['model']}--{record['version']}.json" + path.write_text(json.dumps(record), encoding="utf-8") + return directory + + +class TestCollectResults: + def test_folds_one_file_per_cell(self, tmp_path): + results = write_results( + tmp_path / "results", + [ + {"model": "a_model", "version": "26.7.0", "result": "pass"}, + {"model": "a_model", "version": "main", "result": "fail"}, + ], + ) + status = _docs.collect_results(results) + assert status["models"]["a_model"]["results"] == { + "26.7.0": "pass", + "main": "fail", + } + assert status["models"]["a_model"]["last_pass"] == "26.7.0" + + def test_an_expected_cell_that_reported_nothing_is_recorded(self, tmp_path): + results = write_results( + tmp_path / "results", + [{"model": "a_model", "version": "26.7.0", "result": "pass"}], + ) + expected = [ + {"model": "a_model", "version": "26.7.0"}, + {"model": "a_model", "version": "26.8.0"}, + {"model": "b_model", "version": "main"}, + ] + status = _docs.collect_results(results, expected=expected) + assert status["models"]["a_model"]["results"]["26.8.0"] == _docs.MISSING + assert status["models"]["b_model"]["results"] == {"main": _docs.MISSING} + + def test_a_reported_cell_is_not_overwritten_as_missing(self, tmp_path): + results = write_results( + tmp_path / "results", + [{"model": "a_model", "version": "main", "result": "fail"}], + ) + status = _docs.collect_results( + results, expected=[{"model": "a_model", "version": "main"}] + ) + assert status["models"]["a_model"]["results"] == {"main": "fail"} + + +class TestBadge: + def test_all_passing_is_green(self): + record = {"results": {"26.7.0": "pass"}, "last_pass": "26.7.0"} + assert _docs.badge(record)["color"] == _docs.BADGE_COLORS["pass"] + + def test_a_missing_cell_does_not_read_as_passing(self): + record = {"results": {"26.7.0": "pass", "main": _docs.MISSING}} + assert _docs.badge(record)["color"] == _docs.BADGE_COLORS["missing"] + assert "main" in _docs.badge(record)["message"] + + def test_a_failure_outranks_a_missing_cell(self): + record = {"results": {"26.7.0": "fail", "main": _docs.MISSING}} + assert _docs.badge(record)["color"] == _docs.BADGE_COLORS["fail"] + assert "failing on 26.7.0" in _docs.badge(record)["message"] + + def test_no_results_at_all_is_untested(self): + assert _docs.badge({})["message"] == "untested" + + +class TestStamp: + RESULTS = {"a_model": {"results": {"main": "pass"}, "last_pass": None}} + + def test_an_unchanged_result_keeps_its_timestamp(self): + previous = {"generated": "2026-01-01T00:00:00Z", "models": self.RESULTS} + stamped = _docs.stamp( + {"models": self.RESULTS}, previous, "2026-02-02T00:00:00Z" + ) + assert stamped["generated"] == "2026-01-01T00:00:00Z" + + def test_a_changed_result_is_restamped(self): + previous = {"generated": "2026-01-01T00:00:00Z", "models": self.RESULTS} + changed = {"a_model": {"results": {"main": "fail"}, "last_pass": None}} + stamped = _docs.stamp({"models": changed}, previous, "2026-02-02T00:00:00Z") + assert stamped["generated"] == "2026-02-02T00:00:00Z" + + def test_a_first_run_is_stamped(self): + stamped = _docs.stamp({"models": {}}, {"models": {}}, "2026-02-02T00:00:00Z") + assert stamped["generated"] == "2026-02-02T00:00:00Z" From 87a2c12590e248f57be4bf83945a6d2e9fd492d9 Mon Sep 17 00:00:00 2001 From: bradyplanden Date: Fri, 21 Aug 2026 11:10:51 +0100 Subject: [PATCH 6/7] fix: address review findings on the model zoo Correctness: - `LinearisedSPM` no longer raises `TypeError` under `{"particle size": "distribution"}`: `dUdT` is broadcast onto the potential's domains, which is what the base class size-averages against. - The tangent's slope is taken at `T_ref`, so it is a constant and the published entropic change is the exact temperature derivative of the linearised potential. Previously the slope carried `T`, leaving the reversible heat inconsistent once the stoichiometry moved. - `--zoo-tier` prunes a model's folder before pytest imports anything in it, so a community model that fails to import can no longer abort the merge gate or poison every weekly compatibility cell. Marker deselection alone ran too late. - The compatibility matrix filters releases per model before taking each model's newest N, so a model with an upper bound keeps the released cells it still supports. - `missing_dependencies` evaluates environment markers, so a platform-specific dependency no longer skips every check elsewhere. - Examples run as `__main__`, so a main-guarded example is executed. - The scaffold and the `manifest` check reject Python keywords, which are identifier-shaped but render invalid code. - The generated floor pins major.minor rather than the bare major. Test suite and CI: - Warning filters are strict: the three blanket class ignores were unused. The weekly job downgrades deprecations on released legs only. - The advisory job runs only what the gate does not, rather than every core model twice. - The zoo filter routes `.github/CODEOWNERS`, `model_zoo_status.yml`, and the generated docs, all of which the contract checks cover. - New tests: an inherited-options matrix and a non-isothermal pair for `LinearisedSPM`, collection selection, the release window, environment markers, keyword rejection, and the main-guard. Cleanups: - `_versions.py` owns the release ordering and window that `_docs.py` and `matrix.py` had duplicated. - The registry cache is `_registry_instance`, so it stops shadowing the `_registry` submodule. - The docs landing page and README give a supported install path. - Both changelog bullets end with the pull request link. --- .github/workflows/model_zoo_status.yml | 8 ++- .github/workflows/test_on_push.yml | 13 +++- CHANGELOG.md | 2 +- docs/source/model_zoo/index.md | 10 ++- noxfile.py | 31 +++++++-- packages/pybamm-model-zoo/CHANGELOG.md | 2 +- packages/pybamm-model-zoo/README.md | 19 +++++- packages/pybamm-model-zoo/conftest.py | 41 +++++++++++ packages/pybamm-model-zoo/pyproject.toml | 9 +-- packages/pybamm-model-zoo/scripts/matrix.py | 48 ++++--------- .../pybamm-model-zoo/scripts/new_model.py | 10 +++ .../src/pybamm_model_zoo/__init__.py | 18 ++--- .../src/pybamm_model_zoo/_docs.py | 19 +++--- .../src/pybamm_model_zoo/_registry.py | 11 +++ .../src/pybamm_model_zoo/_template.py | 23 +++++-- .../src/pybamm_model_zoo/_versions.py | 46 +++++++++++++ .../pybamm_model_zoo/linearised_spm/model.py | 15 ++-- .../tests/test_linearised_spm.py | 68 +++++++++++++++++++ .../src/pybamm_model_zoo/testing/contract.py | 14 ++-- .../pybamm-model-zoo/tests/test_contract.py | 36 ++++++++++ .../pybamm-model-zoo/tests/test_examples.py | 21 +++++- .../pybamm-model-zoo/tests/test_selection.py | 64 +++++++++++++++++ .../pybamm-model-zoo/tests/test_template.py | 34 ++++++++++ .../pybamm-model-zoo/tests/test_versions.py | 63 +++++++++++++++++ 24 files changed, 534 insertions(+), 91 deletions(-) create mode 100644 packages/pybamm-model-zoo/src/pybamm_model_zoo/_versions.py create mode 100644 packages/pybamm-model-zoo/tests/test_selection.py create mode 100644 packages/pybamm-model-zoo/tests/test_versions.py diff --git a/.github/workflows/model_zoo_status.yml b/.github/workflows/model_zoo_status.yml index 7cfb7b9111..d7ba98759a 100644 --- a/.github/workflows/model_zoo_status.yml +++ b/.github/workflows/model_zoo_status.yml @@ -105,9 +105,15 @@ jobs: echo "$CELLS" | uv run --no-sync python -c \ 'import json,os,sys; print("\n".join(c["model"] for c in json.load(sys.stdin) if c["version"] == os.environ["VERSION"]))' \ > models.txt + # A frozen release can warn where `main` no longer does, which is no + # zoo regression, so only released legs downgrade. `main` stays strict. + filters=() + if [ "$VERSION" != main ]; then + filters=(-W default::DeprecationWarning -W default::PendingDeprecationWarning) + fi while read -r model; do [ -n "$model" ] || continue - if uv run --no-sync python -m pytest -m zoo \ + if uv run --no-sync python -m pytest -m zoo "${filters[@]}" \ packages/pybamm-model-zoo --zoo-model="$model"; then result=pass else diff --git a/.github/workflows/test_on_push.yml b/.github/workflows/test_on_push.yml index e60d09e123..411ccd7da4 100644 --- a/.github/workflows/test_on_push.yml +++ b/.github/workflows/test_on_push.yml @@ -72,6 +72,13 @@ jobs: model_zoo: - 'packages/pybamm-model-zoo/**' - '.github/workflows/test_on_push.yml' + # Ownership is a contract check, so dropping an owner line has to + # run the check that would have caught it. + - '.github/CODEOWNERS' + # The weekly job consumes the matrix and the docs generator, and + # nothing else validates it on a pull request. + - '.github/workflows/model_zoo_status.yml' + - 'docs/source/model_zoo/**' # Map "what changed" to the solver wheels to build: solver PRs validate every # platform; pybamm/push need the sparse matrix's runners; docs-only needs Linux. @@ -305,8 +312,8 @@ jobs: timeout_minutes: 30 legs: '[{"os": "ubuntu-latest", "python": "3.13"}]' - # Advisory like run_unit_tests_advisory above: the whole zoo runs, `community` - # models included, and reports red without blocking a merge. + # Advisory like run_unit_tests_advisory above: `community` reports red without + # blocking. The gate above covers `core`, so this runs only what it does not. run_zoo_tests_advisory: needs: [changes, build_solver] if: ${{ needs.changes.outputs.run_zoo_tests == 'true' }} @@ -315,7 +322,7 @@ jobs: contents: read uses: ./.github/workflows/_nox.yml with: - sessions: zoo + sessions: zoo-community texlive: false fetch_depth: 0 timeout_minutes: 60 diff --git a/CHANGELOG.md b/CHANGELOG.md index bd67e540ae..1ab35d02bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Features -- Added the model zoo (`packages/pybamm-model-zoo/`), a home for community- and partner-contributed models: one self-contained folder per model, with a declarative `model.toml` manifest as the only boilerplate a contributor writes. Models are either `community` tier (advisory CI) or `core` tier (in the merge gate); nothing in `pybamm` itself changed. ([#5727](https://github.com/pybamm-team/PyBaMM/issues/5727)) +- Added the model zoo (`packages/pybamm-model-zoo/`), a home for community- and partner-contributed models: one self-contained folder per model, with a declarative `model.toml` manifest as the only boilerplate a contributor writes. Models are either `community` tier (advisory CI) or `core` tier (in the merge gate); nothing in `pybamm` itself changed. ([#5727](https://github.com/pybamm-team/PyBaMM/pull/5727)) ## Bug fixes diff --git a/docs/source/model_zoo/index.md b/docs/source/model_zoo/index.md index 7870a4bc55..0d0899fa42 100644 --- a/docs/source/model_zoo/index.md +++ b/docs/source/model_zoo/index.md @@ -8,7 +8,15 @@ Community- and partner-contributed PyBaMM models. Each entry is one self-contained folder with its own maintainer, tests, examples, and citation; the table below is generated from those folders' manifests. -Zoo models are reached through the zoo's registry, not the `pybamm` +The zoo is a separate distribution that is **not yet published to PyPI**, so +`pip install pybamm` does not provide it. Until it is released, install it +from the repository: + +```bash +pip install "pybamm-model-zoo @ git+https://github.com/pybamm-team/PyBaMM.git#subdirectory=packages/pybamm-model-zoo" +``` + +Zoo models are then reached through the zoo's registry, not the `pybamm` namespace: ```python diff --git a/noxfile.py b/noxfile.py index ebafeaa36e..b5458cacee 100644 --- a/noxfile.py +++ b/noxfile.py @@ -328,9 +328,21 @@ def install_zoo(session): ) -def zoo_pytest(session, marker): +def zoo_pytest(session, marker, *args, allow_empty=False): """Run the zoo suite, selecting by marker.""" - session.run("python", "-m", "pytest", "-m", marker, ZOO_TESTS, *session.posargs) + session.run( + "python", + "-m", + "pytest", + "-m", + marker, + *args, + ZOO_TESTS, + *session.posargs, + # 5 is "collected nothing", which is the honest state of the community + # tier until someone contributes to it, not a failure. + success_codes=[0, 5] if allow_empty else [0], + ) @nox.session(name="zoo", default=False) @@ -342,9 +354,20 @@ def run_zoo(session): @nox.session(name="zoo-gating", default=False) def run_zoo_gating(session): - """Run what is in PyBaMM's merge gate: `core`-tier models and the zoo itself.""" + """Run what is in PyBaMM's merge gate: `core`-tier models and the zoo itself. + + `--zoo-tier` keeps every other tier out of collection entirely, so a model + the gate does not cover cannot break the gate by failing to import. + """ + install_zoo(session) + zoo_pytest(session, "zoo and gating", "--zoo-tier=core") + + +@nox.session(name="zoo-community", default=False) +def run_zoo_community(session): + """Run the advisory half: everything the merge gate does not already cover.""" install_zoo(session) - zoo_pytest(session, "zoo and gating") + zoo_pytest(session, "zoo and not gating", allow_empty=True) @nox.session(name="zoo-examples", default=False) diff --git a/packages/pybamm-model-zoo/CHANGELOG.md b/packages/pybamm-model-zoo/CHANGELOG.md index c05c4e17f0..46dfeffa12 100644 --- a/packages/pybamm-model-zoo/CHANGELOG.md +++ b/packages/pybamm-model-zoo/CHANGELOG.md @@ -9,4 +9,4 @@ PyBaMM's. - The model zoo: per-model manifests, a registry, a ten-check contract suite, a template and generator, generated docs pages and status badges, and the - `linearised_spm` reference entry ([#5511](https://github.com/pybamm-team/PyBaMM/issues/5511)) + `linearised_spm` reference entry ([#5727](https://github.com/pybamm-team/PyBaMM/pull/5727)) diff --git a/packages/pybamm-model-zoo/README.md b/packages/pybamm-model-zoo/README.md index 5bb2476bcc..ec0e538dbf 100644 --- a/packages/pybamm-model-zoo/README.md +++ b/packages/pybamm-model-zoo/README.md @@ -23,8 +23,14 @@ print(solution["Voltage [V]"](150)) ``` The zoo is a `uv` workspace member, so `uv sync --extra all --group dev` from the -repository root installs it editable alongside `pybamm`. It is not published to -PyPI. +repository root installs it editable alongside `pybamm`. + +It is not published to PyPI yet, so `pip install pybamm` does not provide it. +Until it is released, users install it from the repository: + +```bash +pip install "pybamm-model-zoo @ git+https://github.com/pybamm-team/PyBaMM.git#subdirectory=packages/pybamm-model-zoo" +``` ## Adding a model @@ -88,7 +94,8 @@ Advisory-ness lives in the CI job, never in an `xfail` marker: a community model that fails, fails, and that is exactly the signal its badge reports. Being advisory is what lets the zoo test contributed models on every pull request without a contributed model ever blocking a PyBaMM release. The manifest tier is -what puts a model's tests behind the `gating` marker. +what puts a model's tests behind the `gating` marker, and what `--zoo-tier=core` +keeps out of the merge gate's collection entirely. ## Testing @@ -131,10 +138,16 @@ in the manifest diff. The `manifest` check itself cannot be waived. nox -s zoo # the whole zoo, both tiers, plus examples nox -s zoo -- --zoo-model=my_model # one model nox -s zoo-gating # only core-tier models (the merge gate) +nox -s zoo-community # only what the gate does not cover (advisory CI) nox -s zoo-examples # every model's example scripts nox -s zoo-docs # regenerate docs pages and badges ``` +`--zoo-model` and `--zoo-tier` drop the folders they exclude *before* pytest +imports anything in them, so a model that fails to import can only fail its own +run. That is what keeps the merge gate independent of the community tier: marker +deselection alone would not, since it happens after every test module is loaded. + Aim for contract checks under 60 s per model and a model's own tests under 5 minutes; `solve_time` in the manifest is the lever. diff --git a/packages/pybamm-model-zoo/conftest.py b/packages/pybamm-model-zoo/conftest.py index 7abb9800db..397e6ac0c8 100644 --- a/packages/pybamm-model-zoo/conftest.py +++ b/packages/pybamm-model-zoo/conftest.py @@ -26,6 +26,47 @@ def pytest_addoption(parser): "pull request changed" ), ) + parser.addoption( + "--zoo-tier", + action="store", + default=None, + choices=zoo.TIERS, + metavar="TIER", + help=( + "keep only the models of one tier in collection, so the merge gate " + "cannot be broken by a model outside it" + ), + ) + + +def _model_folders(): + """Every registered model's folder, with its slug and tier. + + Manifests are parsed rather than imported, so this is safe to call before a + single test module has been loaded. + """ + return [ + (entry.path.resolve(), entry.slug, entry.tier) for entry in zoo.all_entries() + ] + + +def pytest_ignore_collect(collection_path, config): + """Drop a model's folder before pytest imports anything inside it. + + It does not replace the marker: the contract suite is parametrized over every + model from a single module, so those cases are filtered by marker instead. + """ + selected = config.getoption("--zoo-model") + tier = config.getoption("--zoo-tier") + if selected is None and tier is None: + return None + for folder, slug, model_tier in _model_folders(): + if collection_path != folder and folder not in collection_path.parents: + continue + if selected is not None and slug != selected: + return True + return True if tier is not None and model_tier != tier else None + return None def _slug_from_path(path): diff --git a/packages/pybamm-model-zoo/pyproject.toml b/packages/pybamm-model-zoo/pyproject.toml index dbdff9a438..b794efc368 100644 --- a/packages/pybamm-model-zoo/pyproject.toml +++ b/packages/pybamm-model-zoo/pyproject.toml @@ -55,12 +55,9 @@ markers = [ "unit: mark test as a unit test", "integration: mark test as an integration test", ] -filterwarnings = [ - "error", - "ignore::DeprecationWarning", - "ignore::UserWarning", - "ignore::RuntimeWarning", -] +# No blanket class ignores: this suite is small enough to keep clean. The weekly +# job relaxes them for legacy releases, where a warning is not a zoo regression. +filterwarnings = ["error"] log_cli = true log_level = "INFO" log_format = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" diff --git a/packages/pybamm-model-zoo/scripts/matrix.py b/packages/pybamm-model-zoo/scripts/matrix.py index 40ce153b44..76b997191d 100644 --- a/packages/pybamm-model-zoo/scripts/matrix.py +++ b/packages/pybamm-model-zoo/scripts/matrix.py @@ -1,8 +1,9 @@ """Emit the model zoo compatibility matrix for the weekly status workflow. Prints a GitHub Actions ``include`` list: one ``{model, version}`` cell per pair. -A pair a manifest's ``pybamm_requires`` excludes is left out, so a badge never -reports "failing" on a release the model never claimed to support. +Each model is paired with the newest releases its own ``pybamm_requires`` admits, +so a model with an upper bound still gets tested against the releases it does +support, and a badge never reports "failing" on a release it never claimed. uv run --with packaging python packages/pybamm-model-zoo/scripts/matrix.py """ @@ -11,7 +12,6 @@ import argparse import json -import re import sys import urllib.request from pathlib import Path @@ -19,44 +19,24 @@ sys.path.insert(0, str(Path(__file__).parents[1] / "src")) from pybamm_model_zoo._registry import Registry +from pybamm_model_zoo._versions import MAIN, sorted_releases, window_for PYPI_URL = "https://pypi.org/pypi/pybamm/json" -#: Final CalVer releases only: no prereleases, no yanked-empty entries. -CALVER = re.compile(r"^\d+(\.\d+)*$") -#: The checkout itself, which has no release number to match a specifier against. -MAIN = "main" -def version_order(version: str) -> tuple[int, list[int]]: - """Sort releases numerically, and sort ``main`` last.""" - try: - return (0, [int(part) for part in version.split(".")]) - except ValueError: - return (1, []) - - -def released_versions(count: int) -> list[str]: - """The ``count`` most recent final PyBaMM releases on PyPI, oldest first.""" +def released_versions() -> list[str]: + """Every final PyBaMM release on PyPI, oldest first.""" with urllib.request.urlopen(PYPI_URL) as response: # nosec B310 - literal https URL releases = json.load(response)["releases"] - published = sorted( - ( - version - for version, files in releases.items() - if files and CALVER.match(version) - ), - key=version_order, - ) - return published[-count:] + return sorted_releases(version for version, files in releases.items() if files) -def matrix(versions: list[str]) -> list[dict[str, str]]: - """One cell per (model, version) pair the model's declared range admits.""" +def matrix(releases: list[str], count: int) -> list[dict[str, str]]: + """One cell per model per release in that model's own window, plus ``main``.""" cells = [] for entry in sorted(Registry().values(), key=lambda entry: entry.slug): - for version in versions: - if version == MAIN or entry.admits(version): - cells.append({"model": entry.slug, "version": version}) + for version in [*window_for(entry, releases, count), MAIN]: + cells.append({"model": entry.slug, "version": version}) return cells @@ -66,7 +46,7 @@ def main(argv: list[str] | None = None) -> int: "--releases", type=int, default=2, - help="how many of the most recent PyPI releases to test against", + help="how many of the most recent releases each model is tested against", ) parser.add_argument( "--github-output", @@ -75,11 +55,11 @@ def main(argv: list[str] | None = None) -> int: ) args = parser.parse_args(argv) - cells = matrix([*released_versions(args.releases), MAIN]) + cells = matrix(released_versions(), args.releases) if args.github_output: # The workflow matrixes on version and loops models inside the cell, so # it needs the versions that survived filtering as well as the pairs. - versions = sorted({cell["version"] for cell in cells}, key=version_order) + versions = [*sorted_releases({cell["version"] for cell in cells}), MAIN] print(f"include={json.dumps(cells)}") print(f"versions={json.dumps(versions)}") else: diff --git a/packages/pybamm-model-zoo/scripts/new_model.py b/packages/pybamm-model-zoo/scripts/new_model.py index 5626ace384..be6eb05142 100644 --- a/packages/pybamm-model-zoo/scripts/new_model.py +++ b/packages/pybamm-model-zoo/scripts/new_model.py @@ -29,6 +29,15 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: parser.add_argument("--github", required=True, help="maintainer's GitHub handle") parser.add_argument("--tier", default="community", choices=TIERS) parser.add_argument("--license", default="BSD-3-Clause", help="SPDX identifier") + parser.add_argument( + "--pybamm-requires", + default=None, + metavar="SPECIFIER", + help=( + "the PyBaMM versions this model supports, e.g. '>=26.8'; defaults to a " + "floor of the installed release" + ), + ) parser.add_argument( "--dry-run", action="store_true", help="report what would be written" ) @@ -56,6 +65,7 @@ def main(argv: list[str] | None = None) -> int: github=args.github, tier=args.tier, license=args.license, + pybamm_requires=args.pybamm_requires, ) destination = PACKAGE_ROOT / args.slug if args.dry_run: diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py index 1b7a762c7b..ad5897ad12 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/__init__.py @@ -49,15 +49,17 @@ "registry", ] -_registry: Registry | None = None +# Not `_registry`: that name is the submodule imported above, and rebinding it +# here would leave `pybamm_model_zoo._registry` pointing at this cache. +_registry_instance: Registry | None = None def registry() -> Registry: """Return the model registry, building it on first use.""" - global _registry - if _registry is None: - _registry = Registry() - return _registry + global _registry_instance + if _registry_instance is None: + _registry_instance = Registry() + return _registry_instance def refresh( @@ -74,9 +76,9 @@ def refresh( Directories of third-party model folders. Defaults to those advertised through the ``pybamm_zoo_models`` entry point. """ - global _registry - _registry = Registry(paths, external_paths=external_paths) - return _registry + global _registry_instance + _registry_instance = Registry(paths, external_paths=external_paths) + return _registry_instance def list_models() -> list[str]: diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py index 1c665a0fd7..833ebf3508 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_docs.py @@ -15,6 +15,7 @@ from pybamm_model_zoo._paths import BADGES_DIR, DOCS_DIR, STATUS_FILE from pybamm_model_zoo._registry import ModelEntry +from pybamm_model_zoo._versions import version_key MODELS_DIR = DOCS_DIR / "models" GENERATED_BY = ( @@ -45,7 +46,15 @@ self-contained folder with its own maintainer, tests, examples, and citation; the table below is generated from those folders' manifests. -Zoo models are reached through the zoo's registry, not the `pybamm` +The zoo is a separate distribution that is **not yet published to PyPI**, so +`pip install pybamm` does not provide it. Until it is released, install it +from the repository: + +```bash +pip install "pybamm-model-zoo @ git+https://github.com/pybamm-team/PyBaMM.git#subdirectory=packages/pybamm-model-zoo" +``` + +Zoo models are then reached through the zoo's registry, not the `pybamm` namespace: ```python @@ -88,14 +97,6 @@ """ -def version_key(version: str) -> tuple[int, list[int]]: - """Sort CalVer releases numerically, and sort anything else (``main``) last.""" - try: - return (0, [int(part) for part in version.split(".")]) - except ValueError: - return (1, []) - - def read_status(path: Path | None = None) -> dict: """The committed compatibility results, or an empty set of them.""" path = path or STATUS_FILE diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py index 858ec97683..85d3276d25 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_registry.py @@ -11,6 +11,7 @@ from __future__ import annotations import importlib +import keyword import re import sys import warnings @@ -37,6 +38,16 @@ NAME_PATTERN = re.compile(r"^[A-Za-z][A-Za-z0-9_]*$") +def usable_identifier(value: str, pattern: re.Pattern[str]) -> bool: + """Whether ``value`` fits ``pattern`` and can be written in code as itself. + + The patterns describe the shape of an identifier but cannot exclude a + keyword, and a slug becomes a module name while a name becomes a class name, + so ``class`` would pass the shape check and render a syntax error. + """ + return bool(pattern.match(value)) and not keyword.iskeyword(value) + + def split_class_path(class_path: str) -> tuple[str, str]: """Split a manifest's ``module.path:AttributeName`` into its two halves. diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py index 28ad19a584..7f78abad5e 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_template.py @@ -18,7 +18,11 @@ from pybamm_model_zoo._exceptions import ZooError from pybamm_model_zoo._paths import TEMPLATE_ROOT, codeowners_folder -from pybamm_model_zoo._registry import NAME_PATTERN, SLUG_PATTERN +from pybamm_model_zoo._registry import ( + NAME_PATTERN, + SLUG_PATTERN, + usable_identifier, +) TEMPLATE_SUFFIX = ".in" #: Matches what ``string.Template`` would substitute, for asserting none is left. @@ -37,7 +41,7 @@ def template_root() -> Path: def default_pybamm_requires() -> str: - """A floor of the installed PyBaMM's major version — what it was written for. + """A floor of the installed PyBaMM release — what the model was written for. Reads the distribution metadata rather than importing PyBaMM, so scaffolding a model does not pay for an import it has no other use for. @@ -52,7 +56,8 @@ def default_pybamm_requires() -> str: "this model was written against. Install it, or pass " "--pybamm-requires explicitly." ) from error - return f">={Version(installed).major}" + parsed = Version(installed) + return f">={parsed.major}.{parsed.minor}" def citation_key_for(author: str, year: int) -> str: @@ -75,11 +80,15 @@ def tokens( license: str = "BSD-3-Clause", ) -> dict[str, str]: """Build the substitution map, validating the contributor's inputs.""" - if not SLUG_PATTERN.match(slug): - raise ZooError(f"slug '{slug}' must be lower_snake_case, e.g. 'my_new_model'") - if not NAME_PATTERN.match(name): + if not usable_identifier(slug, SLUG_PATTERN): raise ZooError( - f"name '{name}' must be a valid Python identifier, e.g. 'MyModel'" + f"slug '{slug}' must be lower_snake_case and not a Python keyword, " + f"e.g. 'my_new_model'" + ) + if not usable_identifier(name, NAME_PATTERN): + raise ZooError( + f"name '{name}' must be a valid Python identifier and not a Python " + f"keyword, e.g. 'MyModel'" ) today = datetime.date.today() year = year if year is not None else today.year diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/_versions.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_versions.py new file mode 100644 index 0000000000..db1a2b6c66 --- /dev/null +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/_versions.py @@ -0,0 +1,46 @@ +"""Ordering PyBaMM releases, and choosing which ones a model is tested against. + +Shared by the compatibility matrix the weekly job runs and the status tables the +docs generator renders, so a release window is decided in one tested place +rather than once per consumer. + +Neither PyBaMM nor ``packaging`` is imported at module scope: both consumers run +in environments with no PyBaMM install, and the docs generator has no +``packaging`` either, which only :func:`window_for` reaches for. +""" + +from __future__ import annotations + +import re +from collections.abc import Iterable + +from pybamm_model_zoo._registry import ModelEntry + +#: Final CalVer releases only: no prereleases, no yanked-empty entries. +CALVER = re.compile(r"^\d+(\.\d+)*$") +#: The checkout itself, which has no release number to match a specifier against. +MAIN = "main" + + +def version_key(version: str) -> tuple[int, list[int]]: + """Sort CalVer releases numerically, and sort anything else (``main``) last.""" + try: + return (0, [int(part) for part in version.split(".")]) + except ValueError: + return (1, []) + + +def sorted_releases(versions: Iterable[str]) -> list[str]: + """Every final CalVer release among ``versions``, oldest first.""" + return sorted((v for v in versions if CALVER.match(v)), key=version_key) + + +def window_for(entry: ModelEntry, releases: Iterable[str], count: int) -> list[str]: + """The ``count`` newest releases ``entry`` admits, oldest first. + + The filtering has to come before the window, not after: taking the newest + releases globally and then dropping the ones a model excludes leaves a model + with an upper bound no released cells at all, even though older releases it + does support are right there. + """ + return [release for release in releases if entry.admits(release)][-count:] diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py index 77037d2f84..4b8ccba0d1 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/model.py @@ -27,16 +27,15 @@ def get_coupled_variables(self, variables: dict) -> dict: variables ) sto_0 = self.phase_param.sto_init_av - gradient = self.phase_param.U(sto_0, T).diff(sto_0) - gradient_bulk = self.phase_param.U(sto_0, T_bulk).diff(sto_0) + # At T_ref - not T, as a temperature-dependent slope makes `dUdT` wrong. + gradient = self.phase_param.U(sto_0, self.param.T_ref).diff(sto_0) ocp_surf = self.phase_param.U(sto_0, T) + gradient * (sto_surf - sto_0) - ocp_bulk = self.phase_param.U(sto_0, T_bulk) + gradient_bulk * ( - sto_bulk - sto_0 - ) - # Evaluated at the linearisation point too, so the model stays linear - # in the state when a thermal submodel reads it. + ocp_bulk = self.phase_param.U(sto_0, T_bulk) + gradient * (sto_bulk - sto_0) dUdT = self.phase_param.dUdT(sto_0) + # dU/dT is size-averaged alongside the potential, so it needs its domains. + if ocp_surf.domain and ocp_surf.domain[0].endswith("particle size"): + dUdT = pybamm.FullBroadcast(dUdT, broadcast_domains=ocp_surf.domains) variables.update(self._get_standard_ocp_variables(ocp_surf, ocp_bulk, dUdT)) self._alias_ocp_as_equilibrium(variables) @@ -46,7 +45,7 @@ def get_coupled_variables(self, variables: dict) -> dict: f"{Domain} electrode {phase_name}linearisation open-circuit " "potential [V]": self.phase_param.U(sto_0, T_bulk), f"{Domain} electrode {phase_name}open-circuit potential " - "gradient [V]": gradient_bulk, + "gradient [V]": gradient, } ) return variables diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py index 701db9dc5d..fff1e7f666 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/linearised_spm/tests/test_linearised_spm.py @@ -15,6 +15,17 @@ PULSE = 60.0 #: Radial points needed to resolve the sqrt(D t) boundary layer of a short pulse. PARTICLE_POINTS = 200 +#: Option sets `pybamm.lithium_ion.SPM` accepts, which a subclass must not break. +INHERITED_OPTIONS = [ + {}, + {"particle size": "distribution"}, + {"thermal": "lumped"}, + {"thermal": "x-full"}, + {"surface form": "differential"}, + {"working electrode": "positive"}, + {"particle phases": ("2", "1")}, + {"SEI": "reaction limited"}, +] def half_cell(diffusivity, pulse=PULSE, particle_points=PARTICLE_POINTS): @@ -121,3 +132,60 @@ def test_weppner_huggins_bias_shrinks_with_the_pulse(self): def test_rejects_an_open_circuit_potential_it_would_ignore(self): with pytest.raises(pybamm.OptionError, match=r"open-circuit potential"): zoo.load("LinearisedSPM")({"open-circuit potential": "current sigmoid"}) + + @pytest.mark.parametrize("options", INHERITED_OPTIONS) + def test_builds_under_the_spm_options_it_inherits(self, options): + """Whatever the parent accepts, the subclass must accept. + + Substituting a submodel is easy to get right for the default options and + wrong for every other combination, so this pins the claim the docstring + makes: ordinary SPM options, minus the open-circuit potential. + """ + pybamm.lithium_ion.SPM(options).check_well_posedness() + zoo.load("LinearisedSPM")(options).check_well_posedness() + + @pytest.mark.parametrize("domain", ["Negative", "Positive"]) + def test_the_tangent_slope_carries_no_state(self, domain): + """The slope must be a constant, or the reported entropic change is wrong. + + PyBaMM writes ``U(x, T)`` as ``U_ref(x) + (T - T_ref) dU/dT(x)``, so a + slope differentiated at ``T`` rather than ``T_ref`` puts a temperature in + the tangent. That both makes the potential bilinear in the state and + leaves the true ``d/dT`` of the tangent equal to + ``dU/dT(x_0) + dU/dT'(x_0) (x - x_0)``, while the thermal submodel is + handed only ``dU/dT(x_0)``. A state-free slope is what keeps the two equal. + """ + model = zoo.load("LinearisedSPM")({"thermal": "lumped"}) + gradient = model.variables[ + f"{domain} electrode open-circuit potential gradient [V]" + ] + assert not gradient.has_symbol_of_classes(pybamm.VariableBase) + + def test_reports_the_entropic_change_at_the_linearisation_point(self): + """Non-isothermal: the entropic change is the parameter set's, at x_0. + + Together with :meth:`test_the_tangent_slope_carries_no_state` this pins + the reversible heat: a constant slope makes ``dU/dT(x_0)`` the exact + temperature derivative of the potential the model reports. + """ + model = zoo.load("LinearisedSPM")({"thermal": "lumped"}) + parameter_values = model.default_parameter_values + solution = pybamm.Simulation(model, parameter_values=parameter_values).solve( + [0, 600] + ) + times = np.linspace(0, 600, 10) + for domain in ("negative", "positive"): + capitalised = domain.capitalize() + entropic_change = parameter_values[ + f"{capitalised} electrode OCP entropic change [V.K-1]" + ] + stoichiometry = solution[ + f"{capitalised} electrode linearisation stoichiometry" + ](0.0) + np.testing.assert_allclose( + solution[f"X-averaged {domain} electrode entropic change [V.K-1]"]( + times + ), + entropic_change(stoichiometry), + rtol=1e-10, + ) diff --git a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py index 0ce48b0a3f..77a446df01 100644 --- a/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py +++ b/packages/pybamm-model-zoo/src/pybamm_model_zoo/testing/contract.py @@ -34,6 +34,7 @@ ModelEntry, read_manifest, split_class_path, + usable_identifier, ) #: A portable rule any zoo model must satisfy, wherever it lives. @@ -118,14 +119,15 @@ def check_manifest(entry: ModelEntry) -> None: if unknown := sorted(set(model) - MODEL_KEYS): raise AssertionError(f"{where}: unknown [model] key(s) {unknown}") - assert SLUG_PATTERN.match(entry.slug), ( - f"{where}: slug '{entry.slug}' must be lower_snake_case" + assert usable_identifier(entry.slug, SLUG_PATTERN), ( + f"{where}: slug '{entry.slug}' must be lower_snake_case and not a keyword" ) assert entry.slug == entry.path.name, ( f"{where}: slug '{entry.slug}' must equal the folder name '{entry.path.name}'" ) - assert NAME_PATTERN.match(entry.name), ( - f"{where}: name '{entry.name}' must be a valid Python identifier" + assert usable_identifier(entry.name, NAME_PATTERN), ( + f"{where}: name '{entry.name}' must be a valid Python identifier and not " + f"a keyword" ) for label, value in (("title", entry.title), ("summary", entry.summary)): assert value.strip(), f"{where}: {label} must be a non-empty string" @@ -482,6 +484,10 @@ def missing_dependencies(entry: ModelEntry) -> list[str]: missing = [] for item in entry.dependencies.packages: requirement = Requirement(item) + # A false marker means the package is not expected here, so counting it + # missing would skip the model's checks on every other platform. + if requirement.marker and not requirement.marker.evaluate(): + continue try: installed = Version(version(requirement.name)) except PackageNotFoundError: diff --git a/packages/pybamm-model-zoo/tests/test_contract.py b/packages/pybamm-model-zoo/tests/test_contract.py index 33e563fd4c..9ef6104e32 100644 --- a/packages/pybamm-model-zoo/tests/test_contract.py +++ b/packages/pybamm-model-zoo/tests/test_contract.py @@ -5,6 +5,8 @@ :data:`pybamm_model_zoo.testing.contract.CHECKS` adds a row. """ +from pathlib import Path + import pytest import pybamm_model_zoo as zoo @@ -99,6 +101,40 @@ def test_names_are_compared_canonically(self, tmp_path): self.check(tmp_path, ["Scikit_FEM>=12.0.2"], ["scikit-fem>=12.0.2"]) +class TestMissingDependencies: + """What counts as "not installed", which gates the import/build/solve checks.""" + + def entry(self, packages): + return ModelEntry( + slug="a_model", + name="AModel", + path=Path("a_model"), + raw={"model": {"dependencies": {"packages": packages}}}, + ) + + def test_an_absent_package_is_missing(self): + assert contract.missing_dependencies( + self.entry(["definitely-not-installed-xyz"]) + ) == ["definitely-not-installed-xyz"] + + def test_an_installed_package_is_not_missing(self): + assert contract.missing_dependencies(self.entry(["packaging>=23.0"])) == [] + + def test_a_requirement_whose_marker_is_false_is_not_missing(self): + """Otherwise a platform-specific dependency skips the checks everywhere else.""" + assert ( + contract.missing_dependencies( + self.entry(['definitely-not-installed-xyz; python_version < "3.0"']) + ) + == [] + ) + + def test_a_requirement_whose_marker_is_true_is_still_checked(self): + assert contract.missing_dependencies( + self.entry(['definitely-not-installed-xyz; python_version >= "3.0"']) + ) == ['definitely-not-installed-xyz; python_version >= "3.0"'] + + class TestGeneratedFiles: """The index page and the absence of leftovers, which no per-model check sees.""" diff --git a/packages/pybamm-model-zoo/tests/test_examples.py b/packages/pybamm-model-zoo/tests/test_examples.py index 3a73b332b7..6a3e61af44 100644 --- a/packages/pybamm-model-zoo/tests/test_examples.py +++ b/packages/pybamm-model-zoo/tests/test_examples.py @@ -8,6 +8,11 @@ from pybamm_model_zoo.testing import contract +def run_example(script): + """Execute an example script the way a user would, as ``__main__``.""" + runpy.run_path(str(script), run_name="__main__") + + def example_scripts(): return [ pytest.param( @@ -27,4 +32,18 @@ class TestExamples: def test_example_script(self, entry, script): if missing := contract.missing_dependencies(entry): pytest.skip(f"{entry.slug}: missing {missing}") - runpy.run_path(str(script)) + run_example(script) + + def test_a_main_guarded_example_is_executed(self, tmp_path): + script = tmp_path / "guarded.py" + ran = tmp_path / "ran.txt" + script.write_text( + "from pathlib import Path\n\n\n" + "def main():\n" + f" Path({str(ran)!r}).write_text('ran')\n\n\n" + 'if __name__ == "__main__":\n' + " main()\n", + encoding="utf-8", + ) + run_example(script) + assert ran.read_text(encoding="utf-8") == "ran" diff --git a/packages/pybamm-model-zoo/tests/test_selection.py b/packages/pybamm-model-zoo/tests/test_selection.py new file mode 100644 index 0000000000..53b9e71f42 --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_selection.py @@ -0,0 +1,64 @@ +"""Which models a run collects, and which it never imports. + +The tiering is only as good as this: marker-based deselection runs after every +test module has been imported, so a model outside the run has to be pruned from +collection or it can still break it. +""" + +from pathlib import Path + +import pytest + +import conftest + +CORE = Path("/zoo/src/pybamm_model_zoo/core_model") +COMMUNITY = Path("/zoo/src/pybamm_model_zoo/community_model") +MACHINERY = Path("/zoo/tests/test_registry.py") + + +class FakeConfig: + """Just the ``getoption`` that :func:`conftest.pytest_ignore_collect` reads.""" + + def __init__(self, **options): + self._options = options + + def getoption(self, name): + return self._options.get(name) + + +@pytest.fixture(autouse=True) +def two_tiers(mocker): + mocker.patch.object( + conftest, + "_model_folders", + return_value=[ + (CORE, "core_model", "core"), + (COMMUNITY, "community_model", "community"), + ], + ) + + +def ignored(path, **options): + return conftest.pytest_ignore_collect(path, FakeConfig(**options)) + + +class TestIgnoreCollect: + def test_an_unfiltered_run_prunes_nothing(self): + assert ignored(COMMUNITY / "tests" / "test_it.py") is None + assert ignored(CORE / "tests" / "test_it.py") is None + + @pytest.mark.parametrize( + ("path", "expected"), + [(COMMUNITY, True), (COMMUNITY / "tests" / "test_it.py", True), (CORE, None)], + ) + def test_a_tier_prunes_the_other_tiers_folders(self, path, expected): + assert ignored(path, **{"--zoo-tier": "core"}) is expected + + @pytest.mark.parametrize(("path", "expected"), [(COMMUNITY, True), (CORE, None)]) + def test_one_model_prunes_the_others(self, path, expected): + assert ignored(path, **{"--zoo-model": "core_model"}) is expected + + def test_the_zoos_own_tests_are_never_pruned(self): + """They are the contract suite and the registry: no model owns them.""" + for options in ({"--zoo-tier": "core"}, {"--zoo-model": "core_model"}): + assert ignored(MACHINERY, **options) is None diff --git a/packages/pybamm-model-zoo/tests/test_template.py b/packages/pybamm-model-zoo/tests/test_template.py index 989bef079a..3fd04fa4b9 100644 --- a/packages/pybamm-model-zoo/tests/test_template.py +++ b/packages/pybamm-model-zoo/tests/test_template.py @@ -7,6 +7,7 @@ template. """ +import re import shutil import subprocess # nosec B404 - runs the repo's own ruff over a rendered template import sys @@ -88,6 +89,39 @@ def test_codeowners_line_names_the_contributor(self): assert line.startswith(_paths.codeowners_folder(SLUG)) +class TestTokenValidation: + """The scaffold refuses inputs it would render into invalid Python.""" + + def tokens(self, **overrides): + return _template.tokens( + **{ + "slug": SLUG, + "name": NAME, + "author": "A. Author", + "github": "ahandle", + **overrides, + } + ) + + @pytest.mark.parametrize("keyword_name", ["class", "import", "lambda", "None"]) + def test_a_python_keyword_is_rejected(self, keyword_name): + """`class` is identifier-shaped, so only a keyword check catches it.""" + with pytest.raises(zoo.ZooError, match=r"keyword"): + self.tokens(slug=keyword_name.lower(), name=keyword_name) + + @pytest.mark.parametrize("soft_keyword", ["match", "case", "type"]) + def test_a_soft_keyword_is_still_a_usable_name(self, soft_keyword): + assert self.tokens(slug=soft_keyword, name=soft_keyword)["slug"] == soft_keyword + + def test_the_default_floor_pins_the_minor_release(self): + """A bare major would let a model claim releases predating its own APIs.""" + requires = _template.default_pybamm_requires() + assert re.fullmatch(r">=\d+\.\d+", requires), requires + + def test_an_explicit_specifier_wins_over_the_default(self): + assert self.tokens(pybamm_requires=">=26.4")["pybamm_requires"] == ">=26.4" + + class TestRenderedStyle: """A rendered template must also pass the repository's style job. diff --git a/packages/pybamm-model-zoo/tests/test_versions.py b/packages/pybamm-model-zoo/tests/test_versions.py new file mode 100644 index 0000000000..b913aa1f1e --- /dev/null +++ b/packages/pybamm-model-zoo/tests/test_versions.py @@ -0,0 +1,63 @@ +"""Release ordering and the per-model compatibility window.""" + +from pathlib import Path + +import pytest + +from pybamm_model_zoo import _versions +from pybamm_model_zoo._exceptions import ManifestError +from pybamm_model_zoo._registry import ModelEntry + +RELEASES = ["25.12.0", "26.0.0", "26.5.0", "26.7.1", "26.8.0"] + + +def entry(pybamm_requires): + return ModelEntry( + slug="a_model", + name="AModel", + path=Path("a_model"), + raw={"model": {"pybamm_requires": pybamm_requires}}, + external=False, + ) + + +class TestSortedReleases: + def test_orders_numerically_not_lexically(self): + assert _versions.sorted_releases(["26.10.0", "26.9.0", "26.8.0"]) == [ + "26.8.0", + "26.9.0", + "26.10.0", + ] + + def test_drops_anything_that_is_not_a_final_calver_release(self): + assert _versions.sorted_releases( + ["26.8.0", "26.9.0rc1", "26.9.0.dev0", _versions.MAIN] + ) == ["26.8.0"] + + +class TestWindowFor: + def test_takes_the_newest_releases_a_model_admits(self): + assert _versions.window_for(entry(">=26.0"), RELEASES, 2) == [ + "26.7.1", + "26.8.0", + ] + + def test_an_upper_bound_keeps_the_releases_below_it(self): + """The bug this guards: filtering after the window leaves no cells at all.""" + assert _versions.window_for(entry(">=26.0,<26.7"), RELEASES, 2) == [ + "26.0.0", + "26.5.0", + ] + + def test_a_window_wider_than_the_admitted_set_is_not_padded(self): + assert _versions.window_for(entry("<26.0"), RELEASES, 3) == ["25.12.0"] + + def test_a_model_admitting_nothing_released_gets_no_cells(self): + assert _versions.window_for(entry(">=99.0"), RELEASES, 2) == [] + + def test_an_empty_specifier_admits_everything(self): + assert _versions.window_for(entry(""), RELEASES, 1) == ["26.8.0"] + + def test_a_malformed_specifier_is_reported_against_the_manifest(self): + with pytest.raises(ManifestError, match=r"not a valid specifier"): + _versions.window_for(entry("=>26.0"), RELEASES, 1) From fe4ee42a36b6de0ebade78feb90f77daa9b4350e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:15:44 +0000 Subject: [PATCH 7/7] style: pre-commit fixes --- packages/pybamm-model-zoo/tests/test_selection.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/pybamm-model-zoo/tests/test_selection.py b/packages/pybamm-model-zoo/tests/test_selection.py index 53b9e71f42..def9fdd7b1 100644 --- a/packages/pybamm-model-zoo/tests/test_selection.py +++ b/packages/pybamm-model-zoo/tests/test_selection.py @@ -7,9 +7,8 @@ from pathlib import Path -import pytest - import conftest +import pytest CORE = Path("/zoo/src/pybamm_model_zoo/core_model") COMMUNITY = Path("/zoo/src/pybamm_model_zoo/community_model")