Skip to content

Fix bugs in Scholar.Linear.IsotonicRegression - #368

Merged
josevalim merged 7 commits into
elixir-nx:mainfrom
RicardoSantos-99:fix-isotonic
Sep 1, 2026
Merged

Fix bugs in Scholar.Linear.IsotonicRegression#368
josevalim merged 7 commits into
elixir-nx:mainfrom
RicardoSantos-99:fix-isotonic

Conversation

@RicardoSantos-99

Copy link
Copy Markdown
Contributor
  • A decreasing fit can come back non-monotonic. contiguous_isotonic_regression/4
    writes 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_size instead. Part of the
    last 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.
  • A zero weight poisons the whole fit. A sample with weight zero still opens
    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] returned
    thresholds of -infinity and predicted NaN everywhere. scikit-learn masks
    non-positive weights out before fitting.
  • out_of_bounds was never read. It appears once in the whole module, in the
    schema. predict/2 always clipped, so :nan, the documented default, behaved
    exactly like :clip and the default behaviour did not exist.
  • preprocess/1 raises on models with fewer than three thresholds. The
    duplicate trim slices y[1..-2//1], which needs three. Constant targets or a
    two point fit produce two, and the range 1..0 raises.
  • cutoff_index goes stale. The trim drops thresholds but the index keeps
    pointing into the untrimmed tensors, so running preprocess/1 on its own
    output slices past the end and raises.
  • fit/3 rejects the column shape its own check allows. check_input_shape/1
    documents {n, 1} as valid and predict/2 accepts it, but fit/3
    destructures the shape as {n_samples} and raises a MatchError first.
  • increasing: :auto follows 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..10 with y = 1..9 then -100,
    the slope is -5 and picks decreasing while rho stays at 0.45 and picks
    increasing.

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.
@josevalim

Copy link
Copy Markdown
Contributor

💚 💙 💜 💛 ❤️

@josevalim
josevalim merged commit 9eda322 into elixir-nx:main Sep 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants