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

Streamline "freq", "time_granularity" naming #134

Closed
mbohlkeschneider opened this issue Jun 20, 2019 · 7 comments
Closed

Streamline "freq", "time_granularity" naming #134

mbohlkeschneider opened this issue Jun 20, 2019 · 7 comments

Comments

@mbohlkeschneider
Copy link
Contributor

Find a naming that is consistent and change it throughout the code for freq/time_granularity.

@geoalgo
Copy link
Contributor

geoalgo commented Jun 21, 2019

Given that Pandas use 'freq' (link), this is probably the best option.

@mbohlkeschneider
Copy link
Contributor Author

I agree with that. We should make this distinction clear somewhere in the documentation, though, that this is not confused with seasonality frequency.

@jaheba
Copy link
Contributor

jaheba commented Jun 28, 2019

I'm currently looking into the shell (essentially integrating with SageMaker) and noticed this too.

In SageMaker DeepAR we use time_freq and I think there is an argument to be made that we should aim for consistency.

@mbohlkeschneider
Copy link
Contributor Author

Consistency is a good argument. But consistency with SageMaker might not necessarily be the right thing in a library. Pandas uses freq for generating time series Series objects and I think it would be good to stick to that.

@jaheba
Copy link
Contributor

jaheba commented Jun 28, 2019

I'm fine with either, but at least we should be consistent within GluonTS.

Do we have a set of required hyper-parameters? Besides freq prediction_length comes to mind.

@geoalgo
Copy link
Contributor

geoalgo commented Jun 28, 2019

Yes, freq and prediction_length are the only two required hyper-parameters.

Pandas is much more frequent than Sagemaker, so I also freq will be nicer.

@lostella
Copy link
Contributor

lostella commented Jul 7, 2019

addressed in #172 and #173

@lostella lostella closed this as completed Jul 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants