NMC/Gr bug fix & Switch to cycle_counts vector#763
Conversation
brtietz
left a comment
There was a problem hiding this comment.
This mechanism for cycle counting makes more sense to me, and actually clarifies some of the NMC code.
The major required change before merging is to re-work lib_battery_lifetime_nmc_test, BTMSTest to not have local paths. More comments are nice as you have time - I think the one in the calendar cycle header file will save head scratching later.
|
I'm happy to add other comments where you think they would be helpful, any suggestions? I removed the BTMS test that was based on a bunch of large local files, which I had committed by accident. |
The other one that came to mind was indicating what the DOD indices are in the NMC and LMO/LTO cpp files the first time they are used ex: |
brtietz
left a comment
There was a problem hiding this comment.
Approved, conditional on the tests passing.
|
Thanks, I've replaced all implicit indexes with the ENUMs in the shared library. |
There was a bug in the calculation of the b1 coefficient in #748 that is fixed here. There were also some adaptations to get the Ua and Voc calculations to be more aligned. Many results in tests have changed.
This PR also changes the NMC/Gr life model to use the
cycle_countsstructure in the cycle model, rather than maintain its owncycle_DOD_rangevector. However, the usage is slightly different from the cycle-calendar model. The latter is tracking a frequency table of cycles at the DODs given by the input cycle matrix for the whole simulation duration. Whereas the NMC/Gr model will be tracking each cycle separately (essentially the count is always 1 and the DODs are ordered by when the cycle is completed) and for one day at a time.Needing to easily append new rows to the cycle_count matrix led to my switching that structure from a matrix_t to a 2d vector, so I also added some helper functions for translating between the two.