Issue 569 create model notebooks #877
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #877 +/- ##
========================================
Coverage 97.79% 97.79%
========================================
Files 210 210
Lines 10585 10587 +2
========================================
+ Hits 10352 10354 +2
Misses 233 233
Continue to review full report at Codecov.
|
rtimms
requested review from
valentinsulzer,
Scottmar93,
brosaplanella and
TomTranter
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Note that the keys of this dictionary are strings (i.e. the names of the variables). The model is now completely defined and is ready to be discretised and solved!" |
There was a problem hiding this comment.
could point out the "name" here can be different to the string passed to define the variable
| ], | ||
| "source": [ | ||
| "disc = pybamm.Discretisation() # use the default discretisation\n", | ||
| "disc.process_model(model)" |
There was a problem hiding this comment.
can use ; to suppress output
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "We then create a mesh using the `pybamm.MeshGenerator` class. As inputs this class takes the type of mesh and any parameters required by the mesh. In this case we choose a uniform one-dimensional mesh which doesn't require any parameters. Example of meshes that do require parameters include the `pybamm.Exponential1DSubMesh` which clusters points close to one or both boundaries using an exponential rule. It takes a parameter which sets how closely the points are clustered together. " |
There was a problem hiding this comment.
maybe show an inline example of the exponential 1D submesh?
| " \"negative particle\": {\n", | ||
| " \"primary\": {r: {\"min\": pybamm.Scalar(0), \"max\": pybamm.Scalar(1)}}\n", | ||
| " }\n", | ||
| "}" |
There was a problem hiding this comment.
Not for this issue but it feels like there's one too many nested dictionaries in the geometry. I can't think of how to reformat to make it clearer while keeping the existing functionality, but maybe someone else can?
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "In the [next notebook](/5-comparing-full-and-reduced-order-models.ipynb) we consider the limit of fast diffusion in the particle. This leads to a reduced-order model for the particle behaviour, which we compare with the full (Fickian diffusion) model. " |
There was a problem hiding this comment.
this link is broken (should be 4- not 5-)
| "source": [ | ||
| "We can now populate the variables dictionary of both models with any variables of interest. We can compute the average concentration in the full model using the operator `pybamm.r_average`. We may also wish to compare the concentration profile predicted by the full model with the uniform concentration profile predicted by the reduced model. We can use the operator `pybamm.PrimaryBroadcast` to broadcast the scalar valued uniform concentration across the particle domain so that it can be visualised as a function of $r$. \n", | ||
| "\n", | ||
| "Note: the \"Primary\" refers to the fact the we are broadcasting in only one dimension. For some models, such as the DFN, variables may depend on a \"pseudo-dimension\" (e.g. the position in $x$ across the cell), but spatial operators only act in the \"primary dimension\" (e.g. the position ion $r$ within the particle). If you are unfamiliar with battery models, do not worry, the details of this are not important for this example. " |
There was a problem hiding this comment.
link to the broadcasts notebook (in the expression tree folder)?
There was a problem hiding this comment.
I think there is a typo where it says "(e.g. the position ion
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "In the [next notebook](/5-a-simple-SEI-model.ipynb) we consider a simple model for SEI growth, and show how to correctly pose the model in non-dimensional form and then create and solve it using pybamm." |
There was a problem hiding this comment.
this link is also broken
| "| $c_0$ | mol m$^{-3}$ | $2.5 \\times 10^{4}$ |\n", | ||
| "\n" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
comment on the fact that all battery models in pybamm are dimensionless for better numerical conditioning and that this will be explained further in [link to notebook]
There was a problem hiding this comment.
Great job Rob!
I encountered some issues when rendering it in GitHub, not sure if this is just me or if I should be doing it in another way. If so, please just ignore those comments.
Apart from that, just a few typos and also the references format are not consistent: the DFN paper appears at the very end of the notebook while the Marquis et al. paper appears right after it has been referenced.
| ] | ||
| }, | ||
| { | ||
| "cell_type": "code", |
There was a problem hiding this comment.
Is this final code cell necessary?
There was a problem hiding this comment.
I have noticed it appears in other notebooks too
| "In the [previous notebook](./1-an-ode-model.ipynb) we show how to create, discretise and solve an ODE model in pybamm. In this notebook we show how to create and solve a PDE problem, which will require meshing of the spatial domain.\n", | ||
| "\n", | ||
| "As an example, we consider the problem of linear diffusion on a unit sphere,\n", | ||
| "\\begin{equation*}\n", |
There was a problem hiding this comment.
This LaTeX equation does not render (at least to me) when I visualise it in GitHub. The one in the ODE example worked fine, it might be a syntax issue...
| "metadata": {}, | ||
| "source": [ | ||
| "In the [previous notebook](./2-a-pde-model.ipynb) we saw how to solve a PDE model in pybamm. Now it is time to solve a real-life battery problem! We consider the problem of spherical diffusion in the negative electrode particle within the single particle model. That is,\n", | ||
| "\\begin{equation*}\n", |
There was a problem hiding this comment.
Similarly to the previous notebook, they do not render to me.
| "source": [ | ||
| "We can now populate the variables dictionary of both models with any variables of interest. We can compute the average concentration in the full model using the operator `pybamm.r_average`. We may also wish to compare the concentration profile predicted by the full model with the uniform concentration profile predicted by the reduced model. We can use the operator `pybamm.PrimaryBroadcast` to broadcast the scalar valued uniform concentration across the particle domain so that it can be visualised as a function of $r$. \n", | ||
| "\n", | ||
| "Note: the \"Primary\" refers to the fact the we are broadcasting in only one dimension. For some models, such as the DFN, variables may depend on a \"pseudo-dimension\" (e.g. the position in $x$ across the cell), but spatial operators only act in the \"primary dimension\" (e.g. the position ion $r$ within the particle). If you are unfamiliar with battery models, do not worry, the details of this are not important for this example. " |
There was a problem hiding this comment.
I think there is a typo where it says "(e.g. the position ion
| "deletable": true, | ||
| "editable": true | ||
| }, | ||
| "metadata": {}, | ||
| "source": [ | ||
| "" |
There was a problem hiding this comment.
The image doesn't render
| "deletable": true, | ||
| "editable": true | ||
| }, | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Once we have stated the equations, we can add them to the `model.rhs` dictionary. This is a dictionary which stores the right had sides of the governing equations. Each key in the dictionary corresponds to the variable which the equation is being solved for." |
There was a problem hiding this comment.
Typo: should be "right hand side"
| "deletable": true, | ||
| "editable": true | ||
| }, | ||
| "metadata": {}, | ||
| "source": [ | ||
| "We can also output the dimensional versions of these variables by multiplying by the scalings in the Non-dimensionalisation section. By convention, we recommend include in the units in the output variables name so that they do not overwrite the dimensionless output variables. We also `.update` the disctionary so that we add to the previous output variables." |
There was a problem hiding this comment.
Typo: it should be "dictionary"
| "deletable": true, | ||
| "editable": true | ||
| }, | ||
| "metadata": {}, | ||
| "source": [ | ||
| "And thats it, the model is fully defined and ready to be used. If you plan on reusing the model several times, you can additionally set model defaults which include: a default geometry to run the model on, a default set of parameter values, a default solver, etc." |
There was a problem hiding this comment.
Typo: should be "that's"
| @@ -18,7 +18,7 @@ | |||
| "cell_type": "markdown", | |||
| "metadata": {}, | |||
| "source": [ | |||
| "The SPM consists of two spherically symmetric diffusion equations: one within a representative negative particle ($\\text{k}=\\text{n}$) and one within a representative positive particle ($\\text{k}=\\text{p}$). In the centre of the particle the classical no-flux condition is imposed. Since the SPM assumes that all particles in an electrode behave in exactly the same way, the flux on the surface of a particle is simply the current $I$ divided by the thickness of the electrode $L_{\\text{k}}$. The concentration of lithium in electrode $\\text{k}$ is denoted $c_{\\text{k}}$ and the current is denoted by $I$. All parameters in the model stated here are dimensionless and are given in terms of dimensional parameters at the end of this notebook. The model equations for the SPM are then: \n", | |||
| "The SPM consists of two spherically symmetric diffusion equations: one within a representative negative particle ($\\text{k}=\\text{n}$) and one within a representative positive particle ($\\text{k}=\\text{p}$). In the centre of the particle the standard no-flux condition is imposed. Since the SPM assumes that all particles in an electrode behave in exactly the same way, the flux on the surface of a particle is simply the current $I$ divided by the thickness of the electrode $L_{\\text{k}}$. The concentration of lithium in electrode $\\text{k}$ is denoted $c_{\\text{k}}$ and the current is denoted by $I$. All parameters in the model stated here are dimensionless and are given in terms of dimensional parameters at the end of this notebook. The model equations for the SPM are then: \n", | |||
There was a problem hiding this comment.
This one does not render either.
I always have problems viewing notebooks directly in github. I'm not sure what the fix is for this? |
I don't know either. If they look fine locally then it is OK. I guess they will work fine in the JupyterHub (is it up and running already?) |
It is up, but not "live" |
Description
Added some more notebooks covering creating models, and comparing full and reduced-order models. Also fixes some typos in other notebooks
Fixes #569 #672
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.
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: