Repository navigation
Fix WIS: use w_median * |obs - median| on the array backends - #141
Open
lucaluo925 wants to merge 1 commit into
Open
lucaluo925 wants to merge 1 commit into
lucaluo925 wants to merge 1 commit into
Conversation
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.
Summary
Fixes #140.
In
scoringrules/core/interval/_score.py, changedWIS += w_median * mediantoWIS += w_median * B.abs(obs - median). Per Bracher et al. (2021) Eq. (3), this term is the weight multiplied by the absolute deviation between observation and median, not the median itself. The numba gufunc implementation was already correct; this change aligns the shared array backend path with it. Since_score.pyis shared across numpy, jax, and torch, this fixes all three backends at once.Also updated documentation: the docstring in
_interval.pystated the default weight isw_k = 2/alpha_k, while the actual code usesalpha/2. The code was correct (consistent with the paper); only the docstring is updated.Tests
The original
test_weighted_interval_scorepassed the sameobsvalue as both observation and median, so|obs - median| = 0. Both formulas yielded identical results, and the CI for all four backends remained green, hiding the bug. The test is updated toobs = 0.4, median = 0so this term contributes meaningfully, with a comment explaining why observation and median must differ.Added
test_weighted_interval_score_median_term, which asserts directly against the closed-form solution from Eq. (3) in the paper, without relying on the approximation that "WIS approximates CRPS".Validation
After reverting the source change, the two tests fail under the numpy backend (0.458 vs expected 0.338), while the numba backend still passes — confirming the bug only affects non-numba paths. After restoring the fix, all 6 tests in
tests/test_interval.pypass. Environment: Python 3.14, numpy 2.5.3, numba 0.67.0.jax and torch are not installed in my environment, so I could only exercise the numpy and numba backends locally; CI will be the first run covering all four.
AI note
The code changes and tests were written with assistance from an AI assistant. I reproduced the bug locally, ran the tests, and verified that the tests fail when the fix is reverted.
Checklist
uv sync --all-extras --devthenuv run pytest tests/(exercises every installed backend: numpy, numba, jax, torch)breaking,enhancement,bug,backend,documentation, orci)(I can't apply labels myself —
buglooks like the right one here.)