-
-
Notifications
You must be signed in to change notification settings - Fork 534
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
Variable scale and reference #2440
Conversation
Codecov ReportBase: 99.72% // Head: 99.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## more-simplifications #2440 +/- ##
=====================================================
Coverage 99.72% 99.72%
=====================================================
Files 258 258
Lines 19211 19258 +47
=====================================================
+ Hits 19158 19205 +47
Misses 53 53
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
Looks good, but the dimensional DFN is not there yet, is it?
@@ -739,6 +741,9 @@ def process_dict(self, var_eqn_dict): | |||
Equations ({variable: equation} dict) to dicretise | |||
(can be model.rhs, model.algebraic, model.initial_conditions or | |||
model.variables) | |||
ics : bool, optional | |||
Whether the equations are initial conditions. If True, the equations are |
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.
Whether the equations are initial conditions. If True, the equations are | |
Whether the equations have initial conditions. If True, the equations are |
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 meant to be are
, this is the function that discretises the equations (rhs, algebraic, initial conditions) and there is a special case for initial conditions (reference only gets taken away from initial conditions)
@@ -422,7 +422,7 @@ def kappa_e(self, c_e, T): | |||
kappa_scale = self.F**2 * self.D_e_typ * self.c_e_typ / (self.R * self.T_ref) | |||
return self.kappa_e_dimensional(c_e_dimensional, self.T_ref) / kappa_scale | |||
|
|||
def chiT_over_c(self, c_e, T): | |||
def chiRT_over_Fc(self, c_e, T): |
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.
What's the meaning of Fc?
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.
Faraday's constant times c
I removed the dimensional DFN since when we eventually dimensionalize all the models it will not be there, so I don't want people adding it to their code. This PR now only adds the scaling and reference |
Description
Adds
scale
andreference
attributes to a variable, which are used to multiply and/or add a value to a variable to make the resulting numerics better conditioned.Progress towards #2418
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 docs
and then$ make clean; make html
You can run all three at once, using
$ python run-tests.py --quick
.Further checks: