-
-
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
Issue 1575 discharge energy #1969
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1969 +/- ##
========================================
Coverage 99.32% 99.32%
========================================
Files 346 346
Lines 19064 19078 +14
========================================
+ Hits 18936 18950 +14
Misses 128 128
Continue to review full report at Codecov.
|
@tinosulzer this is a lot of code changes just for one calculation. Did it really slow things down so much to warrant making it an option? |
Here are the benchmarks with the discharge energy calculation: https://github.com/pybamm-team/PyBaMM/runs/5458023045?check_suite_focus=true It could probably be done as a post-processing step, with low overhead, but that would be a bigger change |
It's not that many changes? Most of them are just passing |
ok if you think might be useful anyway. I just prefer not having too many options |
Description
Add "Discharge energy [W.h]" as an output variable (integral of power).
Set option "calculate discharge energy" to "true" to turn on. It's false by default as calculating it can be slow.
Fixes #1575
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: