Fix bugs in Scholar.Linear.IsotonicRegression - #368
Merged
Conversation
contiguous_isotonic_regression/4 pools blocks of points and then writes each pooled value back over the block it covers. That second pass stops at max_size, the index of the last distinct x, which is only where the valid range ends when fitting increasing. Fitting decreasing reverses the input, so the placeholder slots left by tied x values move to the front and the valid range ends at y_size instead. The pass then stops short and leaves part of the last block holding its unpooled value. fit([0, 0, 1, 1, 2], [4, 4, 5, 5, 3], increasing: false) returned thresholds [4.0, 4.5, 3.0], which rises where the caller asked for a decreasing fit. scikit-learn returns [4.5, 4.5, 3.0].
make_unique/3 groups samples by x and divides each group's weighted sum of y by its total weight. A sample whose weight is zero still opens or joins a group, so a group made only of such samples divides by zero and its threshold becomes NaN, which then spreads through the pooling and leaves every prediction NaN. fit([1, 2, 3, 4, 5], [1, 5, 3, 4, 5], sample_weights: [1, 0, 1, 1, 1]) returned thresholds of -infinity and predicted NaN everywhere. scikit-learn masks non-positive weights out before fitting, and now so does this, which makes the result identical to fitting the remaining rows by hand.
The option is validated and then never read: it appears once in the whole module, in the schema. predict/2 always clips into the fitted range, so :nan, the documented default, behaved exactly like :clip and the default behaviour did not exist. Keep the option on the struct and apply it, so a point outside the fitted range predicts NaN under :nan and the boundary value under :clip, matching scikit-learn.
The duplicate trim compares each threshold against its neighbours by slicing y[1..-2//1], which needs at least three of them. A model with two, which is what constant targets or a two point fit produce, asks for the range 1..0 and raises. There is nothing to trim below three thresholds, since the trim only ever drops interior points, so skip it.
preprocess/1 trims duplicate thresholds but leaves cutoff_index pointing at the last index of the untrimmed tensors, and the field documents itself as the index of the last valid threshold. Running preprocess/1 on its own output then slices past the end and raises.
With increasing: :auto the direction came from the sign of an ordinary least squares slope, which a single outlier can flip. scikit-learn uses the sign of Spearman's rho, which reads the ranks and does not move with the size of an outlier. On x = 1..10 with y = 1..9 followed by -100, the slope is -5 and picks decreasing while rho stays at 0.45 and picks increasing. A constant input leaves the correlation undefined, and the comparison then picks decreasing, which is what scikit-learn also does.
check_input_shape/1 documents a {n, 1} tensor as valid, and predict/2 accepts
one, but fit/3 destructures the shape as {n_samples} and raises a MatchError
before reaching that check.
RicardoSantos-99
force-pushed
the
fix-isotonic
branch
from
September 1, 2026 14:49
bf48e1d to
5a920c5
Compare
Contributor
|
💚 💙 💜 💛 ❤️ |
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.
contiguous_isotonic_regression/4writes each pooled value back over the block it covers, but that pass stops at
max_size, which is only where the valid range ends when fitting increasing.Fitting decreasing reverses the input, so the placeholder slots left by tied x
values move to the front and the range ends at
y_sizeinstead. Part of thelast block keeps its unpooled value:
fit([0, 0, 1, 1, 2], [4, 4, 5, 5, 3], increasing: false)returned thresholds[4.0, 4.5, 3.0], which rises where the caller asked for a decreasing fit.or joins a group, so a group made only of those divides by zero. The NaN then
spreads through the pooling:
sample_weights: [1, 0, 1, 1, 1]returnedthresholds of
-infinityand predicted NaN everywhere. scikit-learn masksnon-positive weights out before fitting.
out_of_boundswas never read. It appears once in the whole module, in theschema.
predict/2always clipped, so:nan, the documented default, behavedexactly like
:clipand the default behaviour did not exist.preprocess/1raises on models with fewer than three thresholds. Theduplicate trim slices
y[1..-2//1], which needs three. Constant targets or atwo point fit produce two, and the range
1..0raises.cutoff_indexgoes stale. The trim drops thresholds but the index keepspointing into the untrimmed tensors, so running
preprocess/1on its ownoutput slices past the end and raises.
fit/3rejects the column shape its own check allows.check_input_shape/1documents
{n, 1}as valid andpredict/2accepts it, butfit/3destructures the shape as
{n_samples}and raises a MatchError first.increasing: :autofollows a least squares slope. One outlier flips it.scikit-learn uses the sign of Spearman's rho, which reads ranks and does not
move with the size of an outlier. On
x = 1..10withy = 1..9then-100,the slope is -5 and picks decreasing while rho stays at 0.45 and picks
increasing.