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

remove pandas-related FutureWarning #88

Merged
merged 2 commits into from
Jun 7, 2019

Conversation

lostella
Copy link
Contributor

@lostella lostella commented Jun 7, 2019

Issue #, if available: #87

Description of changes: addressed pandas complaints.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@szha
Copy link
Member

szha commented Jun 7, 2019

Job PR-88/1 is complete.
Docs are uploaded to http://gluon-ts-staging.s3-accelerate.dualstack.amazonaws.com/PR-88/1/index.html

@lostella lostella requested a review from jgasthaus June 7, 2019 13:00
@szha
Copy link
Member

szha commented Jun 7, 2019

Job PR-88/2 is complete.
Docs are uploaded to http://gluon-ts-staging.s3-accelerate.dualstack.amazonaws.com/PR-88/2/index.html

@alexw91
Copy link
Member

alexw91 commented Jun 7, 2019

Codecov Report

Merging #88 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
+ Coverage   78.63%   78.64%   +<.01%     
==========================================
  Files         110      110              
  Lines        6282     6283       +1     
==========================================
+ Hits         4940     4941       +1     
  Misses       1342     1342
Impacted Files Coverage Δ
src/gluonts/dataset/artificial/_base.py 32.16% <ø> (ø) ⬆️
src/gluonts/evaluation/_base.py 99.38% <100%> (ø) ⬆️
src/gluonts/model/seasonal_naive/_predictor.py 96.66% <100%> (ø) ⬆️
src/gluonts/evaluation/backtest.py 87.5% <100%> (ø) ⬆️

@lostella lostella merged commit 3c141d4 into awslabs:master Jun 7, 2019
@lostella lostella deleted the fix-pd-future-warning branch June 7, 2019 14:39
@rherbrich74
Copy link

Is this fix already installable via pip install --upgrade gluonts? I am running python 3.7.3 and I am still getting this issue when I am trying to train a feedforward model.

@lostella
Copy link
Contributor Author

@rherbrich74 not yet, we will make a new release tomorrow probably. In the meantime you can

pip install git+https://github.com/awslabs/gluon-ts.git  

@rherbrich74
Copy link

@lostella: That worked - thanks! I have one more question on the semantics of make_evaluation_predictions and the parameter num_eval_samples: If I follow the tutorial, the first element of the forecasts array contains 100 sample paths and they are all around the target values:

forecasts[0].samples
array([[ 678.3231 , 576.42914, 561.24054, ..., 800.3523 , 901.3257 ,
916.05774],
[ 690.817 , 593.2066 , 1075.1001 , ..., 852.2531 , 808.0277 ,
665.4199 ],
[ 684.40814, 601.9213 , 523.0439 , ..., 847.2773 , 773.35095,
712.46436],
...,
[ 676.9368 , 563.7203 , 405.0648 , ..., 679.5283 , 732.5705 ,
680.9393 ],
[ 655.64545, 642.0739 , 524.64435, ..., 819.95447, 777.0701 ,
768.63086],
[ 741.4884 , 668.1726 , 518.23926, ..., 840.6607 , 858.57776,
774.19196]], dtype=float32)

But the second element of this array is a factor of five too high:

forecasts[1].samples
array([[3119.2852, 2552.321 , 2850.9817, ..., 3502.2512, 3602.2888,
2993.7017],
[2985.6204, 2787.8015, 2402.0273, ..., 3434.465 , 2011.0767,
3282.1006],
[2831.7378, 2684.3398, 2630.1077, ..., 1831.2285, 3129.4214,
2915.8489],
...,
[2978.8643, 2787.955 , 2404.7107, ..., 3094.3599, 3225.1023,
3651.5989],
[2733.5464, 2616.9783, 2448.9888, ..., 3635.505 , 3800.9192,
3365.458 ],
[3255.8765, 2840.524 , 2388.526 , ..., 3959.009 , 3052.5603,
2729.477 ]], dtype=float32)

Also, I do not understand the logic of this parameter. The number of sample paths for the predictor are initialized in the constructor

estimator = SimpleFeedForwardEstimator(
num_hidden_dimensions=[10],
prediction_length=dataset.metadata.prediction_length,
context_length=100,
freq=dataset.metadata.time_granularity,
num_eval_samples=1000,
trainer=Trainer(ctx="cpu", epochs=5, learning_rate=1E-3, hybridize=True, num_batches_per_epoch=200,),
)

Can you help clarify?

@lostella
Copy link
Contributor Author

lostella commented Jun 13, 2019

@rherbrich74 sure:

  1. The forecasts[1] I get looks OK to me. Are you comparing it to the corresponding time series? In the notebook, you should change
tss[0][-plot_length:].plot(ax=ax)  # plot the time series
forecasts[0].plot(prediction_intervals=prediction_intervals, color='g')

into

tss[1][-plot_length:].plot(ax=ax)  # plot the time series
forecasts[1].plot(prediction_intervals=prediction_intervals, color='g')

to do the right comparison.

  1. I believe the num_eval_samples argument of make_evaluation_predictions is unused by some models (e.g. SimpleFeedForward, as you noticed) which have it hard-coded since their creation. But maybe other types of predictors (like the R-based one) allow customizing this at inference time. This is definitely a potentially confusing point, we'll think about how to streamline the API wrt this. Thanks for the feedback!

@rherbrich74
Copy link

Thanks for the quick response, @lostella ! I still don't get the right output; this is what I see

image

I made the code publicly available at https://github.com/rherbrich74/gluon-ts-experiments/; maybe there is something obvious I am doing wrong.

@vafl
Copy link
Contributor

vafl commented Jun 13, 2019

That does look funny. Thanks for the sample.

FYI num_eval_samples should be fixed by #103

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

Successfully merging this pull request may close these issues.

6 participants