Issue 579 volume fractions #726
Conversation
valentinsulzer
changed the base branch from
master
to
issue-699-remove-autograd
Codecov Report
@@ Coverage Diff @@
## issue-699-remove-autograd #726 +/- ##
============================================================
Coverage ? 98.15%
============================================================
Files ? 175
Lines ? 9264
Branches ? 0
============================================================
Hits ? 9093
Misses ? 171
Partials ? 0
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
thanks @tinosulzer looks great! just a comment on naming... I think tortuosity is defined so that e.g. D_eff = (eps/tau)D where eps is the porosity and tau is the tortuosity. Maybe the naming is ok or there is some other standard name for the correction factor for the effective properties?
#547 remove sto times zero
There was a problem hiding this comment.
Thanks @tinosulzer, it looks great! Apart from the two minor comments below, I agree with Robert about the tortuosity definition. Personally, I prefer the mathematical definition you used because if we ever include anisotropy and allow that factor to be a tensor then the tortuosity would require to invert the tensor, which is not ideal. In addition, when you homogenise, the factor/tensor appears multiplying. So I would suggest leaving the mathematical expressions as they are but calling them something else to not confuse people. I don't know if there is an agreed term in the literature, I can ask James LeHoux from Southampton who did a quite thorough literature review on the topic.
|
|
||
| eps_n, eps_s, eps_p = eps.orphans | ||
| tor = pybamm.Concatenation( | ||
| eps_n ** param.b_n, eps_s ** param.b_s, eps_p ** param.b_p |
There was a problem hiding this comment.
If I understand it correctly, here all the parameters param.b_j are the same for electrode and electrolyte in a given part. I think that is not necessarily true because the geometry that defines the tortuosity is quite different between the electrode and electrolyte. Would it be possible to pass each b_j as an argument when you call a Bruggeman tortuosity model?
There was a problem hiding this comment.
Would it just be a case of having a different parameter for electrolyte and solid?
There was a problem hiding this comment.
Yes. Also, they could be different for different transport phenomena in the same domain. Right now it is not a problem as we only need one number for electrode and one for electrolyte, but later on we might need more.
| @@ -277,6 +277,11 @@ def U_p_dimensional(sto, T): | |||
| pybamm.FullBroadcast(epsilon_s, ["separator"], "current collector"), | |||
| pybamm.FullBroadcast(epsilon_p, ["positive electrode"], "current collector"), | |||
| ) | |||
| epsilon_s_n = pybamm.Parameter("Negative electrode active material volume fraction") | |||
| epsilon_s_p = pybamm.Parameter("Positive electrode active material volume fraction") | |||
| epsilon_binder_n = 1 - epsilon_n - epsilon_s_n | |||
There was a problem hiding this comment.
Minor point: it might be more accurate to call it epsilon_inactive_n.
|
Tortuosity is a mess in terms of a consistent definition in the literature. This paper explains a common misinterpretation and distinguishes between tortuosity being the ratio of the diffusion path length and domain length and tortuosity factor being this ratio squared. http://www.sciencedirect.com/science/article/pii/0009250989850535 |
Issue 699 remove autograd
|
Thanks Tom for the info. It sounds like there are lots of different options and it would be interesting to implement a few and compare their impact on the DFN (at the low C-rates we have been considering so far electrolyte diffusion is a higher order effect, but it could play a big role if we start to consider higher C rates). |
|
I think tortuosity is ok
…On Wed, 13 Nov 2019, 13:31 Valentin Sulzer, ***@***.***> wrote:
Thanks Tom for the info. It sounds like there are lots of different
options and it would be interesting to implement a few and compare their
impact on the DFN (at the low C-rates we have been considering so far
electrolyte diffusion is a higher order effect, but it could play a big
role if we start to consider higher C rates).
For this particular issue I'm just going to focus on getting the framework
in place to allow adding more submodels. What would you suggest calling
these submodels (and corresponding variables) instead of "tortuosity"?
Maybe "effective transport factor"?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#726?email_source=notifications&email_token=ABV5YRKQ37L6EVH3XURE3A3QTP6S3A5CNFSM4JMMOUL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED6EWHY#issuecomment-553405215>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABV5YRMGPMLYGUAVN5ZRPR3QTP6S3ANCNFSM4JMMOULQ>
.
|
Description
Add tortuosity submodels, and add active/inactive material volume fractions
Fixes #579
Fixes #648
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.
Key checklist:
$ flake8$ python run-tests.py --unit$ cd docsand then$ make clean; make htmlYou can run all three at once, using
$ python run-tests.py --quick.Further checks: