num_sample variation fixes #421
Conversation
|
Looks like this broke the tests for |
There was a problem hiding this comment.
One thing that is missing is making every predictor’s predict method accept the num_samples argument (in case they produce samples). For example, NPTS does not yet comply with what is proposed in #301: https://github.com/awslabs/gluon-ts/blob/df20e54a3a7671b3592ca1e7621ee2e5344f9fbd/src/gluonts/model/npts/_predictor.py#L172
Edit:
Neither does Prophet https://github.com/awslabs/gluon-ts/blob/df20e54a3a7671b3592ca1e7621ee2e5344f9fbd/src/gluonts/model/prophet/_predictor.py#L129
The R predictor has both things https://github.com/awslabs/gluon-ts/blob/df20e54a3a7671b3592ca1e7621ee2e5344f9fbd/src/gluonts/model/r_forecast/_predictor.py#L80
benidis
force-pushed
the
num_samples_naming
branch
from
0d39896
to
f64faca
Compare
src/gluonts/model/npts/_predictor.py
Outdated
| @@ -169,11 +168,11 @@ def __init__( | |||
| use_default_time_features: bool = True, | |||
| num_default_time_features: int = 1, | |||
| feature_scale: float = 1000.0, | |||
| num_parallel_samples: int = 100, | |||
| num_samples: int = 100, | |||
There was a problem hiding this comment.
My proposal was to remove this from the constructor of predictors, and keep it only as an argument to the predict method, see #301
Incorporate change in awslabs#421 to this example.
Incorporate change in #421 to this example.
* fix num_sample variations
Incorporate change in awslabs#421 to this example.
Fixes issue #301 (following #232).