docs: document both ConformalSeasonalPool interval thresholds and validate n_samples - #1225
Open
Jorge Polanco (Jorge-Polanco-Roque) wants to merge 2 commits into
Conversation
…idate n_samples Addresses the mechanical items from Nixtla#1202: - n_samples docstring now gives both the lower (ceil(2/a)-1) and upper (ceil(4/a)-1) thresholds and fixes the off-by-one 95% example. - predict_in_sample notes its width depends on R.size / calib_frac, not n_samples. - __init__ rejects n_samples < 1. Thanks to @shivamlalakiya for the analysis. Fixes Nixtla#1202
Author
|
Gentle ping — this has been open ~a week. It documents both ConformalSeasonalPool interval thresholds and validates |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
Fixes #1202 (items 1–3).
What does this implement/fix? Explain your changes.
Addresses the mechanical items from Shivam Lalakiya (@shivamlalakiya)'s detailed analysis in #1202:
n_samplesdocstring: it documented only the lower-bound threshold and its 95% example was off by one. It now gives both thresholds —ceil(2/a) - 1(lower bound) andceil(4/a) - 1(upper bound), witha = 1 - L/100— and the worked example reads ≥39 / ≥79.predict_in_sample: added a note that its interval width depends on the calibration pool sizeR.size(set bycalib_fracand the history length), not onn_samples.__init__: now rejectsn_samples < 1(0 produced a degenerate, zero-width interval);n_samples=1stays allowed.Item 4 (a runtime
warnings.warnwhen the pool is below the threshold for a requested level) is a behaviour addition and is left to the maintainers' discretion, as the reporter noted.Does your contribution introduce a new dependency? If yes, which one?
No.
Testing
Added
test_invalid_n_samples_raises; verified it fails without the guard.ruffand thetest_csp.pysuite (23 tests) are green.Thanks to Shivam Lalakiya (@shivamlalakiya) for the thorough analysis.