FIX SAGA solver ignoring sample_weight when drawing samples (gh-21305) - #34799
Open
Boubker10 wants to merge 1 commit into
Open
FIX SAGA solver ignoring sample_weight when drawing samples (gh-21305)#34799Boubker10 wants to merge 1 commit into
Boubker10 wants to merge 1 commit into
Conversation
|
Thank you for opening your first pull request to scikit-learn! 🎉 To help get your contribution reviewed, please make sure that:
|
Boubker10
force-pushed
the
fix-saga-sample-weight-sampling
branch
from
August 25, 2026 19:42
cb52bfa to
2919ccf
Compare
…earngh-21305) SAG/SAGA draw one sample per inner iteration via SequentialDataset._get_random_index(), which is uniform over the rows and never accounts for sample_weight -- sample_weight only scaled the loss of whichever sample was drawn, not the probability of drawing it. When weights are far from uniform this makes the stochastic gradient estimate very high-variance and the solver converges poorly or not at all. Since sample_weight is fixed for the duration of a solver run, build a Walker/Vose alias table once (O(n_samples)) so each iteration can draw a sample proportional to its weight in O(1), and apply the matching inverse-probability correction to SAGA's direct gradient term to keep the estimator unbiased (the sum_gradient/intercept_sum_gradient accumulators are deterministic bookkeeping and must keep accumulating the raw, unscaled delta). Restricted to SAGA (is_saga=True): plain SAG has no per-draw bias correction and relies on every sample being revisited regularly, so skewing the draw towards heavy samples can slow it down instead of helping -- a separate, pre-existing limitation this does not address. Highly skewed weights combined with L1's proximal operator can also still need more than the default max_iter to satisfy the strict tol criterion, though the reached solution is markedly better than before at any given iteration budget. No-op for uniform sample_weight (including the default, None): the original code path is taken unchanged, verified bit-identical for a fixed random_state.
Boubker10
force-pushed
the
fix-saga-sample-weight-sampling
branch
from
August 25, 2026 19:51
2919ccf to
3efa3fb
Compare
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 #21305
What does this implement/fix? Explain your changes.
SAG/SAGA draw one sample per inner iteration via
SequentialDataset._get_random_index(), which is uniform over the rows andnever accounts for
sample_weight--sample_weightonly scaled the loss ofwhichever sample was drawn, not the probability of drawing it. When weights
are far from uniform this makes the stochastic gradient estimate very
high-variance and the solver converges poorly or not at all.
Since
sample_weightis fixed for the duration of a solver run, this PRbuilds a Walker/Vose alias table once (
O(n_samples)) so each iteration candraw a sample proportional to its weight in
O(1), and applies the matchinginverse-probability correction to SAGA's direct gradient term to keep the
estimator unbiased (the
sum_gradient/`intercept_are deterministic bookkeeping and must keep accumulating the raw, unscaled
delta).
Restricted to SAGA (
is_saga=True): plain SAG hascorrection and relies on every sample being revisited regularly, so skewing
the draw towards heavy samples can slow it down in
separate, pre-existing limitation this does not address. Highly skewed
weights combined with L1's proximal operator can a
the default
max_iterto satisfy the stricttolcriterion, though thereached solution is markedly better than before at
budget.
No-op for uniform
sample_weight(including the default,None): theoriginal code path is taken unchanged, verified bi
random_state.