Merge branch 'develop' into telemetry

valentinsulzer · valentinsulzer · commit fd567d20d7ab · 2024-10-18T14:04:05.000-04:00
diff --git 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/.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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/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/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
diff --git 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/.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.*]
diff --git 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 basic telemetry to record which functions are being run. See [Telemetry section in the User Guide](https://docs.pybamm.org/en/latest/source/user_guide/index.html#telemetry) for more information. ([#4441](https://github.com/pybamm-team/PyBaMM/pull/4441))
@@ -20,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/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-BADGE:START - Do not remove or modify this section -->
-[![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)
 <!-- ALL-CONTRIBUTORS-BADGE:END -->
 
 </div>
diff --git a/all_contributors.md b/all_contributors.md
@@ -124,6 +124,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
     </tr>
     <tr>
       <td align="center" valign="top" width="14.28%"><a href="http://marcberliner.com"><img src="https://avatars.githubusercontent.com/u/34451391?v=4?s=100" width="100px;" alt="Marc Berliner"/><br /><sub><b>Marc Berliner</b></sub></a><br /><a href="https://github.com/pybamm-team/PyBaMM/commits?author=MarcBerliner" title="Code">💻</a> <a href="https://github.com/pybamm-team/PyBaMM/commits?author=MarcBerliner" title="Documentation">📖</a> <a href="#infra-MarcBerliner" title="Infrastructure (Hosting, Build-Tools, etc)">🚇</a> <a href="#maintenance-MarcBerliner" title="Maintenance">🚧</a></td>
+      <td align="center" valign="top" width="14.28%"><a href="https://github.com/Aswinr24"><img src="https://avatars.githubusercontent.com/u/135364633?v=4?s=100" width="100px;" alt="Aswinr24"/><br /><sub><b>Aswinr24</b></sub></a><br /><a href="https://github.com/pybamm-team/PyBaMM/commits?author=Aswinr24" title="Tests">⚠️</a></td>
     </tr>
   </tbody>
 </table>
diff --git 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/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/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
@@ -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
@@ -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
@@ -48,13 +48,19 @@ 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):
         # 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/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
diff --git 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")
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
@@ -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):
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
@@ -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
@@ -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
@@ -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
@@ -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
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
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
diff --git a/tests/unit/test_parameters/test_bpx.py b/tests/unit/test_parameters/test_bpx.py
diff --git a/tests/unit/test_parameters/test_process_parameter_data.py b/tests/unit/test_parameters/test_process_parameter_data.py
