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

Feat/document model saving loading #1210

Merged
merged 21 commits into from
Sep 28, 2022

Conversation

amadejkocbek
Copy link
Contributor

@amadejkocbek amadejkocbek commented Sep 13, 2022

Fixes #.

Summary

Add Reproducibility section in the Forecasting Overview part of the User Guide, describing model saving and loading.

Other Information

Still WIP, as more detail and care is needed for torch models.

@amadejkocbek amadejkocbek marked this pull request as draft September 13, 2022 21:43
@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2022

Codecov Report

Base: 94.02% // Head: 94.01% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (376d43d) compared to base (2d0a035).
Patch has no changes to coverable lines.

❗ Current head 376d43d differs from pull request most recent head 61c9097. Consider uploading reports for the commit 61c9097 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1210      +/-   ##
==========================================
- Coverage   94.02%   94.01%   -0.01%     
==========================================
  Files          73       73              
  Lines        8215     8203      -12     
==========================================
- Hits         7724     7712      -12     
  Misses        491      491              
Impacted Files Coverage Δ
darts/timeseries.py 92.34% <0.00%> (-0.06%) ⬇️
...arts/models/forecasting/torch_forecasting_model.py 87.42% <0.00%> (-0.05%) ⬇️
darts/models/forecasting/block_rnn_model.py 98.24% <0.00%> (-0.04%) ⬇️
darts/models/forecasting/nhits.py 99.27% <0.00%> (-0.01%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@amadejkocbek amadejkocbek marked this pull request as ready for review September 18, 2022 20:46
@@ -40,6 +40,54 @@ Furthermore, we define the following types of time series consumed by the models
* **Target series:** the series that we are interested in forecasting.
* **Covariate series:** some other series that we are not interested in forecasting, but that can provide valuable inputs to the forecasting model.

## Reproducibility

If the user wishes to save a particular model and use it elsewhere or at a later point in time, darts leverages pickle to achieve that.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpicking] You shouldn't talk about the user in third person. Either don't address the reader, or say "you".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted and fixed.

@@ -40,6 +40,54 @@ Furthermore, we define the following types of time series consumed by the models
* **Target series:** the series that we are interested in forecasting.
* **Covariate series:** some other series that we are not interested in forecasting, but that can provide valuable inputs to the forecasting model.

## Reproducibility

If the user wishes to save a particular model and use it elsewhere or at a later point in time, darts leverages pickle to achieve that.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Darts does not only leverage pickle. Torch models rely on saving PyTorch Lightning trainer checkpoints.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lightning trainer checkpoints are now mentioned in the description.

docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved
model.save("my_model.pkl")
model_loaded = RegressionModel.load("my_model.pkl")

from darts.models import NaiveSeasonal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this second example? I'd say maybe the one above is enough. Also the one below seems to be saving to a .py file?

docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved
docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved

Special class methods for torch models:

* **save_checkpoint** In addition, we need to use PTL save_checkpoint() to properly save the trainer and model. It is used to be able to save a snapshot of the model mid-training, and then be able to retrieve the model later.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we offer this as a method?

docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just made a couple of suggestions.

docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved
docs/userguide/forecasting_overview.md Outdated Show resolved Hide resolved
model = NBEATSModel(input_chunk_length=24,
output_chunk_length=12)

model.fit([series1, series2])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we don't even need this line. We can save a model that's not been fitted yet too.

amadejkocbek and others added 4 commits September 28, 2022 11:51
Update title

Co-authored-by: Julien Herzen <j.herzen@gmail.com>
Update saved model file name

Co-authored-by: Julien Herzen <j.herzen@gmail.com>
Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@hrzn hrzn merged commit d97932a into unit8co:master Sep 28, 2022
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.

None yet

3 participants