Skip to content

Fix linear interpolation predict for unsorted input - #348

Open
RicardoSantos-99 wants to merge 1 commit into
elixir-nx:mainfrom
RicardoSantos-99:fix-linear-interpolation-order
Open

Fix linear interpolation predict for unsorted input#348
RicardoSantos-99 wants to merge 1 commit into
elixir-nx:mainfrom
RicardoSantos-99:fix-linear-interpolation-order

Conversation

@RicardoSantos-99

Copy link
Copy Markdown
Contributor

Closes #346.

Scholar.Interpolation.Linear.predict/2 sorts target_x so its sequential scan can walk the bins in order, then maps the results back at the end. It applied the sort permutation where it needed the inverse, so unsorted input came back shuffled:

model = Linear.fit(Nx.tensor([1.0, 2.0, 3.0]), Nx.tensor([10.0, 20.0, 30.0]))
Linear.predict(model, Nx.tensor([3.0, 1.0, 2.0]))
#=> [20.0, 30.0, 10.0]   expected [30.0, 10.0, 20.0]

Why this reaches further than interpolation

Scholar.Linear.IsotonicRegression.predict/2 calls into it, so isotonic predictions stopped being monotonic in x whenever the input wasn't sorted. Fitting with increasing: true and predicting on unsorted x returned x=1.0 -> 4.5 alongside x=5.0 -> 2.5.

Non-metric MDS then calls isotonic with unsorted dissimilarities, so it has been optimizing against shuffled disparities.

On the MDS test values

The reference values in mds_test.exs encoded the old behaviour, so they change here. Three things say the new ones are the correct values rather than merely different:

normalized stress ordering recovered (Kendall tau vs [0..9])
scikit-learn 1.6.1 (best of 20 seeds) 0.090 +1.000
this PR 0.136 +0.867
before 0.249 +0.467

Stress is what MDS minimizes, and it drops in both affected tests (1.288 to 0.197, and 0.249 to 0.136). The fixture input is Nx.iota({10, 50}), whose rows are collinear, so a correct 2-D embedding should lay the points out in order along one axis. The remaining gap to scikit-learn is that it takes the best of 20 random inits on a non-convex problem while Scholar runs a single init from key(42).

Isotonic itself is deterministic, and it now agrees with scikit-learn to within f32 precision on unsorted input.

Tests

Regression tests for unsorted target_x in Linear, including values outside the training bounds so the reorder is covered across all three loops, plus an order-independence test for IsotonicRegression.

Worth noting the existing isotonic predict test already used unsorted input and still passed. Its argsort is [1, 0, 2], a swap of two entries, which is its own inverse and so cannot distinguish a permutation from its inverse. The new tests use a 3-cycle.

Also removed a stray {res, i} expression in predict_n that had no effect. Happy to drop that from the diff if you would rather keep this to the fix.

`predict/2` sorts `target_x` to run its sequential scan, then has to map the
results back to the order they came in. It applied the sort permutation instead
of its inverse, so any unsorted input came back shuffled.

This surfaces beyond the interpolation module. `IsotonicRegression.predict/2`
calls it, so isotonic predictions stopped being monotonic in x for unsorted
input, and non-metric MDS calls isotonic with unsorted dissimilarities.

The MDS reference values in the tests encoded the old behaviour. The new ones
score better on the metric MDS minimizes (stress 1.288 -> 0.197 and
0.249 -> 0.136) and recover the 1-D ordering of the collinear input, matching
what scikit-learn produces on the same data. Isotonic predictions now agree
with scikit-learn to within f32 precision.

The existing isotonic `predict` test already used unsorted input, but its
permutation was a swap of two entries, which is its own inverse and so cannot
tell the two apart. The new tests use a 3-cycle.

Closes elixir-nx#346
@RicardoSantos-99
RicardoSantos-99 force-pushed the fix-linear-interpolation-order branch from cec3d20 to 6047a5c Compare August 6, 2026 16:43
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.

Linear interpolator predict doesn't handle unsorted input

2 participants