Skip to content

Replace interp1d #2394 #2741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jason-rpkt wants to merge 4 commits into
base: main
Choose a base branch
from
+69 −26
Open

Replace interp1d #2394 #2741

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8773810
Replace interp1d #2394
jason-rpkt Apr 23, 2026
ab6c74f
Replace CubicSpline by make_interp_spline k=3
jason-rpkt Apr 24, 2026
c658199
clean up/ simplify/move import to top
jason-rpkt Apr 27, 2026
486cf38
remove list input
jason-rpkt May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/examples/shading/plot_partial_module_shading_simple.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from pvlib import pvsystem, singlediode
import pandas as pd
import numpy as np
from scipy.interpolate import interp1d
from scipy.interpolate import make_interp_spline
import matplotlib.pyplot as plt

from scipy.constants import e as qe, k as kB
Expand Down Expand Up @@ -178,9 +178,9 @@ def plot_curves(dfs, labels, title):


def interpolate(df, i):
"""convenience wrapper around scipy.interpolate.interp1d"""
f_interp = interp1d(np.flipud(df['i']), np.flipud(df['v']), kind='linear',
fill_value='extrapolate')
"""convenience wrapper around scipy.interpolate"""
f_interp = make_interp_spline(np.flipud(df['i']), np.flipud(df['v']), k=1)

Copy link
Copy Markdown
Member

@adriesse adriesse May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one where performance is likely important. Maybe check if np.interp is faster?

return f_interp(i)


Expand Down
31 changes: 26 additions & 5 deletions pvlib/iam.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ def interp(aoi, theta_ref, iam_ref, method='linear', normalize=True):
method : str, default 'linear'
Specifies the interpolation method.
Useful options are: 'linear', 'quadratic', 'cubic'.
See scipy.interpolate.interp1d for more options.
See scipy.interpolate for more options.
Copy link
Copy Markdown
Member

@kandersolar kandersolar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line may need to be edited, depending on https://github.com/pvlib/pvlib-python/pull/2741/changes#r3137773820


normalize : boolean, default True
When true, the interpolated values are divided by the interpolated
Expand Down Expand Up @@ -470,7 +470,7 @@ def interp(aoi, theta_ref, iam_ref, method='linear', normalize=True):
'''
# Contributed by Anton Driesse (@adriesse), PV Performance Labs. July, 2019

from scipy.interpolate import interp1d
from scipy.interpolate import make_interp_spline
Comment thread
kandersolar marked this conversation as resolved.
Outdated

# Scipy doesn't give the clearest feedback, so check number of points here.
MIN_REF_VALS = {'linear': 2, 'quadratic': 3, 'cubic': 4, 1: 2, 2: 3, 3: 4}
Copy link
Copy Markdown
Member

@adriesse adriesse May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a dictionary with names and k values, or something like that, to use below.

Expand All @@ -483,10 +483,31 @@ def interp(aoi, theta_ref, iam_ref, method='linear', normalize=True):
raise ValueError("Negative value(s) found in 'iam_ref'. "
"This is not physically possible.")

interpolator = interp1d(theta_ref, iam_ref, kind=method,
fill_value='extrapolate')
aoi_input = aoi
theta_ref = np.asarray(theta_ref)
iam_ref = np.asarray(iam_ref)
Copy link
Copy Markdown
Member

@kandersolar kandersolar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is np.asarray needed here?

Copy link
Copy Markdown
Contributor Author

@jason-rpkt jason-rpkt Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to use np.asanyarray to keep consistency with np.asanyarray(aoi) below? Or to remove entirely?

np.asarray is to convert to numpy array but I can remove if you prefer.

Copy link
Copy Markdown
Member

@cwhanse cwhanse Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the issue is that the docstrings states numeric for input parameters. pvlib means numeric as one of float, array or Series. Testing is all in one function test_iam_interp which inputs either list, float or array (but not Series).

My opinion: rewrite the test to remove inputting lists, which obviates np.asarray inside iam.interp. And add a test that inputs Series.

Copy link
Copy Markdown
Contributor Author

@jason-rpkt jason-rpkt May 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi both, thanks for your comments. Not sure if I got this right, but I have rewritten the test to remove inputting lists. (And removed np.asarray in iam.interp )
I have also added a check for list inputs at the beginning of iam.interp
Let me know if this is OK.

--Looking at it again, it seems that keeping np.asarray might be "cleaner" than the latest version.


if method == "linear":
spline = make_interp_spline(theta_ref, iam_ref, k=1)

def interpolator(x):
return spline(x)

elif method == "quadratic":
spline = make_interp_spline(theta_ref, iam_ref, k=2)

def interpolator(x):
return spline(x)

elif method == "cubic":
spline = make_interp_spline(theta_ref, iam_ref, k=3)

def interpolator(x):
return spline(x)
Copy link
Copy Markdown
Member

@kandersolar kandersolar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can surely be simplified as @cwhanse mentioned in #2741 (comment)

Copy link
Copy Markdown
Contributor Author

@jason-rpkt jason-rpkt Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplified


else:
raise ValueError(f"Invalid interpolation method '{method}'.")
Copy link
Copy Markdown
Member

@kandersolar kandersolar Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, technically, a breaking change, since the interp1d way also supported 'nearest', 'nearest-up', 'zero', 'slinear', 'previous', and 'next'. I doubt these got much use, if any. Any thoughts on how to handle that?

Copy link
Copy Markdown
Contributor Author

@jason-rpkt jason-rpkt Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I know how to update to support all of those.
Is it OK to update the error message?

Copy link
Copy Markdown
Member

@cwhanse cwhanse May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since interp1d is legacy but not intended for removal, it is still available and we could fall back:

elif method in {'nearest', 'nearest-up', 'zero', 'slinear', 'previous', 'next'}:
    interpolator = interp1d(theta_ref, iam_ref, kind=method,
                               fill_value='extrapolate')

Copy link
Copy Markdown
Member

@kandersolar kandersolar May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's reasonable, as long as we deprecate these weird methods too. If someone really wants these special interpolations, they can do it themselves and provide a pre-calculated input.

Copy link
Copy Markdown
Member

@adriesse adriesse May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of those options really make any sense here. I would vote for dropping them immediately.

Copy link
Copy Markdown
Member

@cwhanse cwhanse May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError(f"Invalid interpolation method '{method}'.")
raise ValueError(f"Interpolation method '{method}' is not supported in pvlib-python.")


aoi_input = aoi
aoi = np.asanyarray(aoi)
aoi = np.abs(aoi)
iam = interpolator(aoi)
Expand Down
17 changes: 8 additions & 9 deletions pvlib/spectrum/response.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import numpy as np
import pandas as pd
import scipy.constants
from scipy.interpolate import interp1d
from scipy.interpolate import make_interp_spline


_PLANCK_BY_LIGHT_SPEED_OVER_ELEMENTAL_CHARGE_BY_BILLION = (
Expand Down Expand Up @@ -66,16 +66,15 @@ def get_example_spectral_response(wavelength=None):
if wavelength is None:
resolution = 5.0
wavelength = np.arange(280, 1200 + resolution, resolution)
x = SR_DATA[0]
y = SR_DATA[1]
spline = make_interp_spline(
x, y, k=3)
Comment thread
kandersolar marked this conversation as resolved.
Outdated

interpolator = interp1d(SR_DATA[0], SR_DATA[1],
kind='cubic',
bounds_error=False,
fill_value=0.0,
copy=False,
assume_sorted=True)

sr = pd.Series(data=interpolator(wavelength), index=wavelength)
values = spline(wavelength)
values[(wavelength < x[0]) | (wavelength > x[-1])] = 0.0

sr = pd.Series(data=values, index=wavelength)
sr.index.name = 'wavelength'
sr.name = 'spectral_response'

Expand Down
26 changes: 26 additions & 0 deletions tests/test_iam.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,32 @@ def test_iam_interp():
with pytest.raises(ValueError):
_iam.interp(0.0, [0, 90], [1, -1])

# check linear after updating interp1d
theta_ref = np.array([0, 60, 90])
iam_ref = np.array([1.0, 0.8, 0.0])

aoi = np.array([0, 30, 60])
iam = _iam.interp(
aoi, theta_ref, iam_ref,
method="linear", normalize=False)
expected = np.array([1.0, 0.9, 0.8])
np.testing.assert_allclose(iam, expected)

# check quadratic
theta_ref = np.array([0, 30, 60, 90])
iam_ref = 1.0 - 1e-4 * theta_ref**2
aoi = np.array([15, 45, 75])
iam = _iam.interp(
aoi,
theta_ref,
iam_ref,
method="quadratic",
normalize=False
)

expected = 1.0 - 1e-4 * aoi**2
np.testing.assert_allclose(iam, expected, rtol=1e-12)


@pytest.mark.parametrize('aoi,expected', [
(45, 0.9975036250000002),
Expand Down
Loading