Skip to content

Add Nearest Neighbor interpolator#287

Closed
kshitij-maths wants to merge 2 commits intomathLab:masterfrom
kshitij-maths:feature/nearest-interpolator
Closed

Add Nearest Neighbor interpolator#287
kshitij-maths wants to merge 2 commits intomathLab:masterfrom
kshitij-maths:feature/nearest-interpolator

Conversation

@kshitij-maths
Copy link
Copy Markdown
Member

This PR implements the Nearest Neighbor interpolator requested in #166, supporting both univariate and multivariate parameter spaces.

Key Features:

  • Uses scipy.interpolate.interp1d(kind='nearest') for 1D spaces (with fill_value='extrapolate' for robust boundary handling).
  • Uses scipy.interpolate.NearestNDInterpolator for ND spaces.
  • Exposes the rescale=True argument by default for the ND interpolator to prevent severe distance scaling distortions across physical parameters of varying magnitudes.
    Testing:
  • Added comprehensive geometric and rank-consistency unit tests in tests/test_nearest.py (8 passing tests).
  • Added parallel framework compatibility tests in tests/test_parallel/test_nearest.py.

More interpolators such as splines etc will be added in subsequent additions.

@ndem0
Copy link
Copy Markdown
Member

ndem0 commented Apr 28, 2026

Dear @kshitij-maths, I'm quite lost in understanding the new approximator.
We already have RadiusNeighbors and KNeighbors (nearest neighbors that also have a parameter for radius or for the number of neighbors).
Your implementation is actually the same as KNeighbors (with k=1). Why do we need this addition?

@kshitij-maths
Copy link
Copy Markdown
Member Author

This implementation was added specifically to resolve Issue #166, which explicitly requests wrappers for the standard SciPy interpolators.

While the theoretical background is identical to KNeighbors(k=1) on normalized data, this implementation provides two significant benefits for EZyRB's core use cases:

  1. Native Scale Invariance: For ND data, it wraps SciPy's NearestNDInterpolator and defaults to rescale=True. This is essential for physical ROM datasets with vastly different parameter magnitudes to prevent severe metric distortion. While one could theoretically inject a MinMaxScaler pipeline into the existing KNeighborsRegressor wrapper to achieve this, doing so would break its clean 1-to-1 transparency with scikit-learn. The Nearest wrapper provides this geometric safety natively.
  2. 1D Efficiency: It routes 1D data directly to scipy.interpolate.interp1d, completely bypassing the unnecessary spatial tree construction overhead required by sklearn.

In conclusion, this provides the pure geometric interpolation toolset requested in #166, logically separated from the ML regression pipeline. Feel free to close the PR without merging if you find these additions are useless.

@ndem0
Copy link
Copy Markdown
Member

ndem0 commented Apr 30, 2026

I do not agree with the two points:

  1. Actually, Plugin has also been introduced for this reason. You should scale your input if you have different scales, no matter the interpolation/regression.
  2. Yes, it's probably more efficient (but please, check it in practice when you claim "my implementation is faster", you just need to run the code and take the elapsed time), but very limited to 1D parameters. But so, doesn't make more sense to have a wrapper of interp1d?

Once more, please do not change the original message of the issues: you wrote that

This PR implements the Nearest Neighbor interpolator requested in #166,

But it's not the truth! The issue is just asking for new interpolators, since Scipy has something like 20-30 different implementations, and it would be nice to exploit them on EZyRB. But no one asked for re-implementing an interpolator already in EZyRB. Maybe next time consider which addition adds more value to the package, instead of just "doing something because you have to". And if you're not sure about it, please ask directly in the issue (or open discussion, propose a plan, etc, there are plenty of possible ways for a constructive development.

@kshitij-maths
Copy link
Copy Markdown
Member Author

Feel free to close the PR!!

@ndem0 ndem0 closed this Apr 30, 2026
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