WillianFuks / tfcausalimpact

Python Causal Impact Implementation Based on Google's R Package. Built using TensorFlow Probability.
Apache License 2.0
600 stars 72 forks source link

Breaking change with tfp==0.14.0 #26

Closed rj678 closed 2 years ago

rj678 commented 2 years ago

Hi @WillianFuks , just wanted to give you a heads up that the latest version of tfp, 0.14.0 released on Sep 15, 2021 causes some examples in this repo to break.

For e.g., this code:

import pandas as pd
from causalimpact import CausalImpact

data = pd.read_csv('https://raw.githubusercontent.com/WillianFuks/tfcausalimpact/master/tests/fixtures/comparison_data.csv', index_col=['DATE'])
pre_period = ['2019-04-16', '2019-07-14']
post_period = ['2019-7-15', '2019-08-01']
ci = CausalImpact(data, pre_period, post_period, model_args={'fit_method': 'hmc'})

gives this error:

ValueError: Pandas DataFrame or Series has a DatetimeIndex with no set frequency, but STS requires regularly spaced observations. Consider using `tfp.sts.regularize_series` to infer a frequency and build a regularly spaced series (by marking unobserved steps as missing observations).

for now, I downgraded tfp to 0.13.0 in my conda env, and that seems to have fixed the issue - a quick fix might be to fix the version of tfp that is installed along with this repo.

thanks,

WillianFuks commented 2 years ago

Thanks for the warning @rishi1016 !

I'll try to work on that tomorrow.

WillianFuks commented 2 years ago

Hi @rishi1016 ,

Took me longer than expected but finally the update is ready. Could you please test it to see if it's working for you now? A new rc is available at:

pip install tfcausalimpact==0.0.7rc1

As soon as you confirm it's working I'll merge it to master and make the final release.

Thanks,

Will

rj678 commented 2 years ago

Hi @WillianFuks - I tested this with the new versions of tfcausalimpact and tfp, and can confirm that the above example now returns the expected output. Thanks so much for fixing this!

FYI - with fit_method: vi, the fitting (CausalImpact()) took 18 secs, while it took 80 secs with hmc - hope that looks right to you

WillianFuks commented 2 years ago

Hi @rishi1016 ,

Thanks for the follow up. I just published the new code on pypi.

As for the time consumed on each algorithm, this is quite expected indeed. hmc is much slower but more accurate than vi.

Best Regards,

Will