-
-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue 563 parameters by material #647
Conversation
some comments for discussion:
|
I like @Scottmar93 suggestion in #563 about defining an experiment. It is good to distinguish between the following: battery parameters (intrinsic of the battery materials or, from a maths point of view, of the equations), which could be split into further subcategories; geometry and those related to the experiment (how we cycle the battery). This poses some interesting questions about defining the SoC of the electrodes (or maybe we should use state of lithiation to avoid confusion) as they are related to the potentials. I agree also with @rtimms points above. In particular, I think that we should define a new parameter/function that accounts for the effectiveness of the medium, so then one can choose between different models. In particular, I think it would be interesting to use the values obtained via homogenisation using Rayleigh's multipole method (see Bruna & Chapman 2015). |
Codecov Report
@@ Coverage Diff @@
## master #647 +/- ##
==========================================
+ Coverage 98.49% 98.53% +0.03%
==========================================
Files 167 169 +2
Lines 8180 8255 +75
==========================================
+ Hits 8057 8134 +77
+ Misses 123 121 -2
Continue to review full report at Codecov.
|
…/pybamm-team/PyBaMM into issue-563-parameters-by-material
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great, thanks @tinosulzer !
"separator": "separator_Marquis2019", | ||
"cathode": "lico2_Marquis2019", | ||
"electrolyte": "lipf6_Marquis2019", | ||
"experiment": "1C_discharge_from_full_Marquis2019", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there should be a very brief description of the parameters that go into the experiment folder? Though maybe it is clear from looking at the existing parameter sets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think that a list of the required parameters in the documentation would be helpful.
@@ -0,0 +1,49 @@ | |||
# | |||
# Parameter sets from papers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very user-friendly!
Issue 600 interpolate
Description
Fixes #563
Type of change
Please delete options that are not relevant.
Key checklist:
$ flake8
$ python run-tests.py --unit
$ cd docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: