Skip to content

Fix BasePipeline.forecast when prediction intervals are estimated on history data with presence of NaNs #1291

Merged
merged 5 commits into from
Jun 20, 2023

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Jun 16, 2023

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?

Proposed Changes

Look #899.

Closing issues

Closes #899.

@Mr-Geekman Mr-Geekman self-assigned this Jun 16, 2023
@Mr-Geekman Mr-Geekman marked this pull request as ready for review June 16, 2023 15:29
@github-actions
Copy link

github-actions bot commented Jun 16, 2023

@github-actions github-actions bot temporarily deployed to pull request June 16, 2023 17:30 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2023

Codecov Report

Merging #1291 (e26997c) into master (f4509bb) will increase coverage by 0.34%.
The diff coverage is 93.33%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #1291      +/-   ##
==========================================
+ Coverage   87.74%   88.08%   +0.34%     
==========================================
  Files         187      187              
  Lines       10859    10886      +27     
==========================================
+ Hits         9528     9589      +61     
+ Misses       1331     1297      -34     
Impacted Files Coverage Δ
etna/pipeline/base.py 95.66% <93.33%> (-0.25%) ⬇️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

etna/pipeline/base.py Outdated Show resolved Hide resolved
f"There are NaNs in target on time span from {min_timestamp} to {max_timestamp}. "
f"It can obstruct prediction interval estimation on history data."
)
if np.any(non_nan_counts < 2):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is it exactly 2?

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 is the minimal value that can give non-zero result.

etna/pipeline/base.py Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request June 19, 2023 14:47 Inactive
@Mr-Geekman Mr-Geekman merged commit 3b0027f into master Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Forecast with predictive intervals fails in case of data gaps
3 participants