Skip to content
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

Add a number of features to the cycle statistics data #303

Open
agerwick opened this issue Mar 11, 2024 · 6 comments
Open

Add a number of features to the cycle statistics data #303

agerwick opened this issue Mar 11, 2024 · 6 comments
Assignees
Milestone

Comments

@agerwick
Copy link

We would like to add a number of features on cycle statistics.
Most are just simple aggregations (Min/Max/Mean), Total Charge/Discharge/Cycle/Rest time, etc., but we also have some more ambitious goals :)
We would be happy to help adding these features.

@jepegit jepegit assigned jepegit and agerwick and unassigned jepegit Mar 12, 2024
@jepegit
Copy link
Owner

jepegit commented Mar 12, 2024

Yes, that sounds good.

@jepegit jepegit added this to the v1.0.2 milestone Mar 12, 2024
@jepegit
Copy link
Owner

jepegit commented Apr 25, 2024

@agerwick, I am moving this to a later release milestone. Need to make a v1.0.2 release very soon - there has been an accumulation of small bugs in the batch-processing utility that needs fixing...

@jepegit jepegit modified the milestones: v1.0.2, v.1.1.0 Apr 25, 2024
@agerwick
Copy link
Author

@jepegit No worries, I didn't get time to work on it yet, but it's coming...

@valentinsulzer
Copy link

More generally, is it possible to customize the summary features locally by passing in arguments to make_summary, or is the only way to overwrite the cellpy code?

@jepegit
Copy link
Owner

jepegit commented Jun 28, 2024

I guess we can conclude that the make_summary method needs to be updated:

Starting with #315 probably makes most sense? Or maybe both should be implemented within one branch?

@morrowrasmus
Copy link
Contributor

After having given this some more thought, I don't think #315 is the way to achieve what we want after all. In the summary-table, the calculations are explicitly done (mostly in the _generate_absolute_summary_columns()-method), and keeping additional columns would only be a pre-requisite to calculate more values for the summary-table and not achieve anything by itself. It might be a idea to still implement #315 so the user has a way to receive a cleaner output of choice, but from our point of view that would not be a priority (we can clean up the table later). Maybe @valentinsulzer has some input on what he would want included that might change that?

Instead of the user specifying the columns to keep, I think what we really want to do is to expand both the steps and summary-tables to include more things (and thus also expand the two lists of columns to keep for the two tables, respectively, as needed):

steps:

summary:

  • include more simple aggregations as @agerwick suggested, for example min/max potential and initial/end potential on charge/discharge (can be added directly from the steps-table). This does create some redundancy as the data will be present in both the steps and summary table, but it provides (in my opinion) a more convenient way of accessing this data
  • include the CV-share and other CV-share-related metrics (following the implementation of Split single-step CCCV-steps into CC and CV substeps #312)
  • include calculated IR and related values (following implementation of Estimate internal resistance at start and end of (dis)charge #313)
  • include time-based values (e.g. total charge time, discharge time, rest time etc. for the cycle)

Apart from the previous issues on CV-share and IR-calculations, I think most of this should be fairly straightforward to implement (maybe except the weighted averages which I struggled to implement into the call to .agg() when I played around a little with it).

Maybe we should close #315, keep this issue open for the changes to the summary-table and create a new issue for the changes to the steps-table?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants