Issue 784 simplify solver#800
Conversation
Codecov Report
@@ Coverage Diff @@
## master #800 +/- ##
==========================================
+ Coverage 98.29% 98.41% +0.11%
==========================================
Files 177 179 +2
Lines 9746 9910 +164
==========================================
+ Hits 9580 9753 +173
+ Misses 166 157 -9
Continue to review full report at Codecov.
|
…BaMM into issue-784-simplify-solver
Scottmar93
left a comment
There was a problem hiding this comment.
Looks really good @tinosulzer. Certainly much simpler to work out what is going on in the solvers now by getting rid of the DAESolver and ODESolver classes and those multiple set_ups!! Also really like that you have tidied up external variables and have them and the input parameters treated in a similar way.
| ) | ||
| ) | ||
|
|
||
| def solve(self, model, t_eval, external_variables=None, inputs=None): |
There was a problem hiding this comment.
Does external variables actually work when just solving? I thought it was only implemented for "stepping"
There was a problem hiding this comment.
Yes, it should work identically with both now - though I guess the main use cases will be for stepping
Description
Simplify the various solver classes by merging the
set_upmethods and getting rid ofcompute_solution. Also combined the implementations for external variables and inputsFixes #784
Fixes #789
Fixes #463
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: