From 3ee7dd03d58713ff9136327e024f8a014454466a Mon Sep 17 00:00:00 2001 From: Aswinr24 <135364633+Aswinr24@users.noreply.github.com> Date: Mon, 14 Oct 2024 01:33:57 +0530 Subject: [PATCH 1/8] refactor: use pytest fixtures to reduce repetition in tests (#4509) * refactor: use pytest fixtures to reduce repetition in tests * style: pre-commit fixes * refactor: use pytest fixtures to reduce repetition in tests(v2) * refactor: use pytest fixtures to reduce repetition in tests(v2) * refactor: use pytest fixtures to reduce repetition in tests(v2) * style: pre-commit fixes --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../test_process_parameter_data.py | 64 ++++++++----------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/tests/unit/test_parameters/test_process_parameter_data.py b/tests/unit/test_parameters/test_process_parameter_data.py index 9352894c5c..dc363b862b 100644 --- a/tests/unit/test_parameters/test_process_parameter_data.py +++ b/tests/unit/test_parameters/test_process_parameter_data.py @@ -2,56 +2,48 @@ # Tests for the parameter processing functions # - -import os import numpy as np import pybamm - import pytest +from pathlib import Path -class TestProcessParameterData: - def test_process_1D_data(self): - name = "lico2_ocv_example" - path = os.path.abspath(os.path.dirname(__file__)) - processed = pybamm.parameters.process_1D_data(name, path) - assert processed[0] == name - assert isinstance(processed[1], tuple) - assert isinstance(processed[1][0][0], np.ndarray) - assert isinstance(processed[1][1], np.ndarray) +@pytest.fixture +def parameters_path(): + return Path(__file__).parent.resolve() - def test_process_2D_data(self): - name = "lico2_diffusivity_Dualfoil1998_2D" - path = os.path.abspath(os.path.dirname(__file__)) - processed = pybamm.parameters.process_2D_data(name, path) - assert processed[0] == name - assert isinstance(processed[1], tuple) - assert isinstance(processed[1][0][0], np.ndarray) - assert isinstance(processed[1][0][1], np.ndarray) - assert isinstance(processed[1][1], np.ndarray) - def test_process_2D_data_csv(self): - name = "data_for_testing_2D" - path = os.path.abspath(os.path.dirname(__file__)) - processed = pybamm.parameters.process_2D_data_csv(name, path) +@pytest.fixture( + params=[ + ("lico2_ocv_example", pybamm.parameters.process_1D_data), + ("lico2_diffusivity_Dualfoil1998_2D", pybamm.parameters.process_2D_data), + ("data_for_testing_2D", pybamm.parameters.process_2D_data_csv), + ("data_for_testing_3D", pybamm.parameters.process_3D_data_csv), + ] +) +def parameter_data(request, parameters_path): + name, processing_function = request.param + processed = processing_function(name, parameters_path) + return name, processed + +class TestProcessParameterData: + def test_processed_name(self, parameter_data): + name, processed = parameter_data assert processed[0] == name + + def test_processed_structure(self, parameter_data): + name, processed = parameter_data assert isinstance(processed[1], tuple) assert isinstance(processed[1][0][0], np.ndarray) - assert isinstance(processed[1][0][1], np.ndarray) assert isinstance(processed[1][1], np.ndarray) - def test_process_3D_data_csv(self): - name = "data_for_testing_3D" - path = os.path.abspath(os.path.dirname(__file__)) - processed = pybamm.parameters.process_3D_data_csv(name, path) + if len(processed[1][0]) > 1: + assert isinstance(processed[1][0][1], np.ndarray) - assert processed[0] == name - assert isinstance(processed[1], tuple) - assert isinstance(processed[1][0][0], np.ndarray) - assert isinstance(processed[1][0][1], np.ndarray) - assert isinstance(processed[1][0][2], np.ndarray) - assert isinstance(processed[1][1], np.ndarray) + elif len(processed[1]) == 3: + assert isinstance(processed[1][0][1], np.ndarray) + assert isinstance(processed[1][0][2], np.ndarray) def test_error(self): with pytest.raises(FileNotFoundError, match="Could not find file"): From 96d52e62818be72d74c5dd6232f5be8603de3e25 Mon Sep 17 00:00:00 2001 From: "allcontributors[bot]" <46447321+allcontributors[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 03:52:49 +0100 Subject: [PATCH 2/8] docs: add Aswinr24 as a contributor for test (#4511) * docs: update all_contributors.md [skip ci] * docs: update README.md [skip ci] * docs: update .all-contributorsrc [skip ci] --------- Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com> Co-authored-by: Arjun Verma --- .all-contributorsrc | 9 +++++++++ README.md | 2 +- all_contributors.md | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index ae78b29401..dcde101e80 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -982,6 +982,15 @@ "infra", "maintenance" ] + }, + { + "login": "Aswinr24", + "name": "Aswinr24", + "avatar_url": "https://avatars.githubusercontent.com/u/135364633?v=4", + "profile": "https://github.com/Aswinr24", + "contributions": [ + "test" + ] } ], "contributorsPerLine": 7, diff --git a/README.md b/README.md index 8f52b98ed8..2b5250d856 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ [![OpenSSF Scorecard](https://api.scorecard.dev/projects/github.com/pybamm-team/PyBaMM/badge)](https://scorecard.dev/viewer/?uri=github.com/pybamm-team/PyBaMM) -[![All Contributors](https://img.shields.io/badge/all_contributors-92-orange.svg)](#-contributors) +[![All Contributors](https://img.shields.io/badge/all_contributors-93-orange.svg)](#-contributors) diff --git a/all_contributors.md b/all_contributors.md index 9c00e118e3..d4a41ba8e1 100644 --- a/all_contributors.md +++ b/all_contributors.md @@ -124,6 +124,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d Marc Berliner
Marc Berliner
💻 📖 🚇 🚧 + Aswinr24
Aswinr24
⚠️ From f14ebed70fbc18a80592db1f1f2c85d667222c52 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 15 Oct 2024 02:34:21 +0530 Subject: [PATCH 3/8] Build(deps): bump the actions group with 3 updates (#4513) Bumps the actions group with 3 updates: [lycheeverse/lychee-action](https://github.com/lycheeverse/lychee-action), [actions/upload-artifact](https://github.com/actions/upload-artifact) and [github/codeql-action](https://github.com/github/codeql-action). Updates `lycheeverse/lychee-action` from 1.10.0 to 2.0.2 - [Release notes](https://github.com/lycheeverse/lychee-action/releases) - [Commits](https://github.com/lycheeverse/lychee-action/compare/v1.10.0...v2.0.2) Updates `actions/upload-artifact` from 4.4.1 to 4.4.3 - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4.4.1...v4.4.3) Updates `github/codeql-action` from 3.26.12 to 3.26.13 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/c36620d31ac7c881962c3d9dd939c40ec9434f2b...f779452ac5af1c261dce0346a8f964149f49322b) --- updated-dependencies: - dependency-name: lycheeverse/lychee-action dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/upload-artifact dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions - dependency-name: github/codeql-action dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/lychee_url_checker.yml | 2 +- .github/workflows/periodic_benchmarks.yml | 2 +- .github/workflows/publish_pypi.yml | 8 ++++---- .github/workflows/run_benchmarks_over_history.yml | 2 +- .github/workflows/scorecard.yml | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/lychee_url_checker.yml b/.github/workflows/lychee_url_checker.yml index 9a636fda8a..b93743619b 100644 --- a/.github/workflows/lychee_url_checker.yml +++ b/.github/workflows/lychee_url_checker.yml @@ -28,7 +28,7 @@ jobs: # use stable version for now to avoid breaking changes - name: Lychee URL checker - uses: lycheeverse/lychee-action@v1.10.0 + uses: lycheeverse/lychee-action@v2.0.2 with: # arguments with file types to check args: >- diff --git a/.github/workflows/periodic_benchmarks.yml b/.github/workflows/periodic_benchmarks.yml index ef66cec238..14f97c55d8 100644 --- a/.github/workflows/periodic_benchmarks.yml +++ b/.github/workflows/periodic_benchmarks.yml @@ -51,7 +51,7 @@ jobs: LD_LIBRARY_PATH: $HOME/.local/lib - name: Upload results as artifact - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: asv_periodic_results path: results diff --git a/.github/workflows/publish_pypi.yml b/.github/workflows/publish_pypi.yml index 27e6d1162f..64c8a3a777 100644 --- a/.github/workflows/publish_pypi.yml +++ b/.github/workflows/publish_pypi.yml @@ -92,7 +92,7 @@ jobs: python -c "import pybamm; print(pybamm.IDAKLUSolver())" python -m pytest -m cibw {project}/tests/unit - name: Upload Windows wheels - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: wheels_windows path: ./wheelhouse/*.whl @@ -129,7 +129,7 @@ jobs: python -m pytest -m cibw {project}/tests/unit - name: Upload wheels for Linux - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: wheels_manylinux path: ./wheelhouse/*.whl @@ -261,7 +261,7 @@ jobs: python -m pytest -m cibw {project}/tests/unit - name: Upload wheels for macOS (amd64, arm64) - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: wheels_${{ matrix.os }} path: ./wheelhouse/*.whl @@ -281,7 +281,7 @@ jobs: run: pipx run build --sdist - name: Upload SDist - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: sdist path: ./dist/*.tar.gz diff --git a/.github/workflows/run_benchmarks_over_history.yml b/.github/workflows/run_benchmarks_over_history.yml index ce8eb72ce0..d66704a635 100644 --- a/.github/workflows/run_benchmarks_over_history.yml +++ b/.github/workflows/run_benchmarks_over_history.yml @@ -46,7 +46,7 @@ jobs: ${{ github.event.inputs.commit_start }}..${{ github.event.inputs.commit_end }} - name: Upload results as artifact - uses: actions/upload-artifact@v4.4.1 + uses: actions/upload-artifact@v4.4.3 with: name: asv_over_history_results path: results diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index beb849e9fc..cb7e552857 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -59,7 +59,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@604373da6381bf24206979c74d06a550515601b9 # v4.4.1 + uses: actions/upload-artifact@184d73b71b93c222403b2e7f1ffebe4508014249 # v4.4.1 with: name: SARIF file path: results.sarif @@ -68,6 +68,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard (optional). # Commenting out will disable upload of results to your repo's Code Scanning dashboard - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@c36620d31ac7c881962c3d9dd939c40ec9434f2b # v3.26.12 + uses: github/codeql-action/upload-sarif@f779452ac5af1c261dce0346a8f964149f49322b # v3.26.13 with: sarif_file: results.sarif From e5be8a54cf3ec49da8d6def269f60627704654e7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:34:50 -0400 Subject: [PATCH 4/8] chore: update pre-commit hooks (#4515) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/adamchainz/blacken-docs: 1.18.0 → 1.19.0](https://github.com/adamchainz/blacken-docs/compare/1.18.0...1.19.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b66b38b4db..2fc6b76539 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,7 +13,7 @@ repos: types_or: [python, pyi, jupyter] - repo: https://github.com/adamchainz/blacken-docs - rev: "1.18.0" + rev: "1.19.0" hooks: - id: blacken-docs additional_dependencies: [black==23.*] From 9eb4bc7d304dbb6c61b8242231d2e5a65832cd52 Mon Sep 17 00:00:00 2001 From: Pradyot Ranjan <99216956+prady0t@users.noreply.github.com> Date: Tue, 15 Oct 2024 15:43:15 +0530 Subject: [PATCH 5/8] Replacing setUp fixture with setup_method (#4514) Co-authored-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../test_full_battery_models/test_lithium_ion/test_dfn.py | 3 +-- .../test_lithium_ion/test_dfn_half_cell.py | 4 +--- .../test_lithium_ion/test_newman_tobias.py | 3 +-- .../test_full_battery_models/test_lithium_ion/test_spm.py | 3 +-- .../test_lithium_ion/test_spm_half_cell.py | 4 +--- .../test_full_battery_models/test_lithium_ion/test_spme.py | 3 +-- .../test_lithium_ion/test_spme_half_cell.py | 4 +--- tests/unit/test_parameters/test_bpx.py | 3 +-- 8 files changed, 8 insertions(+), 19 deletions(-) diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn.py index cddd59c352..973a0f348b 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn.py @@ -7,8 +7,7 @@ class TestDFN(BaseUnitTestLithiumIon): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.DFN def test_electrolyte_options(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py index 389fcf9429..395c6f54b9 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_dfn_half_cell.py @@ -4,10 +4,8 @@ import pybamm from tests import BaseUnitTestLithiumIonHalfCell -import pytest class TestDFNHalfCell(BaseUnitTestLithiumIonHalfCell): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.DFN diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_newman_tobias.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_newman_tobias.py index ea641ee7cc..58149a69de 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_newman_tobias.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_newman_tobias.py @@ -7,8 +7,7 @@ class TestNewmanTobias(BaseUnitTestLithiumIon): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.NewmanTobias def test_electrolyte_options(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py index 99affc7ddd..8551967dad 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm.py @@ -7,8 +7,7 @@ class TestSPM(BaseUnitTestLithiumIon): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.SPM def test_electrolyte_options(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm_half_cell.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm_half_cell.py index c1b6b34745..8ca0c7a7b9 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm_half_cell.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spm_half_cell.py @@ -3,10 +3,8 @@ # import pybamm from tests import BaseUnitTestLithiumIonHalfCell -import pytest class TestSPMHalfCell(BaseUnitTestLithiumIonHalfCell): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.SPM diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme.py index b0d38fa9c7..ecab4384fc 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme.py @@ -7,8 +7,7 @@ class TestSPMe(BaseUnitTestLithiumIon): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.SPMe # def test_external_variables(self): diff --git a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme_half_cell.py b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme_half_cell.py index 2a814c113e..f09f42a5a6 100644 --- a/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme_half_cell.py +++ b/tests/unit/test_models/test_full_battery_models/test_lithium_ion/test_spme_half_cell.py @@ -4,10 +4,8 @@ # import pybamm from tests import BaseUnitTestLithiumIonHalfCell -import pytest class TestSPMeHalfCell(BaseUnitTestLithiumIonHalfCell): - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.model = pybamm.lithium_ion.SPMe diff --git a/tests/unit/test_parameters/test_bpx.py b/tests/unit/test_parameters/test_bpx.py index aeb249cbc0..3e0e32d1fd 100644 --- a/tests/unit/test_parameters/test_bpx.py +++ b/tests/unit/test_parameters/test_bpx.py @@ -7,8 +7,7 @@ class TestBPX: - @pytest.fixture(autouse=True) - def setUp(self): + def setup_method(self): self.base = { "Header": { "BPX": 1.0, From 47c165d68ed86f623436cb5d243af667ff4dbdb1 Mon Sep 17 00:00:00 2001 From: Robert Timms <43040151+rtimms@users.noreply.github.com> Date: Tue, 15 Oct 2024 15:37:37 +0100 Subject: [PATCH 6/8] Add "voltage as a state" option (#4507) * add voltage as a state option * improve coverage * update CHANGELOG --------- Co-authored-by: Valentin Sulzer --- CHANGELOG.md | 1 + .../full_battery_models/base_battery_model.py | 4 ++++ .../models/submodels/electrode/base_electrode.py | 6 ++++-- .../explicit_control_external_circuit.py | 14 ++++++++++++++ .../function_control_external_circuit.py | 10 ++++++++++ .../test_base_battery_model.py | 12 ++++++++++++ 6 files changed, 45 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a03ec39715..762a23d019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Features +- Adds an option "voltage as a state" that can be "false" (default) or "true". If "true" adds an explicit algebraic equation for the voltage. ([#4507](https://github.com/pybamm-team/PyBaMM/pull/4507)) - Improved `QuickPlot` accuracy for simulations with Hermite interpolation. ([#4483](https://github.com/pybamm-team/PyBaMM/pull/4483)) - Added Hermite interpolation to the (`IDAKLUSolver`) that improves the accuracy and performance of post-processing variables. ([#4464](https://github.com/pybamm-team/PyBaMM/pull/4464)) - Added `BasicDFN` model for sodium-ion batteries ([#4451](https://github.com/pybamm-team/PyBaMM/pull/4451)) diff --git a/src/pybamm/models/full_battery_models/base_battery_model.py b/src/pybamm/models/full_battery_models/base_battery_model.py index 5340d685e3..a445f47d19 100644 --- a/src/pybamm/models/full_battery_models/base_battery_model.py +++ b/src/pybamm/models/full_battery_models/base_battery_model.py @@ -210,6 +210,9 @@ class BatteryModelOptions(pybamm.FuzzyDict): solve an algebraic equation for it. Default is "false", unless "SEI film resistance" is distributed in which case it is automatically set to "true". + * "voltage as a state" : str + Whether to make a state for the voltage and solve an algebraic equation + for it. Default is "false". * "working electrode" : str Can be "both" (default) for a standard battery or "positive" for a half-cell where the negative electrode is replaced with a lithium metal @@ -321,6 +324,7 @@ def __init__(self, extra_options): "heterogeneous catalyst", "cation-exchange membrane", ], + "voltage as a state": ["false", "true"], "working electrode": ["both", "positive"], "x-average side reactions": ["false", "true"], } diff --git a/src/pybamm/models/submodels/electrode/base_electrode.py b/src/pybamm/models/submodels/electrode/base_electrode.py index 3abe563c77..2b37ceb0d3 100644 --- a/src/pybamm/models/submodels/electrode/base_electrode.py +++ b/src/pybamm/models/submodels/electrode/base_electrode.py @@ -119,7 +119,7 @@ def _get_standard_current_collector_potential_variables( V_cc = phi_s_cp - phi_s_cn # Voltage - # Note phi_s_cn is always zero at the negative tab + # Note phi_s_cn is always zero at the negative tab by definition V = pybamm.boundary_value(phi_s_cp, "positive tab") # Voltage is local current collector potential difference at the tabs, in 1D @@ -128,10 +128,12 @@ def _get_standard_current_collector_potential_variables( "Negative current collector potential [V]": phi_s_cn, "Positive current collector potential [V]": phi_s_cp, "Local voltage [V]": V_cc, + "Voltage expression [V]": V - delta_phi_contact, "Terminal voltage [V]": V - delta_phi_contact, - "Voltage [V]": V - delta_phi_contact, "Contact overpotential [V]": delta_phi_contact, } + if self.options["voltage as a state"] == "false": + variables.update({"Voltage [V]": V - delta_phi_contact}) return variables diff --git a/src/pybamm/models/submodels/external_circuit/explicit_control_external_circuit.py b/src/pybamm/models/submodels/external_circuit/explicit_control_external_circuit.py index 6d1845c3b0..6dcd9a4541 100644 --- a/src/pybamm/models/submodels/external_circuit/explicit_control_external_circuit.py +++ b/src/pybamm/models/submodels/external_circuit/explicit_control_external_circuit.py @@ -19,9 +19,23 @@ def get_fundamental_variables(self): "Current [A]": I, "C-rate": I / self.param.Q, } + if self.options.get("voltage as a state") == "true": + V = pybamm.Variable("Voltage [V]") + variables.update({"Voltage [V]": V}) return variables + def set_initial_conditions(self, variables): + if self.options.get("voltage as a state") == "true": + V = variables["Voltage [V]"] + self.initial_conditions[V] = self.param.ocv_init + + def set_algebraic(self, variables): + if self.options.get("voltage as a state") == "true": + V = variables["Voltage [V]"] + V_expression = variables["Voltage expression [V]"] + self.algebraic[V] = V - V_expression + class ExplicitPowerControl(BaseModel): """External circuit with current set explicitly to hit target power.""" diff --git a/src/pybamm/models/submodels/external_circuit/function_control_external_circuit.py b/src/pybamm/models/submodels/external_circuit/function_control_external_circuit.py index fcb18086da..274d35954a 100644 --- a/src/pybamm/models/submodels/external_circuit/function_control_external_circuit.py +++ b/src/pybamm/models/submodels/external_circuit/function_control_external_circuit.py @@ -48,6 +48,9 @@ def get_fundamental_variables(self): "Current [A]": I, "C-rate": I / self.param.Q, } + if self.options.get("voltage as a state") == "true": + V = pybamm.Variable("Voltage [V]") + variables.update({"Voltage [V]": V}) return variables @@ -55,6 +58,9 @@ def set_initial_conditions(self, variables): # Initial condition as a guess for consistent initial conditions i_cell = variables["Current variable [A]"] self.initial_conditions[i_cell] = self.param.Q + if self.options.get("voltage as a state") == "true": + V = variables["Voltage [V]"] + self.initial_conditions[V] = self.param.ocv_init def set_rhs(self, variables): # External circuit submodels are always equations on the current @@ -71,6 +77,10 @@ def set_algebraic(self, variables): if self.control == "algebraic": i_cell = variables["Current variable [A]"] self.algebraic[i_cell] = self.external_circuit_function(variables) + if self.options.get("voltage as a state") == "true": + V = variables["Voltage [V]"] + V_expression = variables["Voltage expression [V]"] + self.algebraic[V] = V - V_expression class VoltageFunctionControl(FunctionControl): diff --git a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py index 95d0b53a64..7dcfccdb66 100644 --- a/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py +++ b/tests/unit/test_models/test_full_battery_models/test_base_battery_model.py @@ -51,6 +51,7 @@ 'thermal': 'x-full' (possible: ['isothermal', 'lumped', 'x-lumped', 'x-full']) 'total interfacial current density as a state': 'false' (possible: ['false', 'true']) 'transport efficiency': 'Bruggeman' (possible: ['Bruggeman', 'ordered packing', 'hyperbola of revolution', 'overlapping spheres', 'tortuosity factor', 'random overlapping cylinders', 'heterogeneous catalyst', 'cation-exchange membrane']) +'voltage as a state': 'false' (possible: ['false', 'true']) 'working electrode': 'both' (possible: ['both', 'positive']) 'x-average side reactions': 'false' (possible: ['false', 'true']) """ @@ -472,6 +473,17 @@ def test_save_load_model(self): os.remove("test_base_battery_model.json") + def test_voltage_as_state(self): + model = pybamm.lithium_ion.SPM({"voltage as a state": "true"}) + assert model.options["voltage as a state"] == "true" + assert isinstance(model.variables["Voltage [V]"], pybamm.Variable) + + model = pybamm.lithium_ion.SPM( + {"voltage as a state": "true", "operating mode": "voltage"} + ) + assert model.options["voltage as a state"] == "true" + assert isinstance(model.variables["Voltage [V]"], pybamm.Variable) + class TestOptions: def test_print_options(self): From fb81f216064d7e29f144cdd3d7603ab0885b682d Mon Sep 17 00:00:00 2001 From: Pip Liggins Date: Tue, 15 Oct 2024 16:33:38 +0100 Subject: [PATCH 7/8] Add _from_json functionality for pybamm.sign (#4517) * Add _from_json functionality for Sign + test * Update changelog --------- Co-authored-by: Eric G. Kratz --- CHANGELOG.md | 1 + src/pybamm/expression_tree/unary_operators.py | 3 ++- .../test_unary_operators.py | 17 ++++++++++++++--- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 762a23d019..12a7d921d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ## Bug Fixes +- Added `_from_json()` functionality to `Sign` which was erroneously omitted previously. ([#4517](https://github.com/pybamm-team/PyBaMM/pull/4517)) - Fixed bug where IDAKLU solver failed when `output variables` were specified and an extrapolation event is present. ([#4440](https://github.com/pybamm-team/PyBaMM/pull/4440)) ## Breaking changes diff --git a/src/pybamm/expression_tree/unary_operators.py b/src/pybamm/expression_tree/unary_operators.py index ace1cd9942..aa90fd6f4c 100644 --- a/src/pybamm/expression_tree/unary_operators.py +++ b/src/pybamm/expression_tree/unary_operators.py @@ -212,7 +212,8 @@ def __init__(self, child): @classmethod def _from_json(cls, snippet: dict): - raise NotImplementedError() + """See :meth:`pybamm.UnaryOperator._from_json()`.""" + return cls(snippet["children"][0]) def diff(self, variable): """See :meth:`pybamm.Symbol.diff()`.""" diff --git a/tests/unit/test_expression_tree/test_unary_operators.py b/tests/unit/test_expression_tree/test_unary_operators.py index 0dbafa38c5..d7544763fe 100644 --- a/tests/unit/test_expression_tree/test_unary_operators.py +++ b/tests/unit/test_expression_tree/test_unary_operators.py @@ -138,9 +138,20 @@ def test_sign(self): ) # Test from_json - with pytest.raises(NotImplementedError): - # signs are always scalar/array types in a discretised model - pybamm.Sign._from_json({}) + c = pybamm.Multiplication(pybamm.Variable("a"), pybamm.Scalar(3)) + sign_json = { + "name": "sign", + "id": 5341515228900508018, + "domains": { + "primary": [], + "secondary": [], + "tertiary": [], + "quaternary": [], + }, + "children": [c], + } + + assert pybamm.sign(c) == pybamm.Sign._from_json(sign_json) def test_floor(self, mocker): a = pybamm.Symbol("a") From 82f7a0ff9bee2731b4a5ec4699d59a582b9031ea Mon Sep 17 00:00:00 2001 From: "Eric G. Kratz" Date: Thu, 17 Oct 2024 09:20:25 -0400 Subject: [PATCH 8/8] Removing the macos-12 runner (#4520) * Removing the macos-12 runner * Update to latest IREE stable release * Try a different stable release * Change jax version * last attempt before reverting jax-iree changes * Hack to make keep this running --- .github/workflows/publish_pypi.yml | 2 +- .github/workflows/run_periodic_tests.yml | 4 ++-- .github/workflows/test_on_push.yml | 4 ++-- noxfile.py | 2 +- src/pybamm/util.py | 6 ------ 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/publish_pypi.yml b/.github/workflows/publish_pypi.yml index 64c8a3a777..89c9b057c1 100644 --- a/.github/workflows/publish_pypi.yml +++ b/.github/workflows/publish_pypi.yml @@ -242,7 +242,7 @@ jobs: python scripts/install_KLU_Sundials.py python -m cibuildwheel --output-dir wheelhouse env: - # 10.13 for Intel (macos-12/macos-13), 11.0 for Apple Silicon (macos-14 and macos-latest) + # 10.13 for Intel (macos-13), 11.0 for Apple Silicon (macos-14 and macos-latest) MACOSX_DEPLOYMENT_TARGET: ${{ matrix.os == 'macos-14' && '11.0' || '10.13' }} CIBW_ARCHS_MACOS: auto CIBW_BEFORE_BUILD: python -m pip install cmake casadi setuptools wheel delocate diff --git a/.github/workflows/run_periodic_tests.yml b/.github/workflows/run_periodic_tests.yml index 68a8a1ae8b..6e86de054e 100644 --- a/.github/workflows/run_periodic_tests.yml +++ b/.github/workflows/run_periodic_tests.yml @@ -31,7 +31,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ ubuntu-latest, macos-12, macos-14, windows-latest ] + os: [ ubuntu-latest, macos-13, macos-14, windows-latest ] python-version: [ "3.9", "3.10", "3.11", "3.12" ] name: Tests (${{ matrix.os }} / Python ${{ matrix.python-version }}) @@ -46,7 +46,7 @@ jobs: sudo apt-get install gfortran gcc graphviz pandoc libopenblas-dev texlive-latex-extra dvipng - name: Install macOS system dependencies - if: matrix.os == 'macos-12' || matrix.os == 'macos-14' + if: matrix.os == 'macos-13' || matrix.os == 'macos-14' env: HOMEBREW_NO_INSTALL_CLEANUP: 1 HOMEBREW_NO_AUTO_UPDATE: 1 diff --git a/.github/workflows/test_on_push.yml b/.github/workflows/test_on_push.yml index 9be0c7b3ea..32e3017446 100644 --- a/.github/workflows/test_on_push.yml +++ b/.github/workflows/test_on_push.yml @@ -41,7 +41,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-12, macos-14, windows-latest] + os: [ubuntu-latest, macos-13, macos-14, windows-latest] python-version: ["3.9", "3.10", "3.11", "3.12"] name: Tests (${{ matrix.os }} / Python ${{ matrix.python-version }}) @@ -65,7 +65,7 @@ jobs: sudo apt-get install libopenblas-dev texlive-latex-extra dvipng - name: Install macOS system dependencies - if: matrix.os == 'macos-12' || matrix.os == 'macos-14' + if: matrix.os == 'macos-13' || matrix.os == 'macos-14' env: HOMEBREW_NO_INSTALL_CLEANUP: 1 HOMEBREW_NO_AUTO_UPDATE: 1 diff --git a/noxfile.py b/noxfile.py index 14bafcca47..4792cd90c2 100644 --- a/noxfile.py +++ b/noxfile.py @@ -40,7 +40,7 @@ def set_iree_state(): # iree-compiler is currently only available as a wheel on macOS 13 (or # higher) and Python version 3.11 mac_ver = int(platform.mac_ver()[0].split(".")[0]) - if (not sys.version_info[:2] == (3, 11)) or mac_ver < 13: + if (not sys.version_info[:2] == (3, 11)) or mac_ver < 14: warnings.warn( ( "IREE is only supported on MacOS 13 (or higher) and Python" diff --git a/src/pybamm/util.py b/src/pybamm/util.py index 527c55f526..5b10b23fcb 100644 --- a/src/pybamm/util.py +++ b/src/pybamm/util.py @@ -1,9 +1,3 @@ -# -# Utility classes for PyBaMM -# -# The code in this file is adapted from Pints -# (see https://github.com/pints-team/pints) -# import importlib.util import importlib.metadata import numbers