-
-
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
#1343 update get_termination_reason to return solution #1344
Conversation
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.
Ahh that makes sense, thanks for fixing
pybamm/solvers/base_solver.py
Outdated
# to a different event - this isn't captured here (or in the logger info) | ||
solutions[0], termination = self.get_termination_reason( | ||
solutions[0], model.events | ||
) |
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.
isn't this a trivial for loop?
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.
Yeah, I’ll add it. For the logger info shall i just give different info when it’s a list of inputs (ie don’t specify a termination reason since it may be different for each solution).
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.
Yeah sounds good
Codecov Report
@@ Coverage Diff @@
## develop #1344 +/- ##
========================================
Coverage 98.14% 98.14%
========================================
Files 272 272
Lines 15561 15566 +5
========================================
+ Hits 15273 15278 +5
Misses 288 288
Continue to review full report at Codecov.
|
Description
Return the solution as well as the termination reason in
get_termination_reason
. This is so that the updated solution (solution = solution + event_sol
) which includes the event state gets returned by the solver.Fixes #1343
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: