Issue 1195 track cycles in solution #1309
Conversation
…ycles-in-solution
Codecov Report
@@ Coverage Diff @@
## develop #1309 +/- ##
========================================
Coverage 98.10% 98.10%
========================================
Files 272 272
Lines 15214 15229 +15
========================================
+ Hits 14925 14940 +15
Misses 289 289
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @lonnbornj , impressed how few changes you managed to do this in.
Just some edge cases to address. Also, can you edit the cccv.py and gitt.py examples to use cycles, since we want to encourage this in general, and add this change to the changelog.
pybamm/experiments/experiment.py
Outdated
| cond for cycle in operating_conditions for cond in cycle | ||
| ] | ||
| elif all([isinstance(cond, str) for cond in operating_conditions]): | ||
| self.cycle_lengths = [len(operating_conditions)] |
There was a problem hiding this comment.
What about the case where there is a mix of tuples and strings? This should either raise an error, or strings should be made into 1-tuples (i.e. cycles of length 1). Second option slightly better IMO.
There was a problem hiding this comment.
I've implemented a check for the cycles (tuple or str), and the conditions therein (str), and convert any strings to 1-tuples as suggested. The type-checking got a bit complicated so I hope my solution is not too opaque. I thought it made sense to keep all of this together, so I moved the check for string operating_conditions out of experiment.read_operating_conditions and do it all in __init__.
|
This is a really neat solution, thanks @lonnbornj ! I think it would be good to add a "Getting Started" guide on how to use experiments, explaining the syntax for cycles etc., but this can be done as a separate issue. |
|
Actually, just realised we already have an experiment guide here, so it can just be updated to show how to use cycles. |
There was a problem hiding this comment.
Thanks @lonnbornj , looks good! Happy to merge, unless you want to update the tutorial @rtimms linked to
|
@all-contributors add @lonnbornj for code, test, example |
|
@tinosulzer I've put up a pull request to add @lonnbornj! 🎉 |
Description
Altered the Experiment class to parse experiments expressed in terms of cycles (in the form of tuples of operating conditions). The Simulation class creates
solution.cycles-- a list of cycles. Each cycle is a tuple ofsub_solutionobjects.Fixes #1195
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: