TRI-AMDD / beep

Battery evaluation and early prediction
Apache License 2.0
129 stars 59 forks source link

[WIP] update scipy version #794

Open JosephMontoya-TRI opened 1 year ago

JosephMontoya-TRI commented 1 year ago

Changes proposed in this pull request:

ardunn commented 1 year ago

@JosephMontoya-TRI so according to this: https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.interp1d.html

I think the newest scipy changes what slinear does, though I'm not exactly sure how it differs from linear.

Just changing the interpolation keyword to be "linear" removes most of the errors for 'duplicate x', and the remaining seem to be problems with the precision of the interpolation like this:

for col_name in y_at_point.columns:
>           self.assertAlmostEqual(
                pred[col_name].iloc[0], y_at_point[col_name].iloc[0],
                places=2
            )
E           AssertionError: 3320.0375418686945 != 3319.9971375 within 2 places (0.040404368694453296 difference)

Edit: scipy linear does not do what I thought it did. Ignore above

JosephMontoya-TRI commented 1 year ago

Yeah I came to a similar conclusion the last time I tried this (which was like 3 years ago). I think we should probably just update the tests, change the interpolation keyword, and cross our fingers that this isn't breaking too many things downstream (if it's too problematic we can always change it back). @shijingsun-TRI @pkherring does that sound okay?

pkherring commented 1 year ago

Yeah, I agree. Just update the test values and upgrade scipy