ar_coefficient calling params

blue-yonder / tsfresh

Automatic extraction of relevant features from time series:

http://tsfresh.readthedocs.io

MIT License

8.42k stars 1.21k forks source link

ar_coefficient calling params #786

Open ghost opened 3 years ago

ghost commented 3 years ago

Hello,

I am using tsfresh 0.17 from pip. My os is suse SLED 15.2.

I noticed while browsing the source code for the ar_coefficient extractor that tsfresh is calling the statsmodels function with the following arguments: AR.fit(maxlag=k, solver="mle")

But according to statsmodels 0.12.1 documentation, there is no "mle" option for solver keyword. The correct call seems to be AR.fit(maxlag=k, method="mle").

I've been having some numerical instabilities with this feature which results in huge (MAX_FLOAT) results.

Cristian

nils-braun commented 3 years ago

Thank you very much @cgw-github - yes, you are correct! Thanks for spotting this bug.

It seems the AR class is deprecated anyways with 0.12 and we should move away from it. Would you like to do a PR to fix this?

ghost commented 3 years ago

Would you like to do a PR to fix this?

Never done a PR in my life. Oops.

I have a few suggestions about this feature that I could contribute here, but I would not feel confident in writing the PR myself. If that is helpful, i can write a summary of my obs.

nils-braun commented 3 years ago

So I am very happy to help you do your first PR if you like! Do you have experience with git?

But if you just want to write up your findings here, that is also fine

nils-braun commented 3 years ago

So in case you would like to do a pr, @cgw-github:

click on "fork" on the tsfresh github page.
now clone your created fork repository, do your changes, commit them and push them to your repo.
If you now go to the tsfresh github page or to your forked one, you will see a banner which asks you if you would like to create a PR -> yes you do :-) Thats it!

hassan-jvd90 commented 3 years ago

@nils-braun I would like to take up this PR. I have some experience with git and time series modelling.

nils-braun commented 3 years ago

Sure! Thank you very much for that!

hassan-jvd90 commented 3 years ago

Hi, I have created a PR for this: https://github.com/blue-yonder/tsfresh/pull/805