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
Fix/linear quantile interpolation #899
Conversation
… produce sample paths. Quantile sampling changed, now uses linear interpolations between values
Codecov Report
@@ Coverage Diff @@
## master #899 +/- ##
==========================================
+ Coverage 91.67% 91.70% +0.03%
==========================================
Files 74 74
Lines 7375 7402 +27
==========================================
+ Hits 6761 6788 +27
Misses 614 614
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thanks 👍
I just have a couple of minor comments.
|
||
# linear interpolation | ||
weights = (probs - left_q) / (right_q - left_q) | ||
inter = left_value + weights * (right_value - left_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one, definitely an improvement 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks :)
model_output is of shape (n_series * n_samples, output_chunk_length, n_components, n_quantiles) | ||
""" | ||
num_samples, n_timesteps, n_components, n_quantiles = model_output.shape | ||
model_output = model_output.reshape( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this one necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, will remove
|
||
def _quantile_sampling(self, model_output: np.ndarray) -> np.ndarray: | ||
""" | ||
Select a quantile (for each batch example) and return this quantile (over time and components). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this docstring correct? Don't you rather return an interpolation between the quantiles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I forgot to update the docstrings
Fixes #869 .
Summary
Now quantile sampling samples uniformly between fitted quantiles. RegressionModels now correctly construct paths when generating samples.