From 4d9d6e0586f8a5c6bfb6d1cb5feb1b8cd8bec3ed Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Tue, 31 Oct 2023 15:17:06 +0530 Subject: [PATCH 1/8] Execute notebook (a trial of a thousand times) --- .../test_simulation_stochasticity.yml | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 .github/workflows/test_simulation_stochasticity.yml diff --git a/.github/workflows/test_simulation_stochasticity.yml b/.github/workflows/test_simulation_stochasticity.yml new file mode 100644 index 0000000000..2df95a60c9 --- /dev/null +++ b/.github/workflows/test_simulation_stochasticity.yml @@ -0,0 +1,51 @@ +name: Test random failures for half-cell models example notebook (no fixed seed) + +on: + push: + workflow_dispatch: + +jobs: + test_half_cell_example_notebook: + name: Execute half-cell models example notebook / Python ${{ matrix.python-version }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.8", "3.9", "3.10", "3.11"] + steps: + - name: Check out PyBaMM repository + uses: actions/checkout@v4 + + - name: Install Linux system dependencies + run: | + sudo apt-get update + sudo apt-get install gfortran gcc graphviz pandoc libopenblas-dev texlive-latex-extra dvipng + + - name: Set up Python ${{ matrix.python-version }} + id: setup-python + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Install SuiteSparse and SUNDIALS + run: pipx run nox -s pybamm-requires + + - name: Install Python dependencies and PyBaMM + run: | + pip install --upgrade pip setuptools wheel cmake + pip install -e .[all,dev] + + + - name: Run half-cell models example notebook + run: | + for i in {1..500} + do + pytest --nbmake docs/source/examples/notebooks/models/half-cell.ipynb >> logs-${{ matrix.python-version }}.txt || true + done + + - name: Upload results + uses: actions/upload-artifact@v3 + with: + name: Results + path: logs-${{ matrix.python-version }}.txt + if-no-files-found: error From 4b94b297fc30735cb28061703b0ccbf8b72f5301 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Tue, 31 Oct 2023 21:47:25 +0530 Subject: [PATCH 2/8] Execute notebook, this time with a random seed --- pybamm/simulation.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 380105d215..3ecc838c21 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -4,6 +4,7 @@ import pickle import pybamm import numpy as np +import hashlib import warnings import sys from functools import lru_cache @@ -29,6 +30,39 @@ def is_notebook(): return False # Probably standard Python interpreter +def fix_random_seed_for_class(cls): + """ + Wraps a class so that a random seed is set to a SHA-256 hash of the class name. + + As the wrapper fixes the random seed during class initialization, instances of + the class will be initialized with the same random seed for reproducibility. + + Generating a random seed from the class name allows one to alter the seed by + changing the class name if needed. + + Usage: as a decorator on class definition. + + ``` + @FixRandomSeedClass + class Simulation: + def __init__(self, model, solver, other_args): + # Your class initialization code here + ``` + """ + + original_init = cls.__init__ + + def new_init(self, *args, **kwargs): + np.random.seed( + int(hashlib.sha256(cls.__name__.encode()).hexdigest(), 16) % (2**32) + ) + original_init(self, *args, **kwargs) + + cls.__init__ = new_init + return cls + + +@fix_random_seed_for_class class Simulation: """A Simulation class for easy building and running of PyBaMM simulations. From c14adaf6b8a686a821a227c5f19ce5ae819ec780 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Wed, 1 Nov 2023 22:02:09 +0530 Subject: [PATCH 3/8] Set hash inside `__init__` instead of a decorator and trigger another experiment with 500 runs on each Python version, amounting to a total of 2000 executions of the same notebook --- pybamm/simulation.py | 44 +++++++++++--------------------------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 3ecc838c21..b50337eff4 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -30,39 +30,6 @@ def is_notebook(): return False # Probably standard Python interpreter -def fix_random_seed_for_class(cls): - """ - Wraps a class so that a random seed is set to a SHA-256 hash of the class name. - - As the wrapper fixes the random seed during class initialization, instances of - the class will be initialized with the same random seed for reproducibility. - - Generating a random seed from the class name allows one to alter the seed by - changing the class name if needed. - - Usage: as a decorator on class definition. - - ``` - @FixRandomSeedClass - class Simulation: - def __init__(self, model, solver, other_args): - # Your class initialization code here - ``` - """ - - original_init = cls.__init__ - - def new_init(self, *args, **kwargs): - np.random.seed( - int(hashlib.sha256(cls.__name__.encode()).hexdigest(), 16) % (2**32) - ) - original_init(self, *args, **kwargs) - - cls.__init__ = new_init - return cls - - -@fix_random_seed_for_class class Simulation: """A Simulation class for easy building and running of PyBaMM simulations. @@ -156,6 +123,17 @@ def __init__( self._solver = solver or self._model.default_solver self._output_variables = output_variables + # If the solver being used is CasadiSolver or its variant, set a fixed + # random seed during class initialization to the SHA-256 hash of the class + # name for purposes of reproducibility. + if isinstance(self._solver, pybamm.CasadiSolver) or isinstance( + self._solver, pybamm.CasadiAlgebraicSolver + ): + np.random.seed( + int(hashlib.sha256(self.__class__.__name__.encode()).hexdigest(), 16) + % (2**32) + ) + # Initialize empty built states self._model_with_set_params = None self._built_model = None From f6a2f8353bd2b9aeaf593425361815c20a42ce02 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Thu, 2 Nov 2023 02:39:12 +0530 Subject: [PATCH 4/8] Use a member function, then call it in `__init__` --- pybamm/simulation.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/pybamm/simulation.py b/pybamm/simulation.py index b50337eff4..39c182acc0 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -123,17 +123,6 @@ def __init__( self._solver = solver or self._model.default_solver self._output_variables = output_variables - # If the solver being used is CasadiSolver or its variant, set a fixed - # random seed during class initialization to the SHA-256 hash of the class - # name for purposes of reproducibility. - if isinstance(self._solver, pybamm.CasadiSolver) or isinstance( - self._solver, pybamm.CasadiAlgebraicSolver - ): - np.random.seed( - int(hashlib.sha256(self.__class__.__name__.encode()).hexdigest(), 16) - % (2**32) - ) - # Initialize empty built states self._model_with_set_params = None self._built_model = None @@ -145,6 +134,9 @@ def __init__( self._solution = None self.quick_plot = None + # Initialise instances of Simulation class with the same random seed + self.set_random_seed() + # ignore runtime warnings in notebooks if is_notebook(): # pragma: no cover import warnings @@ -168,6 +160,18 @@ def __setstate__(self, state): self.__dict__ = state self.get_esoh_solver = lru_cache()(self._get_esoh_solver) + # If the solver being used is CasadiSolver or its variant, set a fixed + # random seed during class initialization to the SHA-256 hash of the class + # name for purposes of reproducibility. + def set_random_seed(self): + if isinstance(self._solver, pybamm.CasadiSolver) or isinstance( + self._solver, pybamm.CasadiAlgebraicSolver + ): + np.random.seed( + int(hashlib.sha256(self.__class__.__name__.encode()).hexdigest(), 16) + % (2**32) + ) + def set_up_and_parameterise_experiment(self): """ Set up a simulation to run with an experiment. This creates a dictionary of From 9438b2b8d5746c8162c116633369adf16c83b4c2 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Thu, 2 Nov 2023 02:43:23 +0530 Subject: [PATCH 5/8] Update workflow to rename the log files This workflow will subsequently be removed in the next commit, after a final run on a push of this commit. --- .github/workflows/test_simulation_stochasticity.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_simulation_stochasticity.yml b/.github/workflows/test_simulation_stochasticity.yml index 2df95a60c9..fa32f3b5ef 100644 --- a/.github/workflows/test_simulation_stochasticity.yml +++ b/.github/workflows/test_simulation_stochasticity.yml @@ -40,12 +40,12 @@ jobs: run: | for i in {1..500} do - pytest --nbmake docs/source/examples/notebooks/models/half-cell.ipynb >> logs-${{ matrix.python-version }}.txt || true + pytest --nbmake docs/source/examples/notebooks/models/half-cell.ipynb >> new-logs-${{ matrix.python-version }}.txt || true done - name: Upload results uses: actions/upload-artifact@v3 with: name: Results - path: logs-${{ matrix.python-version }}.txt + path: new-logs-${{ matrix.python-version }}.txt if-no-files-found: error From 495d6d709b7ffe867221a11d306fd72624a7ddb3 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:58:07 +0530 Subject: [PATCH 6/8] Delete stress-testing workflow --- .../test_simulation_stochasticity.yml | 51 ------------------- 1 file changed, 51 deletions(-) delete mode 100644 .github/workflows/test_simulation_stochasticity.yml diff --git a/.github/workflows/test_simulation_stochasticity.yml b/.github/workflows/test_simulation_stochasticity.yml deleted file mode 100644 index fa32f3b5ef..0000000000 --- a/.github/workflows/test_simulation_stochasticity.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: Test random failures for half-cell models example notebook (no fixed seed) - -on: - push: - workflow_dispatch: - -jobs: - test_half_cell_example_notebook: - name: Execute half-cell models example notebook / Python ${{ matrix.python-version }} - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - python-version: ["3.8", "3.9", "3.10", "3.11"] - steps: - - name: Check out PyBaMM repository - uses: actions/checkout@v4 - - - name: Install Linux system dependencies - run: | - sudo apt-get update - sudo apt-get install gfortran gcc graphviz pandoc libopenblas-dev texlive-latex-extra dvipng - - - name: Set up Python ${{ matrix.python-version }} - id: setup-python - uses: actions/setup-python@v4 - with: - python-version: ${{ matrix.python-version }} - - - name: Install SuiteSparse and SUNDIALS - run: pipx run nox -s pybamm-requires - - - name: Install Python dependencies and PyBaMM - run: | - pip install --upgrade pip setuptools wheel cmake - pip install -e .[all,dev] - - - - name: Run half-cell models example notebook - run: | - for i in {1..500} - do - pytest --nbmake docs/source/examples/notebooks/models/half-cell.ipynb >> new-logs-${{ matrix.python-version }}.txt || true - done - - - name: Upload results - uses: actions/upload-artifact@v3 - with: - name: Results - path: new-logs-${{ matrix.python-version }}.txt - if-no-files-found: error From 02b67aba407237f954a1c2668a11bbbf9facf370 Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Thu, 2 Nov 2023 16:00:03 +0530 Subject: [PATCH 7/8] Mark seed fixer as semiprivate, do not expose it --- pybamm/simulation.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pybamm/simulation.py b/pybamm/simulation.py index 39c182acc0..8fbcb387f1 100644 --- a/pybamm/simulation.py +++ b/pybamm/simulation.py @@ -135,7 +135,7 @@ def __init__( self.quick_plot = None # Initialise instances of Simulation class with the same random seed - self.set_random_seed() + self._set_random_seed() # ignore runtime warnings in notebooks if is_notebook(): # pragma: no cover @@ -160,10 +160,10 @@ def __setstate__(self, state): self.__dict__ = state self.get_esoh_solver = lru_cache()(self._get_esoh_solver) - # If the solver being used is CasadiSolver or its variant, set a fixed + # If the solver being used is CasadiSolver or its variants, set a fixed # random seed during class initialization to the SHA-256 hash of the class # name for purposes of reproducibility. - def set_random_seed(self): + def _set_random_seed(self): if isinstance(self._solver, pybamm.CasadiSolver) or isinstance( self._solver, pybamm.CasadiAlgebraicSolver ): From e8cf8dc6d9d3e2b3569fd6c3b11b96938b45ca6d Mon Sep 17 00:00:00 2001 From: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Date: Thu, 2 Nov 2023 16:23:26 +0530 Subject: [PATCH 8/8] Add a CHANGELOG entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 008cad125f..70e8622a75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # [Unreleased](https://github.com/pybamm-team/PyBaMM/) +## Bug fixes + +- 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)) + # [v23.9rc0](https://github.com/pybamm-team/PyBaMM/tree/v23.9rc0) - 2023-10-31 ## Features