replace polynomials with horners

[mikofski]

I mentioned it maybe previously, in general Horners is at least 10-15% faster than brute force polynomials with increasing powers because less more efficient operations. Maybe not useful in #2906, but might speed up spa heliocentric longitude:

pvlib-python/pvlib/spa.py

Lines 474 to 475 in 49530b4

	l_rad = (l0 + l1 * jme + l2 * jme**2 + l3 * jme**3 + l4 * jme**4 +
	l5 * jme**5)/10**8

rewritten as Horners:

l0 + (l1 + (l2 + (l3 + (l4 + l5 * jme) * jme) * jme) * jme) * jme

only downside is that it's harder to compare with the original, which could be commented out with a note explaining Horners, and as long as tests pass, it should be okay

[mikofski]

I asked an agent to benchmark Horner's method, substituting repeat calculations like (y-2000) & using scientific notation like 1e8 instead of 10**8. The savings isn't quite what I promised, only 6-7% for a year of hourly or minutely timesteps, but not zero:

spa_python workload	Pre-Horner	Horner	Speedup
1 day	6.029 ms	5.887 ms	2.3%
10 days	45.373 ms	44.449 ms	2.0%
100 days	518.358 ms	508.080 ms	2.0%
2017 hourly (8,760 steps)	45.491 ms	42.596 ms	6.4%
2017 minutely (525,600 steps)	2.766 s	2.564 s	7.3%

Full-year timings are medians of five runs, comparing the pre-Horner implementation with the current one.

It also ran the full test_spa.py suite to make sure nothing changed:

The existing test_spa.py suite passed both before and after: 58 passed, 44 skipped (Numba was unavailable). Numerical comparisons against the original implementation showed maximum absolute differences of 7.28e-12 for delta-T and 1.51e-9 for solar-position outputs. CodeQL found no alerts.

The benchmark branch is here: https://github.com/mikofski/pvlib-python/blob/copilot/benchmark-test-spa/pvlib/spa.py

[cwhanse]

If its not slower, I see no reason not to implement these speedups. 2% here, 5% there, it all adds up.

[echedey-ls]

As you said, as long as readability and reviewability of the code is kept, I'm in favor. For example, comment out the original lines or type the original equation similar to the reference, and explicitly state that the algorithm has been modified to speed it up. Bonus points for indicating the way it has been modified (e.g., applying Horner's method, simplified by exponential properties, etc.).

Another piece that could use an improvement:

pvlib-python/pvlib/atmosphere.py

Lines 389 to 393 in 0c6ab77

	e = coeff[0] * np.exp((coeff[1] * temp_dew) / (coeff[2] + temp_dew))
	es = coeff[0] * np.exp((coeff[1] * temp_air) / (coeff[2] + temp_air))
	# Relative humidity is their ratio, as a percentage.
	relative_humidity = 100 * (e / es)

[adriesse]

I agree this can be made more efficient, but please make the comments really clear about what's happening.

[markcampanelli]

According to the doc's Notes, numpy.polynomial.polynomial.polyval uses Horner's method. Not sure about the overhead of using this API vs. hand coding, as well as other potential compatibility issues with existing pvlib code.

https://numpy.org/doc/stable/reference/generated/numpy.polynomial.polynomial.polyval.html

[adriesse]

polyval has a python loop that saves intermediate sums to a python variable. That probably slows it down compared to hard-coding the formula on one line for a few terms.

[mikofski]

Hi @adriesse & @markcampanelli both were significantly slower (5x) when i tested np.polyval & c-compiled Horner version np.polynomial,polynomial.polyval so i do not recommend them.