Skip to content

Add AutoRegressivePipeline #209

Merged
merged 13 commits into from
Oct 21, 2021
Merged

Add AutoRegressivePipeline #209

merged 13 commits into from
Oct 21, 2021

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Oct 18, 2021

IMPORTANT: Please do not create a Pull Request without creating an issue first.

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Type of Change

  • Examples / docs / tutorials / contributors update
  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves an existing feature)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Proposed Changes

Add AutoRegressivePipeline.

Related Issue

#173.

Closing issues

Closes #173.

@Mr-Geekman Mr-Geekman added the enhancement New feature or request label Oct 18, 2021
@Mr-Geekman Mr-Geekman self-assigned this Oct 18, 2021
@Mr-Geekman Mr-Geekman marked this pull request as ready for review October 18, 2021 14:34
message="You probably set wrong freq.",
action="ignore",
)
cur_ts_forecast = cur_ts.make_future(cur_step)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we use self.ts to generate future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It don't have all the values to make features for the future. And if we change it, it can make it difficult to make another forecast.

Copy link
Contributor

@martins0n martins0n left a comment

Choose a reason for hiding this comment

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

Could you check behaviour of segmentencoder and inplace={False, True} Scalers too.

to_forecast = self.horizon
while to_forecast > 0:
cur_step = min(self.step, to_forecast)
cur_ts = TSDataset(prediction_df, freq=self.ts.freq)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use full name current_step and current_ts

"""
prediction_df = self.ts.to_pandas()
to_forecast = self.horizon
while to_forecast > 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make it with for loop and all logic in one place:

for idx_start in range(0, total_lenght, batch_size):
    len_slice = min(batch_size, total_lenght - idx_start)


Returns
-------
TSDataset
Copy link
Contributor

Choose a reason for hiding this comment

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

Something strange.
There is above

        Returns
        -------
        Pipeline:
            Fitted Pipeline instance

)
cur_ts_forecast = cur_ts.make_future(cur_step)
cur_ts_future = self.model.forecast(cur_ts_forecast)
prediction_df = pd.concat([prediction_df, cur_ts_future.to_pandas()[prediction_df.columns]])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just fill nans without concat every time?

@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2021

Codecov Report

Merging #209 (0f9255a) into master (e288524) will increase coverage by 0.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
+ Coverage   88.68%   88.98%   +0.30%     
==========================================
  Files          77       78       +1     
  Lines        3631     3676      +45     
==========================================
+ Hits         3220     3271      +51     
+ Misses        411      405       -6     
Impacted Files Coverage Δ
etna/pipeline/__init__.py 100.00% <100.00%> (ø)
etna/pipeline/autoregressive_pipeline.py 100.00% <100.00%> (ø)
etna/datasets/tsdataset.py 91.24% <0.00%> (+2.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e288524...0f9255a. Read the comment docs.

martins0n
martins0n previously approved these changes Oct 21, 2021
Copy link
Contributor

@martins0n martins0n left a comment

Choose a reason for hiding this comment

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

👍

martins0n
martins0n previously approved these changes Oct 21, 2021
@martins0n martins0n merged commit 91bcbea into master Oct 21, 2021
@martins0n martins0n deleted the issue-173 branch October 21, 2021 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add AutoRegressivePipeline to etna
4 participants