I3790 idaklu outvars#3803
Conversation
jsbrittain
requested review from
TomTranter,
martinjrobins,
rtimms and
valentinsulzer
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3803 +/- ##
========================================
Coverage 99.60% 99.60%
========================================
Files 258 259 +1
Lines 21226 21287 +61
========================================
+ Hits 21142 21203 +61
Misses 84 84 ☔ View full report in Codecov by Sentry. |
valentinsulzer
left a comment
valentinsulzer
left a comment
There was a problem hiding this comment.
Looks good to me, though I think Symbol.contains duplicates the existing Symbol.has_symbol_of_classes. Might be worth benchmarking to see which implementation is fastest (I'm guessing contains when there is a match since it returns immediately)
pybamm/expression_tree/symbol.py
Outdated
| def contains(self, expr_type): | ||
| """ | ||
| Does the symbol tree contain a given node type? | ||
| """ | ||
| if type(self) is expr_type: | ||
| return True | ||
| elif "children" in dir(self): | ||
| for child in self.children: | ||
| if child.contains(expr_type): | ||
| return True | ||
| return False | ||
|
|
There was a problem hiding this comment.
how is this different from has_symbol_of_classes ?
There was a problem hiding this comment.
@tinosulzer I hadn't spotted that method - I've reverted to using has_symbol_of_classes as the more pythonic implementation to avoid duplicating this behaviour.
martinjrobins
left a comment
martinjrobins
left a comment
There was a problem hiding this comment.
looks great, thanks @jsbrittain
4 participants
Description
When run with
output_variables, the IDAKLU solver does not fail gracefully when requesting a variable that was not specified in the requested variables list. Note that 'model parameters' that do not appear inoutput_variablesshould still be returned upon request.Fixes #3790
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:
$ pre-commit run(or$ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all(or$ nox -s tests)$ python run-tests.py --doctest(or$ nox -s doctests)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick(or$ nox -s quick).Further checks: