Solid-Energy-Systems / NewareNDA

Python module and command line tool for reading and converting Neware nda and ndax battery cycling data files.
BSD 3-Clause "New" or "Revised" License
15 stars 8 forks source link

_interpolate_data() function produces strange results and misleading warning #79

Closed ianrnorthvolt closed 1 month ago

ianrnorthvolt commented 2 months ago

When using the read_ndax() function without interpolating (I commented out the line which calls that function), the output is like so: image

But the output when using the interpolate_data() function is like so: image

There are several strange things happening when interpolating: 1) The first few Charge_ and Discharge_Capacity values are changed to NaN values. 2) The Cumulative Charge Energy and Charge Capacity are summed (but not quite exactly...) 3) The timestamp is now non-monotonic between indices 2 -> 4 (pandas index, not Neware index). 4) The increase in time between (pandas) index 1 and 2 (0.6s) does not match the increase in timestamp (5s). 5) The Charge Capacity is positive in a CC_DChg step (pandas index 3), and then decreases within the same step to zero (pandas index 4).

Furthermore, I am a little confused about when the warning for interpolating data appears. The first clause in the _interpolate_data() function is:

nan_mask = df["Time"].notnull()

And if this is True for any row, then the warning is raised. But surely the interpolation warning (and the interpolation itself) should only happen if this is False (i.e. when none of the rows of Time are null). Then further on in the function, lots of replacing of values happens for rows where Time and TimeStamp is not null, but why are these ones being replaced? Surely these notnull() calls should be isnull()?

I have maybe misunderstood, so if this is the case, could you explain why these weird things are happening?

Thanks, Ian

d-cogswell commented 2 months ago

Hi @ianrnorthvolt the issue here is that the latest Neware equipment doesn't appear to save all of the capacity and energy fields, probably to save on file size. There are just not enough bytes in this new format to hold the data. If you turn off _interpolate_data() you will see just the data extracted from the ndax file, and some fields will have many NaNs. Interestingly if you load the same file in BTSDA, you will see values in place of those NaNs, but the data is perfectly linear with no noise. I believe it's being generated by BTSDA in post-processing and is not an actual measurement (incredibly sketchy in my opinion).

The warning is because NewareNDA is calling _interpolate_data() to try and recreate this data which doesn't actually exist (as far as I can tell).

You may have found bugs in this routine, which is fairly new and difficult to test. I will do some sanity checking on my end. Would you be willing to share your ndax file?

Thanks!

d-cogswell commented 2 months ago

@ianrnorthvolt can you check if your file has any NaN in it when loaded with _data_interpolation() turned off? If your dataset is complete, maybe this function shouldn't be called.

ianrnorthvolt commented 2 months ago

Hi Dan, thanks for getting back to me so fast!

The file does not have any NaNs with _data_interpolation() turned off. I checked all the individual 3 files (data_df, runInfo_df and step_df) and there is no NaN data there whatsoever. This is why I think that the notnull() function used in the _data_interpolation() function is so strange. Why is there any interpolation happening if there are no null values?

I am afraid I cannot share the ndax file with you, it is sensitive data.

d-cogswell commented 2 months ago

Hi Ian, this makes sense. I haven't seen many files in this new format, and I incorrectly assumed they all had NaNs. I just pushed a commit to the development branch 269d9c4 which checks for NaNs before interpolating. Can you check if it resolves the problem?

ianrnorthvolt commented 2 months ago

Hi Dan, that fix works well, thanks!

I have now run into some larger files which required some interpolation, which are causing other issues. But I can try to explain what is going on in a different issue.

Once the change you made is merged in, I will close this issue. Thanks again!