Conversation
|
@rtimms can you review this? (also run the CI wanted to see coverage) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5112 +/- ##
=========================================
Coverage 99.12% 99.12%
=========================================
Files 309 310 +1
Lines 24136 24313 +177
=========================================
+ Hits 23924 24101 +177
Misses 212 212 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@rtimms , I've addressed the reviews and coverage in unit tests. Can you please re-review it? |
rtimms
left a comment
There was a problem hiding this comment.
thanks, looks great! just a few small changes
| """ | ||
| if options is None or type(options) == dict: # noqa: E721 | ||
| options = pybamm.BatteryModelOptions(options) | ||
| if options["cell geometry"] == "cylindrical": |
There was a problem hiding this comment.
Since form_factor is a kwarg of this method, I don't think we should update it in this way. Instead, we should catch it earlier and explicitly pass form_factor to battery_geometry
Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com>
…into gsoc-3d-thermal
|
@rtimms I've addressed all reviews, can you please re-review it? |
Description
Added a
Basic3DThermalSPMmodel with two way coupling.Fixes # (issue)
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)
Important checks:
Please confirm the following before marking the PR as ready for review:
nox -s pre-commitnox -s testsnox -s doctests