Cracking submodel

[weilongai]

Description

A new submodel for particle cracking in pybamm. No dependencies that are required for this change.

Fixes #671

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ flake8
• All tests pass: $ python run-tests.py --unit
• The documentation builds: $ cd docs and then $ make clean; make html

You can run all three at once, using $ python run-tests.py --quick.

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

Additional notes of the actions:

• [Add cracking model] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/pybamm/models/submodels/particle_cracking)
• [Add mechanical parameters] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/pybamm/parameters/mechanical_parameters.py)
• [Add temperature dependence to the thermodynamic factor in lines 631-639] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/pybamm/parameters/lithium_ion_parameters.py)
• [Add a new parameter set for the Enertech cells (LiCoO2-graphite)] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/pybamm/input/parameters/lithium-ion)
• [Add test functions] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/tests/unit/test_models/test_submodels/test_mechanical)
• [Add notebooks] (https://github.com/weilongai/PyBaMM/tree/pre-release/cracking/examples/notebooks/Validating_mechanical_models_Enertech_DFN.ipynb)

[weilongai]

There are errors for testing the crack model, because of not matching shapes as below. Does anyone know how to define them in test functions, e.g. test_SPM, test_DFN and test_crack_propagation? There is no error in notebook examples.

  File "/home/aishine/GEM/PyBaMM/pybamm/models/submodels/particle_cracking/base_cracking.py", line 148, in _get_standard_surface_variables
    a_cr = (roughness - 1) * a0  # normalised crack surface area

  File "/home/aishine/GEM/PyBaMM/pybamm/expression_tree/symbol.py", line 836, in test_shape
    raise pybamm.ShapeError("Cannot find shape (original error: {})".format(e))
pybamm.expression_tree.exceptions.ShapeError: Cannot find shape (original error: operands could not be broadcast together with shapes (11,1) (33,1) )

[valentinsulzer]

For test_crack_propagation, you need to add auxiliary_domains={"secondary": "current collector"} to the Variable you define in CrackPropagation.get_fundamental_variables. The tests are written with pybamm.settings.debug_mode = True, which performs additional shape checks to make sure the model would be compatible with 2+1D. This isn't a problem in your notebooks because you only ever solve it in 1D.
For test_SPM and test_DFN, it seems like a parameter error, you can comment those out if you want and open a new issue specifically for fixing those.

Edit: the parameter values can be fixed by setting e.g.

        options = {"particle": "Fickian diffusion", "particle cracking": None}
        model = pybamm.lithium_ion.SPM(options)
        chemistry = pybamm.parameter_sets.Ai2020
        parameter_values = pybamm.ParameterValues(chemistry=chemistry)
        modeltest = tests.StandardModelTest(model, parameter_values=parameter_values)
        modeltest.test_all()

but then it fails the standard model tests. We can fix in a separate issue

[weilongai]

For test_crack_propagation, you need to add auxiliary_domains={"secondary": "current collector"} to the Variable you define in CrackPropagation.get_fundamental_variables. The tests are written with pybamm.settings.debug_mode = True, which performs additional shape checks to make sure the model would be compatible with 2+1D. This isn't a problem in your notebooks because you only ever solve it in 1D.
For test_SPM and test_DFN, it seems like a parameter error, you can comment those out if you want and open a new issue specifically for fixing those.

Edit: the parameter values can be fixed by setting e.g.

        options = {"particle": "Fickian diffusion", "particle cracking": None}
        model = pybamm.lithium_ion.SPM(options)
        chemistry = pybamm.parameter_sets.Ai2020
        parameter_values = pybamm.ParameterValues(chemistry=chemistry)
        modeltest = tests.StandardModelTest(model, parameter_values=parameter_values)
        modeltest.test_all()

but then it fails the standard model tests. We can fix in a separate issue

Thanks! Any suggestions to improve the coverage of the functions in parameters? It is strange that they are not covered in /PyBaMM/tests/unit/test_parameters/test_parameter_sets/test_LCO_Ai2020.py

BTW, can I simply delete /PyBaMM/.github/release_checklist.md? I cannot find it in the pybamm main repository.

[brosaplanella]

To cover this, add a line either in test_base_battery_model.py (see test_options in it) and add a test that catches the error when you pass a bad cracking model.

[weilongai]

Strange because I have added one like in lines 169-171 of test_base_battery_model.py

        # crack model
        with self.assertRaisesRegex(pybamm.OptionError, "particle cracking"):
            pybamm.BaseBatteryModel({"particle cracking": "bad particle cracking"})

[brosaplanella]

Oh, I see. Then I think you need to add a line in test_options in test_lithium_ion_parameters.py and that should fix it. I think with this, the testing of the parameters and Rob's fix for the DFN & other tests the coverage should be pretty much sorted out.

[rtimms]

looks like the issue with the test is because for some reason c_s_rav has the wrong shape_for_testing:

ipdb> R0.shape_for_testing
(33, 1)
ipdb> R0.domain
['negative electrode']
ipdb> c_s_rav.domain
['negative electrode']
ipdb> c_s_rav.shape_for_testing
(11, 1)

[rtimms]

If you change the definition of _evaluate_for_shape in Integral (line 583 of unary_operators.py) to

    def _evaluate_for_shape(self):
        """ See :meth:`pybamm.Symbol.evaluate_for_shape_using_domain()` """
        return pybamm.evaluate_for_shape_using_domain(
            self.domain, self.auxiliary_domains
        )

then it fixes the tests. Weird that this hadn't caused an error before...

[brosaplanella]

I think we are almost there. To increase the coverage in the LiCo2 parameters you should include the cathode here, either in this model, or just copy the whole test and run it with cracking in the cathode. This will also improve the coverage in base_lithium_ion_model.py.

[brosaplanella]

If Rob's fix sorted out the issue with this test, then uncommenting these lines (for all models) should get the coverage sorted out.

[brosaplanella]

Looks good to me! Really nice work Weilong :)

[valentinsulzer]

Thanks for addressing all our comments @weilongai, this looks great now! We should still figure out at some point what the discrepancy is with your model in Matlab

[valentinsulzer]

@all-contributors add @weilongai for code, example, test

[allcontributors]

@tinosulzer

I've put up a pull request to add @weilongai! 🎉

[weilongai]

Thanks for all your help!