Nixtla / neuralforecast

Scalable and user friendly neural :brain: forecasting algorithms.
https://nixtlaverse.nixtla.io/neuralforecast
Apache License 2.0
2.94k stars 337 forks source link

`mc` model config overwritten by `instantiate_*` makes it unusable. #183

Closed AzulGarza closed 2 years ago

AzulGarza commented 2 years ago

After getting the best hyperopt trial using nf.experiments.utils.hyperopt_tunning, I wanted to use the mc provided in the trial results in nf.experiments.utils.instantiate_nbeats. I got the following error:

image

The problem arises from these lines:

https://github.com/Nixtla/neuralforecast/blob/09ce6d0075bbf287d78483d04f107dec7505fc5c/neuralforecast/experiments/utils.py#L270-L273

Initially, n_blocks and n_layers are integers (denoting the number of blocks and layers). But later, nf.experiments.utils.instantiate_nbeats converts them to a list of integers. Therefore, the mc dictionary becomes unusable for reuse by nf.experiments.utils.instantiate_nbeats.

AzulGarza commented 2 years ago

Since n_layers and n_blocks are parameters of the model, I think the best solution is to rename the hyperopt space parameters to constant_n_layers and constant_n_blocks,

https://github.com/Nixtla/neuralforecast/blob/09ce6d0075bbf287d78483d04f107dec7505fc5c/neuralforecast/models/nbeats/nbeats.py#L827-L828

AzulGarza commented 2 years ago

The same applies for n_mlp_units.