Fix linear interpolation predict for unsorted input - #348
Open
RicardoSantos-99 wants to merge 1 commit into
Open
Fix linear interpolation predict for unsorted input#348RicardoSantos-99 wants to merge 1 commit into
RicardoSantos-99 wants to merge 1 commit into
Conversation
`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
force-pushed
the
fix-linear-interpolation-order
branch
from
August 6, 2026 16:43
cec3d20 to
6047a5c
Compare
josevalim
approved these changes
Aug 6, 2026
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.
Closes #346.
Scholar.Interpolation.Linear.predict/2sortstarget_xso 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:Why this reaches further than interpolation
Scholar.Linear.IsotonicRegression.predict/2calls into it, so isotonic predictions stopped being monotonic in x whenever the input wasn't sorted. Fitting withincreasing: trueand predicting on unsorted x returnedx=1.0 -> 4.5alongsidex=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.exsencoded the old behaviour, so they change here. Three things say the new ones are the correct values rather than merely different:[0..9])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 fromkey(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_xinLinear, including values outside the training bounds so the reorder is covered across all three loops, plus an order-independence test forIsotonicRegression.Worth noting the existing isotonic
predicttest already used unsorted input and still passed. Itsargsortis[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 inpredict_nthat had no effect. Happy to drop that from the diff if you would rather keep this to the fix.