merge develop

js1tr3 · May 1, 2024 · a3c5e5d · a3c5e5d
2 parents 3faa2a8 + 741d8cd
commit a3c5e5d
Show file tree

Hide file tree

Showing 23 changed files with 616 additions and 123 deletions.
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.9.3
+        uses: lycheeverse/lychee-action@v1.10.0
         with:
           # arguments with file types to check
           args: >-

diff --git a/.github/workflows/run_periodic_tests.yml b/.github/workflows/run_periodic_tests.yml
@@ -44,17 +44,8 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, macos-12, windows-latest]
+        os: [ubuntu-latest, macos-12, macos-14, windows-latest]
         python-version: ["3.9", "3.10", "3.11", "3.12"]
-        # Include macOS M1 runners
-        include:
-          - os: macos-14
-            python-version: "3.10"
-          - os: macos-14
-            python-version: "3.11"
-          - os: macos-14
-            python-version: "3.12"
-
     steps:
       - name: Check out PyBaMM repository
         uses: actions/checkout@v4

diff --git a/.github/workflows/test_on_push.yml b/.github/workflows/test_on_push.yml
@@ -36,20 +36,12 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, macos-12, windows-latest]
+        os: [ubuntu-latest, macos-12, macos-14, windows-latest]
         python-version: ["3.9", "3.10", "3.11", "3.12"]
         # We check coverage on Ubuntu with Python 3.12, so we skip unit tests for it here
         exclude:
           - os: ubuntu-latest
             python-version: "3.12"
-        # Include macOS M1 runners
-        include:
-          - os: macos-14
-            python-version: "3.10"
-          - os: macos-14
-            python-version: "3.11"
-          - os: macos-14
-            python-version: "3.12"
     name: Unit tests (${{ matrix.os }} / Python ${{ matrix.python-version }})
 
     steps:
@@ -184,18 +176,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, macos-12, windows-latest]
+        os: [ubuntu-latest, macos-12, macos-14, windows-latest]
         python-version: ["3.9", "3.10", "3.11", "3.12"]
-        # Include macOS M1 runners
-        include:
-          - os: macos-14
-            python-version: "3.10"
-          - os: macos-14
-            python-version: "3.11"
-          - os: macos-14
-            python-version: "3.12"
     name: Integration tests (${{ matrix.os }} / Python ${{ matrix.python-version }})
-
     steps:
       - name: Check out PyBaMM repository
         uses: actions/checkout@v4

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -4,7 +4,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.3.7"
+    rev: "v0.4.2"
     hooks:
       - id: ruff
         args: [--fix, --show-fixes]

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,8 +2,9 @@
 
 ## Features
 
-- "Basic" models are now compatible with experiments ([#3995](https://github.com/pybamm-team/PyBaMM/pull/3995))
+- Added `plot_thermal_components` to plot the contributions to the total heat generation in a battery ([#4021](https://github.com/pybamm-team/PyBaMM/pull/4021))
 - Added functions for normal probability density function (`pybamm.normal_pdf`) and cumulative distribution function (`pybamm.normal_cdf`) ([#3999](https://github.com/pybamm-team/PyBaMM/pull/3999))
+- "Basic" models are now compatible with experiments ([#3995](https://github.com/pybamm-team/PyBaMM/pull/3995))
 - Updates multiprocess `Pool` in `BaseSolver.solve()` to be constructed with context `fork`. Adds small example for multiprocess inputs. ([#3974](https://github.com/pybamm-team/PyBaMM/pull/3974))
 - Added custom experiment steps ([#3835](https://github.com/pybamm-team/PyBaMM/pull/3835))
 - Added support for macOS arm64 (M-series) platforms. ([#3789](https://github.com/pybamm-team/PyBaMM/pull/3789))
@@ -15,6 +16,8 @@
 
 ## Bug Fixes
 
+- Set the `remove_independent_variables_from_rhs` to `False` by default, and moved the option from `Discretisation.process_model` to `Discretisation.__init__`. This fixes a bug related to the discharge capacity, but may make the simulation slower in some cases. To set the option to `True`, use `Simulation(..., discretisation_kwargs={"remove_independent_variables_from_rhs": True})`. ([#4020](https://github.com/pybamm-team/PyBaMM/pull/4020))
+- Fixed a bug where independent variables were removed from models even if they appeared in events ([#4019](https://github.com/pybamm-team/PyBaMM/pull/4019))
 - Fix bug with upwind and downwind schemes producing the wrong discretised system ([#3979](https://github.com/pybamm-team/PyBaMM/pull/3979))
 - Allow evaluation of an `Interpolant` object with a number ([#3932](https://github.com/pybamm-team/PyBaMM/pull/3932))
 - `plot_voltage_components` now works even if the time does not start at 0 ([#3915](https://github.com/pybamm-team/PyBaMM/pull/3915))
@@ -33,6 +36,7 @@
 
 ## Breaking changes
 
+- Removed `check_model` argument from `Simulation.solve`. To change the `check_model` option, use `Simulation(..., discretisation_kwargs={"check_model": False})`. ([#4020](https://github.com/pybamm-team/PyBaMM/pull/4020))
 - Removed multiple Docker images. Here on, a single Docker image tagged `pybamm/pybamm:latest` will be provided with both solvers (`IDAKLU` and `JAX`) pre-installed. ([#3992](https://github.com/pybamm-team/PyBaMM/pull/3992))
 - Removed support for Python 3.8 ([#3961](https://github.com/pybamm-team/PyBaMM/pull/3961))
 - Renamed "ocp_soc_0_dimensional" to "ocp_soc_0" and "ocp_soc_100_dimensional" to "ocp_soc_100" ([#3942](https://github.com/pybamm-team/PyBaMM/pull/3942))

diff --git a/docs/conf.py b/docs/conf.py
@@ -371,6 +371,27 @@
 
 """
 
+if os.environ.get("READTHEDOCS_VERSION") == "latest":
+    # append another admonition to warn about unreleased features
+    # note: this needs to be appended with a newline and correct dedentation
+    nbsphinx_prolog += r"""
+
+    <div class="admonition attention">
+        <p class="admonition-title">
+            Attention
+        </p>
+        <p>
+            You are viewing this notebook on the latest version of the documentation,
+            where these notebooks may not be compatible with the stable release of
+            PyBaMM since they can contain features that are not yet released.
+            We recommend viewing these notebooks from the stable version of the documentation. To install the latest version of PyBaMM that is compatible with the latest notebooks,
+            <a href="https://docs.pybamm.org/en/latest/source/user_guide/installation/index.html\#full-installation-guide
+            ">build PyBaMM from source</a>.
+        </p>
+    </div>
+
+"""
+
 # -- sphinxext/inheritance_diagram.py options --------------------------------
 
 graphviz_output_format = "svg"

diff --git a/docs/source/examples/notebooks/models/loss_of_active_materials.ipynb b/docs/source/examples/notebooks/models/loss_of_active_materials.ipynb
@@ -63,6 +63,7 @@
     "    model,\n",
     "    experiment=experiment,\n",
     "    parameter_values=param,\n",
+    "    discretisation_kwargs={\"remove_independent_variables_from_rhs\": True},\n",
     ")\n",
     "solution = sim.solve(calc_esoh=False)"
    ]
@@ -82,7 +83,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "d3d8dcb00eb7421a83a4655bfd8e1245",
+       "model_id": "6b19474c3912495eb75217e009760637",
        "version_major": 2,
        "version_minor": 0
       },
@@ -96,7 +97,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.plotting.quick_plot.QuickPlot at 0x169320050>"
+       "<pybamm.plotting.quick_plot.QuickPlot at 0x152567510>"
       ]
      },
      "execution_count": 2,
@@ -152,7 +153,7 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "6e6d0630770b475893301455ac4910c7",
+       "model_id": "789a681c8c574bb8b3d3016a844dd9a2",
        "version_major": 2,
        "version_minor": 0
       },
@@ -166,7 +167,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.plotting.quick_plot.QuickPlot at 0x175c86c10>"
+       "<pybamm.plotting.quick_plot.QuickPlot at 0x163635410>"
       ]
      },
      "execution_count": 3,
@@ -186,6 +187,7 @@
     "        model,\n",
     "        experiment=experiment,\n",
     "        parameter_values=param,\n",
+    "        discretisation_kwargs={\"remove_independent_variables_from_rhs\": True},\n",
     "    )\n",
     "    solution = sim.solve(calc_esoh=False)\n",
     "    solutions.append(solution)\n",
@@ -223,12 +225,12 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "979c0f154595488da4fab0bd7cd66565",
+       "model_id": "ad36439975754b29bbbef1bd94379408",
        "version_major": 2,
        "version_minor": 0
       },
       "text/plain": [
-       "interactive(children=(FloatSlider(value=0.0, description='t', max=1.8531299069255607, step=0.01853129906925560…"
+       "interactive(children=(FloatSlider(value=0.0, description='t', max=1.8531298311682403, step=0.01853129831168240…"
       ]
      },
      "metadata": {},
@@ -237,7 +239,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.plotting.quick_plot.QuickPlot at 0x2846e3b90>"
+       "<pybamm.plotting.quick_plot.QuickPlot at 0x164f1f810>"
       ]
      },
      "execution_count": 4,
@@ -298,12 +300,12 @@
     {
      "data": {
       "application/vnd.jupyter.widget-view+json": {
-       "model_id": "42f77d86732043a795b8c3264fd755bc",
+       "model_id": "91ea043e10d342049929095e48e98c5e",
        "version_major": 2,
        "version_minor": 0
       },
       "text/plain": [
-       "interactive(children=(FloatSlider(value=0.0, description='t', max=1.8506642114311478, step=0.01850664211431147…"
+       "interactive(children=(FloatSlider(value=0.0, description='t', max=1.8506629989989005, step=0.01850662998998900…"
       ]
      },
      "metadata": {},
@@ -312,7 +314,7 @@
     {
      "data": {
       "text/plain": [
-       "<pybamm.plotting.quick_plot.QuickPlot at 0x2852402d0>"
+       "<pybamm.plotting.quick_plot.QuickPlot at 0x29d0de550>"
       ]
      },
      "execution_count": 5,
@@ -415,7 +417,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.8.10"
+   "version": "3.11.8"
   },
   "toc": {
    "base_numbering": 1,

diff --git a/pybamm/__init__.py b/pybamm/__init__.py
@@ -180,6 +180,7 @@
 from .plotting.plot import plot
 from .plotting.plot2D import plot2D
 from .plotting.plot_voltage_components import plot_voltage_components
+from .plotting.plot_thermal_components import plot_thermal_components
 from .plotting.plot_summary_variables import plot_summary_variables
 from .plotting.dynamic_plot import dynamic_plot
 

diff --git a/pybamm/batch_study.py b/pybamm/batch_study.py
@@ -102,7 +102,6 @@ def solve(
         self,
         t_eval=None,
         solver=None,
-        check_model=True,
         save_at_cycles=None,
         calc_esoh=True,
         starting_solution=None,
@@ -158,7 +157,6 @@ def solve(
                 sol = sim.solve(
                     t_eval,
                     solver,
-                    check_model,
                     save_at_cycles,
                     calc_esoh,
                     starting_solution,

diff --git a/pybamm/discretisations/discretisation.py b/pybamm/discretisations/discretisation.py
@@ -26,14 +26,32 @@ class Discretisation:
     Parameters
     ----------
     mesh : pybamm.Mesh
-            contains all submeshes to be used on each domain
+        contains all submeshes to be used on each domain
     spatial_methods : dict
-            a dictionary of the spatial methods to be used on each
-            domain. The keys correspond to the model domains and the
-            values to the spatial method.
+        a dictionary of the spatial methods to be used on each
+        domain. The keys correspond to the model domains and the
+        values to the spatial method.
+    check_model : bool, optional
+            If True, model checks are performed after discretisation. For large
+            systems these checks can be slow, so can be skipped by setting this
+            option to False. When developing, testing or debugging it is recommended
+            to leave this option as True as it may help to identify any errors.
+            Default is True.
+    remove_independent_variables_from_rhs : bool, optional
+        If True, model checks to see whether any variables from the RHS are used
+        in any other equation. If a variable meets all of the following criteria
+        (not used anywhere in the model, len(rhs)>1), then the variable
+        is moved to be explicitly integrated when called by the solution object.
+        Default is False.
     """
 
-    def __init__(self, mesh=None, spatial_methods=None):
+    def __init__(
+        self,
+        mesh=None,
+        spatial_methods=None,
+        check_model=True,
+        remove_independent_variables_from_rhs=False,
+    ):
         self._mesh = mesh
         if mesh is None:
             self._spatial_methods = {}
@@ -60,6 +78,10 @@ def __init__(self, mesh=None, spatial_methods=None):
         self._bcs = {}
         self.y_slices = {}
         self._discretised_symbols = {}
+        self._check_model_flag = check_model
+        self._remove_independent_variables_from_rhs_flag = (
+            remove_independent_variables_from_rhs
+        )
 
     @property
     def mesh(self):
@@ -90,13 +112,7 @@ def bcs(self, value):
         # reset discretised_symbols
         self._discretised_symbols = {}
 
-    def process_model(
-        self,
-        model,
-        inplace=True,
-        check_model=True,
-        remove_independent_variables_from_rhs=True,
-    ):
+    def process_model(self, model, inplace=True):
         """
         Discretise a model. Currently inplace, could be changed to return a new model.
 
@@ -108,18 +124,6 @@ def process_model(
         inplace : bool, optional
             If True, discretise the model in place. Otherwise, return a new
             discretised model. Default is True.
-        check_model : bool, optional
-            If True, model checks are performed after discretisation. For large
-            systems these checks can be slow, so can be skipped by setting this
-            option to False. When developing, testing or debugging it is recommended
-            to leave this option as True as it may help to identify any errors.
-            Default is True.
-        remove_independent_variables_from_rhs : bool, optional
-            If True, model checks to see whether any variables from the RHS are used
-            in any other equation. If a variable meets all of the following criteria
-            (not used anywhere in the model, len(rhs)>1), then the variable
-            is moved to be explicitly integrated when called by the solution object.
-            Default is True.
 
         Returns
         -------
@@ -158,7 +162,7 @@ def process_model(
         # set variables (we require the full variable not just id)
 
         # Search Equations for Independence
-        if remove_independent_variables_from_rhs:
+        if self._remove_independent_variables_from_rhs_flag:
             model = self.remove_independent_variables_from_rhs(model)
         variables = list(model.rhs.keys()) + list(model.algebraic.keys())
         # Find those RHS's that are constant
@@ -240,7 +244,7 @@ def process_model(
         model_disc._geometry = getattr(self.mesh, "_geometry", None)
 
         # Check that resulting model makes sense
-        if check_model:
+        if self._check_model_flag:
             pybamm.logger.verbose(f"Performing model checks for {model.name}")
             self.check_model(model_disc)
 
@@ -1103,6 +1107,7 @@ def remove_independent_variables_from_rhs(self, model):
             # only check children of variables, this will skip the variable itself
             # and catch any other cases
             + [child for var in model.variables.values() for child in var.children]
+            + [event.expression for event in model.events]
         )
         all_vars_in_eqns = unpacker.unpack_list_of_symbols(eqns_to_check)
         all_vars_in_eqns = [var.name for var in all_vars_in_eqns]
@@ -1122,6 +1127,7 @@ def remove_independent_variables_from_rhs(self, model):
                     # in variables twice under different names
                     for key in model.variables:
                         if model.variables[key] == var:
+                            print("here")
                             model.variables[key] = model.variables[var.name]
                     del model.rhs[var]
                     del model.initial_conditions[var]