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

Train splitting support in historical_forecasts #2064

Open
adahan opened this issue Nov 10, 2023 · 2 comments
Open

Train splitting support in historical_forecasts #2064

adahan opened this issue Nov 10, 2023 · 2 comments
Labels
feature request Use this label to request a new feature

Comments

@adahan
Copy link

adahan commented Nov 10, 2023

Is your feature request related to a current problem? Please describe.
historical_forecasts doesn't not support Early stopping on val loss because historical_forecasts doesn't support train timeseries splitting.

Describe proposed solution
It would be great to add a split_before param to historical_forecasts so when retrain is not False we can split the training data in train/validation set before each retrain. Therefore enabling Early stopping on val loss for historical forecast which would speedup the process. This would also requires to add a load_from_checkpoint option to reload the model at best validation error.

Describe potential alternatives
Instead of adding parameters to historical_forecasts, we add both split_before and load_from_checkpoint to the properties of the model itself same as the callbacks property for instance and historical_forecasts could simply load split_before and load_from_checkpoint and use it both fit and historical_forecasts and would automatically split and/or load from checkpoint without external call.

Additional context
Passing early_stop on val_loss to callbacks will throw: Early stopping conditioned on metric val_loss which is not available. Pass in or modify your EarlyStopping callback to use any of the following: train_loss when using historical_forecasts.

@adahan adahan added the triage Issue waiting for triaging label Nov 10, 2023
@madtoinou madtoinou added feature request Use this label to request a new feature and removed triage Issue waiting for triaging labels Nov 10, 2023
@madtoinou
Copy link
Collaborator

Hi @adahan,

Thanks you for this feature request, I think it's a partial duplicate of #2004.

I really like the approach you are describing and would like to know your opinion on the following aspects:

  • during the first iterations, when the size of the series before the start (first forecasted timestamp) is rather short, the EarlyStopping might be triggered with only a few samples. How would you deal with that?
  • If a minimum validation length is provided, would simplify shifting the first start date be acceptable? Or the timestamps between the original start and the "shifted" (from which the validation length constraint is fulfilled) start should be forecasted using the current model (fitted before calling historical_forecasts). Or another behavior?
  • would you expect the checkpoints generated by historical_forecasts to be accessible after calling the method or automatically removed from the disk?

Once merged with the master branch, #2050 will allow user to provide a "static" validation series to the model.fit() method and partially cover the feature you are requesting (external validation, not the series parameter of historical_forecasts()).

@adahan
Copy link
Author

adahan commented Nov 13, 2023

You are right, its close to #2004. Thanks for your feedbacks and here are my opinions :

  1. If the size of the series is too short at the beginning I'd say its the responsibility of the user to decide or not to use the early stopping, change the start time or add more date if possible.. We can probably add a warning if the training set is "too short" but then we have to decide what is too short.
  2. Your suggestion to use a minimum val length and calculate a start time makes sense and would work. I'm worried it can be confusing for the user to understand what's going on. I'd keep it simple both for implementation and for user understanding. Let the user responsible for managing the val len. There is a train_length available already to specify a fixed train length. Adding a split_before(x) would therefore give the user a val len of 1-x of train lenght.
  3. For the checkpoint I would expect to keep the current behavior which I think is to not store them. Eventually we could offer to store only the one loaded by the load_from_checkpoint but I dont think its necessary. And it would probably require even more development. If you want to provide this capability maybe we could implement a LambdaCallback similar to what lighning pytorch is doing providing for exemple on_historical_iteration_start, on_historical_iteration_end. on_historical_iteration_start could be used by the user to define specific validation dataset and on_historical_iteration_end used for special handling on checkpoints for exemple.

#2050 in combination with historical iterations callbacks might provide the best way to offer a flexible way to handle this as it could update those static val timeseries at each retrain and serve so many other use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Use this label to request a new feature
Projects
None yet
Development

No branches or pull requests

2 participants