solid tau now uses correct bruggeman coeff

[dion-w]

Description

Solid tortuosity now uses solid bruggeman coefficient of respective electrode and not the bruggeman coefficients of the electrolyte.

Fixes #1734

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)
• Added line to relevant section of changelog

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
• not deemed necessary as this only changes two lines of code in the respective directory

[dion-w]

Since this is my first pull request i appreciate feedback for handling this process.

[brosaplanella]

Looks good, but there is a test related to the sensitivities which is not passing. @martinjrobins is it just a matter of the tolerances or is there a bigger issue?

[valentinsulzer]

Let's try using even tighter tolerances for solving in the sensitivities test: change these both to 1e-12

PyBaMM/tests/integration/test_models/standard_model_tests.py

Lines 113 to 114 in 269629f

	self.solver.rtol = 1e-8
	self.solver.atol = 1e-8

Also, could you make a new section "Unreleased" for the changelog? i.e. at the top add

# [Unreleased](https://github.com/pybamm-team/PyBaMM/)

## Bug fixes
- Solid tortuosity is now correctly calculated with Bruggeman coefficient of the respective electrode ([#1773](https://github.com/pybamm-team/PyBaMM/pull/1773))

Since these changes aren't in the 21.10 release

[valentinsulzer]

I tested this locally and the sensitivity test passed. Can you make the changelog update, merge develop, and push again?

[valentinsulzer]

@dion-w any update on this?

[dion-w]

Haven't been active here recently. Will hopefully be after a deadline on thursday.

[valentinsulzer]

Thanks @dion-w !

[brosaplanella]

@all-contributors please add @dion-w for bug reports and code

[allcontributors]

@brosaplanella

I've put up a pull request to add @dion-w! 🎉

[brosaplanella]

The notebook tests are failing because we need to provide the Bruggeman parameters for the electrode in cell [16] of https://github.com/pybamm-team/PyBaMM/blob/develop/examples/notebooks/parameterization/parameterization.ipynb

[codecov]

Codecov Report

Merging #1773 (203c350) into develop (15992b3) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1773   +/-   ##
========================================
  Coverage    99.26%   99.26%           
========================================
  Files          345      345           
  Lines        18900    18900           
========================================
  Hits         18761    18761           
  Misses         139      139           

Impacted Files	Coverage Δ
...odels/submodels/tortuosity/bruggeman_tortuosity.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15992b3...203c350. Read the comment docs.