Merge pull request #3842 from pybamm-team/fix-minor-bugs

fix some minor annoyances

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ## Bug Fixes
 
+- Initial voltage can now match upper or lower cut-offs exactly ([#3842](https://github.com/pybamm-team/PyBaMM/pull/3842))
 - Fixed a bug where 1+1D and 2+1D models would not work with voltage or power controlled experiments([#3829](https://github.com/pybamm-team/PyBaMM/pull/3829))
 - Update IDAKLU solver to fail gracefully when a variable is requested that was not in the solves `output_variables` list ([#3803](https://github.com/pybamm-team/PyBaMM/pull/3803))
 - Updated `_steps_util.py` to throw a specific exception when drive cycle starts at t>0 ([#3756](https://github.com/pybamm-team/PyBaMM/pull/3756))
@@ -17,6 +18,7 @@
 
 ## Breaking changes
 
+- Renamed "testing" argument for plots to "show_plot" and flipped its meaning (show_plot=True is now the default and shows the plot) ([#3842](https://github.com/pybamm-team/PyBaMM/pull/3842))
 - Dropped support for BPX version 0.3.0 and below ([#3414](https://github.com/pybamm-team/PyBaMM/pull/3414))
 
 # [v24.1](https://github.com/pybamm-team/PyBaMM/tree/v24.1) - 2024-01-31
@@ -35,12 +37,11 @@
 - Added a `get_parameter_info` method for models and modified "print_parameter_info" functionality to extract all parameters and their type in a tabular and readable format ([#3584](https://github.com/pybamm-team/PyBaMM/pull/3584))
 - Mechanical parameters are now a function of stoichiometry and temperature ([#3576](https://github.com/pybamm-team/PyBaMM/pull/3576))
 
-
 ## Bug fixes
 
 - Fixed a bug that lead to a `ShapeError` when specifying "Ambient temperature [K]" as an `Interpolant` with an isothermal model ([#3761](https://github.com/pybamm-team/PyBaMM/pull/3761))
 - Fixed a bug where if the first step(s) in a cycle are skipped then the cycle solution started from the model's initial conditions instead of from the last state of the previous cycle ([#3708](https://github.com/pybamm-team/PyBaMM/pull/3708))
-- Fixed a bug where the lumped thermal model conflates cell volume with electrode volume  ([#3707](https://github.com/pybamm-team/PyBaMM/pull/3707))
+- Fixed a bug where the lumped thermal model conflates cell volume with electrode volume ([#3707](https://github.com/pybamm-team/PyBaMM/pull/3707))
 - Reverted a change to the coupled degradation example notebook that caused it to be unstable for large numbers of cycles ([#3691](https://github.com/pybamm-team/PyBaMM/pull/3691))
 - Fixed a bug where simulations using the CasADi-based solvers would fail randomly with the half-cell model ([#3494](https://github.com/pybamm-team/PyBaMM/pull/3494))
 - Fixed bug that made identical Experiment steps with different end times crash ([#3516](https://github.com/pybamm-team/PyBaMM/pull/3516))

pybamm/models/full_battery_models/lithium_ion/electrode_soh.py

@@ -667,7 +667,7 @@ def get_initial_stoichiometries(self, initial_value, tol=1e-6):
             V_min = parameter_values.evaluate(param.ocp_soc_0_dimensional)
             V_max = parameter_values.evaluate(param.ocp_soc_100_dimensional)
 
-            if not V_min < V_init < V_max:
+            if not V_min <= V_init <= V_max:
                 raise ValueError(
                     f"Initial voltage {V_init}V is outside the voltage limits "
                     f"({V_min}, {V_max})"

pybamm/models/full_battery_models/lithium_ion/electrode_soh_half_cell.py

@@ -93,7 +93,7 @@ def get_initial_stoichiometry_half_cell(
         V_min = parameter_values.evaluate(param.voltage_low_cut)
         V_max = parameter_values.evaluate(param.voltage_high_cut)
 
-        if not V_min < V_init < V_max:
+        if not V_min <= V_init <= V_max:
             raise ValueError(
                 f"Initial voltage {V_init}V is outside the voltage limits "
                 f"({V_min}, {V_max})"

pybamm/plotting/dynamic_plot.py

@@ -8,15 +8,15 @@ def dynamic_plot(*args, **kwargs):
     """
     Creates a :class:`pybamm.QuickPlot` object (with arguments 'args' and keyword
     arguments 'kwargs') and then calls :meth:`pybamm.QuickPlot.dynamic_plot`.
-    The key-word argument 'testing' is passed to the 'dynamic_plot' method, not the
+    The key-word argument 'show_plot' is passed to the 'dynamic_plot' method, not the
     `QuickPlot` class.
 
     Returns
     -------
     plot : :class:`pybamm.QuickPlot`
         The 'QuickPlot' object that was created
     """
-    kwargs_for_class = {k: v for k, v in kwargs.items() if k != "testing"}
+    kwargs_for_class = {k: v for k, v in kwargs.items() if k != "show_plot"}
     plot = pybamm.QuickPlot(*args, **kwargs_for_class)
-    plot.dynamic_plot(kwargs.get("testing", False))
+    plot.dynamic_plot(kwargs.get("show_plot", True))
     return plot

pybamm/plotting/plot.py

@@ -6,7 +6,7 @@
 from pybamm.util import have_optional_dependency
 
 
-def plot(x, y, ax=None, testing=False, **kwargs):
+def plot(x, y, ax=None, show_plot=True, **kwargs):
     """
     Generate a simple 1D plot. Calls `matplotlib.pyplot.plot` with keyword
     arguments 'kwargs'. For a list of 'kwargs' see the
@@ -20,8 +20,9 @@ def plot(x, y, ax=None, testing=False, **kwargs):
         The array to plot on the y axis
     ax : matplotlib Axis, optional
         The axis on which to put the plot. If None, a new figure and axis is created.
-    testing : bool, optional
-        Whether to actually make the plot (turned off for unit tests)
+    show_plot : bool, optional
+        Whether to show the plots. Default is True. Set to False if you want to
+        only display the plot after plt.show() has been called.
     kwargs
         Keyword arguments, passed to plt.plot
 
@@ -34,14 +35,14 @@ def plot(x, y, ax=None, testing=False, **kwargs):
         raise TypeError("y must be 'pybamm.Array'")
 
     if ax is not None:
-        testing = True
+        show_plot = False
     else:
         _, ax = plt.subplots()
 
     ax.plot(x.entries, y.entries, **kwargs)
     ax.set_ylim([ax_min(y.entries), ax_max(y.entries)])
 
-    if not testing:  # pragma: no cover
+    if show_plot:  # pragma: no cover
         plt.show()
 
     return ax

pybamm/plotting/plot2D.py

@@ -6,7 +6,7 @@
 from pybamm.util import have_optional_dependency
 
 
-def plot2D(x, y, z, ax=None, testing=False, **kwargs):
+def plot2D(x, y, z, ax=None, show_plot=True, **kwargs):
     """
     Generate a simple 2D plot. Calls `matplotlib.pyplot.contourf` with keyword
     arguments 'kwargs'.  For a list of 'kwargs' see the
@@ -22,8 +22,9 @@ def plot2D(x, y, z, ax=None, testing=False, **kwargs):
         The array to plot on the z axis. Is of shape (M, N)
     ax : matplotlib Axis, optional
         The axis on which to put the plot. If None, a new figure and axis is created.
-    testing : bool, optional
-        Whether to actually make the plot (turned off for unit tests)
+    show_plot : bool, optional
+        Whether to show the plots. Default is True. Set to False if you want to
+        only display the plot after plt.show() has been called.
 
     """
     plt = have_optional_dependency("matplotlib.pyplot")
@@ -36,7 +37,7 @@ def plot2D(x, y, z, ax=None, testing=False, **kwargs):
         raise TypeError("z must be 'pybamm.Array'")
 
     if ax is not None:
-        testing = True
+        show_plot = False
     else:
         _, ax = plt.subplots()
 
@@ -58,7 +59,7 @@ def plot2D(x, y, z, ax=None, testing=False, **kwargs):
     )
     plt.colorbar(plot, ax=ax)
 
-    if not testing:  # pragma: no cover
+    if show_plot:  # pragma: no cover
         plt.show()
 
     return ax

pybamm/plotting/plot_summary_variables.py

@@ -7,7 +7,7 @@
 
 
 def plot_summary_variables(
-    solutions, output_variables=None, labels=None, testing=False, **kwargs_fig
+    solutions, output_variables=None, labels=None, show_plot=True, **kwargs_fig
 ):
     """
     Generate a plot showing/comparing the summary variables.
@@ -20,8 +20,9 @@ def plot_summary_variables(
         A list of variables to plot automatically. If None, the default ones are used.
     labels: list (optional)
         A list of labels to be added to the legend. No labels are added by default.
-    testing : bool (optional)
-        Whether to actually make the plot (turned off for unit tests).
+    show_plot : bool, optional
+        Whether to show the plots. Default is True. Set to False if you want to
+        only display the plot after plt.show() has been called.
     kwargs_fig
         Keyword arguments, passed to plt.subplots.
 
@@ -74,7 +75,7 @@ def plot_summary_variables(
     # add labels in legend
     if labels is not None:  # pragma: no cover
         fig.legend(labels, loc="lower right")
-    if not testing:  # pragma: no cover
+    if show_plot:  # pragma: no cover
         plt.show()
 
     return axes

pybamm/plotting/plot_voltage_components.py

@@ -13,7 +13,7 @@ def plot_voltage_components(
     ax=None,
     show_legend=True,
     split_by_electrode=False,
-    testing=False,
+    show_plot=True,
     **kwargs_fill,
 ):
     """
@@ -30,8 +30,9 @@ def plot_voltage_components(
     split_by_electrode : bool, optional
         Whether to show the overpotentials for the negative and positive electrodes
         separately. Default is False.
-    testing : bool, optional
-        Whether to actually make the plot (turned off for unit tests)
+    show_plot : bool, optional
+        Whether to show the plots. Default is True. Set to False if you want to
+        only display the plot after plt.show() has been called.
     kwargs_fill
         Keyword arguments, passed to ax.fill_between
 
@@ -48,7 +49,7 @@ def plot_voltage_components(
 
     if ax is not None:
         fig = None
-        testing = True
+        show_plot = False
     else:
         fig, ax = plt.subplots(figsize=(8, 4))
 
@@ -151,7 +152,7 @@ def plot_voltage_components(
     )
     ax.set_ylim([y_min, y_max])
 
-    if not testing:  # pragma: no cover
+    if show_plot:  # pragma: no cover
         plt.show()
 
     return fig, ax

pybamm/plotting/quick_plot.py

@@ -9,7 +9,6 @@
 
 
 class LoopList(list):
-
     """A list which loops over itself when accessing an
     index so that it never runs out"""
 
@@ -649,7 +648,7 @@ def plot(self, t, dynamic=False):
         bottom = max(legend_top, slider_top)
         self.gridspec.tight_layout(self.fig, rect=[0, bottom, 1, 1])
 
-    def dynamic_plot(self, testing=False, step=None):
+    def dynamic_plot(self, show_plot=True, step=None):
         """
         Generate a dynamic plot with a slider to control the time.
 
@@ -658,8 +657,9 @@ def dynamic_plot(self, testing=False, step=None):
         step : float
             For notebook mode, size of steps to allow in the slider. Defaults to 1/100th
             of the total time.
-        testing : bool
-            Whether to actually make the plot (turned off for unit tests)
+        show_plot : bool, optional
+            Whether to show the plots. Default is True. Set to False if you want to
+            only display the plot after plt.show() has been called.
 
         """
         if pybamm.is_notebook():  # pragma: no cover
@@ -692,7 +692,7 @@ def dynamic_plot(self, testing=False, step=None):
             )
             self.slider.on_changed(self.slider_update)
 
-            if not testing:  # pragma: no cover
+            if show_plot:  # pragma: no cover
                 plt.show()
 
     def slider_update(self, t):

pybamm/simulation.py

@@ -1215,7 +1215,7 @@ def plot_voltage_components(
         ax=None,
         show_legend=True,
         split_by_electrode=False,
-        testing=False,
+        show_plot=True,
         **kwargs_fill,
     ):
         """
@@ -1230,8 +1230,9 @@ def plot_voltage_components(
         split_by_electrode : bool, optional
             Whether to show the overpotentials for the negative and positive electrodes
             separately. Default is False.
-        testing : bool, optional
-            Whether to actually make the plot (turned off for unit tests).
+        show_plot : bool, optional
+            Whether to show the plots. Default is True. Set to False if you want to
+            only display the plot after plt.show() has been called.
         kwargs_fill
             Keyword arguments, passed to ax.fill_between.
 
@@ -1244,7 +1245,7 @@ def plot_voltage_components(
             ax=ax,
             show_legend=show_legend,
             split_by_electrode=split_by_electrode,
-            testing=testing,
+            show_plot=show_plot,
             **kwargs_fill,
         )
 

pybamm/solvers/solution.py

@@ -814,7 +814,7 @@ def plot_voltage_components(
         ax=None,
         show_legend=True,
         split_by_electrode=False,
-        testing=False,
+        show_plot=True,
         **kwargs_fill,
     ):
         """
@@ -829,8 +829,9 @@ def plot_voltage_components(
         split_by_electrode : bool, optional
             Whether to show the overpotentials for the negative and positive electrodes
             separately. Default is False.
-        testing : bool, optional
-            Whether to actually make the plot (turned off for unit tests).
+        show_plot : bool, optional
+            Whether to show the plots. Default is True. Set to False if you want to
+            only display the plot after plt.show() has been called.
         kwargs_fill
             Keyword arguments, passed to ax.fill_between.
 
@@ -841,7 +842,7 @@ def plot_voltage_components(
             ax=ax,
             show_legend=show_legend,
             split_by_electrode=split_by_electrode,
-            testing=testing,
+            show_plot=show_plot,
             **kwargs_fill,
         )
 

tests/unit/test_batch_study.py

@@ -1,6 +1,7 @@
 """
 Tests for the batch_study.py
 """
+
 from tests import TestCase
 import os
 import pybamm
@@ -52,7 +53,7 @@ def test_solve(self):
 
         # Tests for BatchStudy when permutations=False
         bs_false.solve()
-        bs_false.plot(testing=True)
+        bs_false.plot(show_plot=False)
         self.assertEqual(2, len(bs_false.sims))
         for num in range(len(bs_false.sims)):
             output_model = bs_false.sims[num].model.name
@@ -72,7 +73,7 @@ def test_solve(self):
 
         # Tests for BatchStudy when permutations=True
         bs_true.solve()
-        bs_true.plot(testing=True)
+        bs_true.plot(show_plot=False)
         self.assertEqual(4, len(bs_true.sims))
         for num in range(len(bs_true.sims)):
             output_model = bs_true.sims[num].model.name
@@ -108,7 +109,7 @@ def test_create_gif(self):
             bs.create_gif(number_of_images=3, duration=1, output_filename=test_file)
 
             # create a GIF after calling the plot method
-            bs.plot(testing=True)
+            bs.plot(show_plot=False)
             bs.create_gif(number_of_images=3, duration=1, output_filename=test_file)
 
 

tests/unit/test_plotting/test_plot.py

@@ -9,10 +9,10 @@ class TestPlot(TestCase):
     def test_plot(self):
         x = pybamm.Array(np.array([0, 3, 10]))
         y = pybamm.Array(np.array([6, 16, 78]))
-        pybamm.plot(x, y, testing=True)
+        pybamm.plot(x, y, show_plot=False)
 
         _, ax = plt.subplots()
-        ax_out = pybamm.plot(x, y, ax=ax, testing=True)
+        ax_out = pybamm.plot(x, y, ax=ax, show_plot=False)
         self.assertEqual(ax_out, ax)
 
     def test_plot_fail(self):
@@ -28,13 +28,13 @@ def test_plot2D(self):
         X, Y = pybamm.meshgrid(x, y)
 
         # plot with array directly
-        pybamm.plot2D(x, y, Y, testing=True)
+        pybamm.plot2D(x, y, Y, show_plot=False)
 
         # plot with meshgrid
-        pybamm.plot2D(X, Y, Y, testing=True)
+        pybamm.plot2D(X, Y, Y, show_plot=False)
 
         _, ax = plt.subplots()
-        ax_out = pybamm.plot2D(X, Y, Y, ax=ax, testing=True)
+        ax_out = pybamm.plot2D(X, Y, Y, ax=ax, show_plot=False)
         self.assertEqual(ax_out, ax)
 
     def test_plot2D_fail(self):

tests/unit/test_plotting/test_plot_summary_variables.py

@@ -36,7 +36,7 @@ def test_plot(self):
         )
         sol = sim.solve(initial_soc=1)
 
-        axes = pybamm.plot_summary_variables(sol, testing=True)
+        axes = pybamm.plot_summary_variables(sol, show_plot=False)
 
         axes = axes.flatten()
         self.assertEqual(len(axes), 9)
@@ -52,7 +52,7 @@ def test_plot(self):
             np.testing.assert_array_equal(var, sol.summary_variables[output_var])
 
         axes = pybamm.plot_summary_variables(
-            [sol, sol], labels=["SPM", "SPM"], testing=True
+            [sol, sol], labels=["SPM", "SPM"], show_plot=False
         )
 
         axes = axes.flatten()