-
-
Notifications
You must be signed in to change notification settings - Fork 661
3D Thermal Model #5112
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
3D Thermal Model #5112
Conversation
@rtimms can you review this? (also run the CI wanted to see coverage) |
…into gsoc-3d-thermal
Codecov ReportAll 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? |
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.
thanks, looks great! just a few small changes
@@ -30,6 +30,8 @@ def battery_geometry( | |||
""" | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Basic3DThermalSPM
model 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-commit
nox -s tests
nox -s doctests